Meta: repository-level linting for data managers and tool data tables

[jmchilton]

Summary

Planemo applies ordinary tool-wrapper lint to data managers and can execute data-manager tests, but it does not validate a data-manager/reference-data repository as a connected bundle.

This meta issue proposes a repository-level linting block covering:

• data-manager wrappers and data_manager_conf.xml;
• tool_data_table_conf.xml, .sample, and .test files;
• referenced .loc / .loc.sample fixtures;
• literal data_manager_json examples; and
• expanded tool consumers using from_data_table or data_ref.

The goal is to automate objective bundle-contract checks before adding more requirements to the IUC reviewer checklist.

Motivation and evidence

A frozen audit of 6,791 galaxyproject/tools-iuc pull requests and their review comments combined text/history searches with a current-tree producer/configuration/consumer graph.

The current-tree scan found:

• 82 data managers;
• 297 table consumers;
• 689 table configurations;
• 655 loc paths; and
• 1,321 producer/configuration/consumer relationships.

Its highest-confidence static queue contained ten missing test loc fixtures and six loc/configuration row-shape inconsistencies. A separate review of nine rare sentinels confirmed four schema/row issues, three missing or ineffective fixture contracts, one path/configuration problem, and one macro-expansion false positive.

Representative cases:

• schema/separator/row problems: Add the vsnp_get_snps tool tools-iuc#3381, Add CoverM tool tools-iuc#4524, Add Ensembl VEP (Variant Effect Predictor) tool and corresponding data manager tools-iuc#4533, Add DRAM data manager and set db locations tools tools-iuc#4671;
• missing fixture/configuration problems: Test hisat2 data manager tools-iuc#2682, Add vsnp_determine_ref_from_data tools-iuc#3215, Homer tools-iuc#3867, Adding miRNAture to Galaxy  tools-iuc#4953, Fix bbgbigwig deployment lack of dbkey_loc.sample tools-iuc#6089;
• cross-component table naming/design: Standardise kraken_databases.loc fields tools-iuc#2080, Match twobit table definition to data_manager definition tools-iuc#2736, Update STAR/STARsolo to latest version and fix index data table tools-iuc#3151, Fix kaiju capitalization tools-iuc#6933; and
• required macro-expansion false-positive guard: Data manager update for metaphlan tools-iuc#5003.

This overlaps several long-standing Planemo issues:

• linting: Check sample loc files exist under tool-data/ #96 — check sample loc files exist;
• planemo lint checking for tool_data_table_conf.xml.sample #531 — require tool_data_table_conf.xml.sample when needed;
• Lint does not notice missing tool_data_table_conf.xml.sample #706 — missing table configuration is not noticed; and
• planemo lint should spot empty *.loc files #869 — detect empty/undocumented loc files.

Issue #96 records IUC cases that passed review and failed later during Tool Shed update. The proposed work should consolidate rather than duplicate those issues.

Current behavior and gap

planemo lint delegates individual expanded wrapper sources to galaxy-tool-util. Generic linters now run on tool_type="manage_data", but they only see an individual wrapper and check ordinary command/input/output/test/help/citation rules.

planemo shed_lint --tools adds repository and Tool Shed metadata checks, but it does not parse data_manager_conf.xml, tool_data_table_conf.xml*, and loc assets into one graph.

planemo test discovers tool_data_table_conf.xml.test, falling back to .sample, and can execute a data manager. That is runtime setup rather than lint. Planemo's own data-manager fixture still contains TODO: need some way to test that new entry was added to data table.

Galaxy already has reusable runtime behavior for loc-file resolution, .sample fallback, configured-separator parsing, short-row detection, and unknown returned table names. The missing layer is deterministic repository lint that surfaces these conditions before Galaxy startup or Tool Shed installation.

Proposed repository model

Build a reusable realized data-table bundle containing, with source locations:

• expanded tool/data-manager wrappers;
• manager IDs, declared outputs, and output_ref values;
• manager-configured table names and column mappings;
• configured table names, types, columns/indexes, separators, duplicate policy, and file paths;
• resolved .test and .sample loc assets;
• literal/example data_manager_json table names and keys; and
• consumer references from from_data_table, data_ref, filters, validators, and output actions.

Every diagnostic should distinguish:

• direct, statically resolved evidence;
• unresolved dynamic/macro/generated evidence; and
• tables intentionally supplied by Galaxy core or another installed repository.

Unresolved cases should be reported as advisory or “not checked,” never as demonstrably missing.

P0 static checks

• Parse every repository-owned tool_data_table_conf.xml.test and .sample selected for the realized bundle.
• Resolve recognized path variables such as ${__HERE__} and fail when a literal .test fixture path does not exist.
• Apply Galaxy's configured separator and fail when a non-comment loc row cannot supply every declared column index.
• Resolve literal from_data_table consumers after macro/token expansion and validate them against local definitions.
• Validate literal manager table names against configured tables.
• Validate data_manager_conf.xml output_ref values against real outputs in the expanded manager wrapper.
• Detect duplicate column names/indexes and conflicting schemas for the same local table.
• Validate the expected local bundle is complete: manager/configuration, production sample configuration/loc, test configuration/loc, and consumer references as applicable.
• Detect repository-owned absolute paths and unresolved literal paths that cannot be portable through Tool Shed installation.

Proposed severity

Hard errors should initially be limited to conditions Planemo can prove:

• a referenced literal test fixture is absent;
• a loc row is too short for a declared index or uses an unmistakably wrong separator;
• an output_ref names no expanded wrapper output;
• a literal local table name conflicts across components; or
• a local configuration is structurally ambiguous.

P1 warnings to calibrate

• Warn when a loc row contains extra unused fields. Galaxy currently requires enough fields to address declared indexes but permits extras, so exact-width enforcement needs an explicit policy decision.
• Warn when a local from_data_table reference has no repository definition but might be supplied externally.
• Warn when a .loc.sample is empty and contains no comment documenting its format; do not require default data rows.
• Warn when no declared functional test appears to activate a literal cached-table-backed branch.
• Warn on duplicate rows in shipped loc fixtures and, if adopted as a convention, on an unspecified allow_duplicate_entries policy.
• Treat literal producer coverage as discovery-only when arbitrary Python, shell, or Cheetah constructs data_manager_json.

False-positive and compatibility requirements

1. Expand macros before resolving names. In Data manager update for metaphlan tools-iuc#5003, raw @IDX_DATA_TABLE@ text looks unconfigured but resolves to a complete versioned table bundle.
2. Model externally supplied tables. Galaxy core or another installed repository may validly provide a consumer table.
3. Follow Galaxy row semantics. Too few fields is demonstrably invalid; extra ignored fields are not necessarily invalid today.
4. Do not execute arbitrary producer code during lint. Literal/example JSON can be validated; dynamic producers require functional tests.
5. Preserve source locations and linter names so diagnostics can be skipped narrowly and rendered as PR annotations.

Functional contract tests: follow-up, not static lint

The following are important but cannot be proven reliably by static analysis. They should become a separate Planemo data-manager contract-test mode or follow-up issue:

• run a manager twice and verify intentional reject/replace/coexist behavior (examples: Fix creating new data table on python 3 tools-iuc#2654 and Fix: Prevent duplicate AMRFinderPlus database paths tools-iuc#7949);
• apply and inspect the produced table update, not only compare the data_manager_json file;
• run an ordinary consumer against the cached entry;
• mount cached/reference inputs read-only to detect writes (example: fermikit: avoid writing to inputdir tools-iuc#4105);
• run from a relocated profile/workspace to expose administrator-local assumptions (example: magicblast: don't hard-code default db name tools-iuc#4707); and
• inject download/checksum/extraction/build failures and verify that no loc row or partial reference is published.

Reference-source suitability, compatibility, licensing, checksum policy, and the semantics of duplicate/update behavior remain maintainer decisions. Static lint can enforce those only after a schema or convention exists.

Implementation sketch

The cross-file model and core validations should be reusable galaxy-tool-util logic. Planemo can invoke them from its repository-aware shed_lint path and as a preflight for planemo test.

Keeping the implementation solely in the current per-wrapper linter would not provide enough repository-boundary or cross-file context.

Suggested sequence:

1. Introduce the realized-bundle model and parser tests.
2. Implement path existence and minimum-row-shape checks.
3. Implement expanded consumer/configuration/manager name relationships.
4. Implement output_ref, conflicting-schema, and bundle-completeness checks.
5. Add warnings and calibrate them across tools-iuc before promoting any to errors.
6. Close or supersede linting: Check sample loc files exist under tool-data/ #96, planemo lint checking for tool_data_table_conf.xml.sample #531, Lint does not notice missing tool_data_table_conf.xml.sample #706, and planemo lint should spot empty *.loc files #869 as their scopes are implemented.
7. Open the functional contract-test follow-up with a concrete harness design.

Definition of done

• The P0 checks have unit tests derived from confirmed IUC cases and counterexamples.
• Macro-generated and externally supplied tables have explicit non-error behavior.
• The lints run from a repository-aware Planemo command and before functional tests.
• Diagnostics identify the exact component/path/table and explain the violated relationship.
• Existing issues linting: Check sample loc files exist under tool-data/ #96, planemo lint checking for tool_data_table_conf.xml.sample #531, Lint does not notice missing tool_data_table_conf.xml.sample #706, and planemo lint should spot empty *.loc files #869 are linked, resolved, or explicitly left for remaining scope.
• The new checks are calibrated against tools-iuc, with false positives reviewed before error-level enforcement.
• A separate dynamic contract-test issue captures idempotence, actual table application/consumption, write isolation, relocation, and failure atomicity.

Completing this block should leave any future IUC data/reference checklist focused on behavioral and policy judgments rather than mechanically detectable bundle errors.

[jmchilton]

Posted by Claude (AI assistant) on jmchilton's behalf — not authored by them personally.

Proposed Planemo ↔ galaxy-tool-util seam

Sketching where the work should split, mirrored on APIs that exist on both sides today.

Anchors:

• galaxy-tool-util already provides the reporting bus. LintContext + lint_ctx.lint(name, fn, target) (galaxy/tool_util/lint.py). Planemo's shed_lint.lint_repository already drives exactly this pattern over its RealizedRepository (planemo/shed_lint.py), and lint_tool_source_with(lint_ctx, tool_source) is the existing precedent for "planemo hands an object + the shared ctx to galaxy-tool-util, diagnostics flow back."
• galaxy-tool-util already owns the row/loc primitives. TabularToolDataTable.parse_file_fields(filename, errors=..., here="__HERE__"), .separator, parse_column_spec, allow_duplicate_entries all live in galaxy.tool_util.data.
• galaxy-tool-util already models the manager/producer side. galaxy/tool_util/data/bundles/models.py (shipped in 26.0.x) has convert_data_tables_xml(elem) -> DataTableBundleProcessorDescription, which parses the data-manager <data_tables> block into output_ref values, per-table column mappings, moves, and value translations — with ready helpers output_ref_by_data_table and data_table_names. The runtime DataTableBundle there is the emitted data_manager_json artifact. This work should reuse that, not reinvent it.

So the seam is: planemo owns discovery (which files), galaxy-tool-util owns the cross-file model + checks, LintContext is the wire. Same shape as tool linting today.

galaxy-tool-util side (new — galaxy/tool_util/data/bundles/repository.py)

A cross-file lint view that composes the existing bundle models rather than shadowing them. (The existing DataTableBundle name means "runtime data_manager_json output" — deliberately not reused for this.)

# repository.py  (new — beside existing models.py)
from galaxy.tool_util.data.bundles.models import DataTableBundleProcessorDescription

@dataclass(frozen=True)
class ManagerDecl:
    id: str
    tool_output_names: FrozenSet[str]              # real outputs in the expanded DM wrapper (for output_ref check)
    processor: DataTableBundleProcessorDescription # REUSED: output_ref values, table names, column maps
    source: SourceLoc

@dataclass(frozen=True)
class RepositoryDataTables:
    """Cross-file lint view of one repository's data-table bundle. Pure data — no lint logic."""
    managers: List[ManagerDecl]
    configured_tables: List[TableDecl]   # from tool_data_table_conf.xml* — name, columns/indexes, separator,
                                         #   dup policy, loc paths; wraps a configured TabularToolDataTable
    loc_assets: List[LocAsset]           # resolved .test / .sample, exists flag
    consumers: List[ConsumerRef]         # from_data_table / data_ref / filters / validators + SourceLoc
    external_table_names: FrozenSet[str] # supplied by Galaxy core / other installed repo (req #2)

# build.py
def build_repository_data_tables(
    tool_sources,             # ALREADY macro-expanded by caller (req #1)
    data_manager_conf,        # parsed via existing convert_data_tables_xml
    tool_data_table_confs,    # .test preferred, .sample fallback — caller decides
    repo_root,
) -> RepositoryDataTables: ...

Net-new modeling is really just the configured-table side (tool_data_table_conf.xml* has only the runtime TabularToolDataTable today, no clean model), the resolved loc assets, and consumer refs. The manager side and literal data_manager_json reuse bundles/models.py.

galaxy-tool-util side (checks + group entry point)

Model each P0 check as a plain lint_func(target, lint_ctx) callable — same style as planemo's existing lint_expansion(realized_repository, lint_ctx) — so each gets a stable skippable name. Wrap them in one entry point analogous to lint_tool_source_with_modules:

# lint.py
def lint_repository_data_tables(lint_ctx: LintContext, model: RepositoryDataTables) -> LintContext:
    """Single entry point planemo calls. Each sub-check independently skippable by name."""
    lint_ctx.lint("data_table_fixture_exists",  _lint_fixture_exists,  model)  # P0: literal .test path missing
    lint_ctx.lint("data_table_row_shape",       _lint_row_shape,       model)  # P0: short row / wrong separator
    lint_ctx.lint("data_table_output_ref",      _lint_output_ref,      model)  # P0: output_ref names no output
    lint_ctx.lint("data_table_consumer_ref",    _lint_consumer_ref,    model)  # P0: from_data_table vs local defs
    lint_ctx.lint("data_table_schema_conflict", _lint_schema_conflict, model)  # P0: conflicting columns
    lint_ctx.lint("data_table_bundle_complete", _lint_completeness,    model)
    return lint_ctx

_lint_output_ref is mostly reuse — cross ManagerDecl.processor.output_ref_by_data_table against ManagerDecl.tool_output_names. Same for literal manager table names via processor.data_table_names.

Row-shape reuses the shipped runtime primitive rather than re-parsing. Note parse_file_fields is an instance method — it relies on self.separator, self.largest_index, self.comment_char, self.name, all set from the table's config element in TabularToolDataTable.__init__ + parse_column_spec. So the model must carry a configured TabularToolDataTable per table; you cannot call it off the class:

def _lint_row_shape(model, lint_ctx):
    for table in model.configured_tables:
        data_table = table.instance   # configured TabularToolDataTable (separator + largest_index set)
        for loc in table.loc_assets:
            errors: list = []
            data_table.parse_file_fields(loc.path, errors=errors, here=data_table.here)
            for e in errors:
                lint_ctx.error(f"{loc.path}: {e}", linter="data_table_row_shape")

(The primitive already prefixes each error with Line N in tool data table '...', so the row number is in the text without extra work.)

Checks emit through lint_ctx.error/warn/info exactly as tool linters do — planemo's handle_lint_complete picks up the counts, skip_types handles narrow suppression, no new reporting path.

On source locations / req #5 (PR annotations): the framework's automatic file:line rendering only exists for XML targets — XMLLintMessageLine / XMLLintMessageXPath read node.sourceline / node.getroottree() off a live lxml Element passed as lint_ctx.error(..., node=elem). That path is available for XML-node diagnostics (a <column> in data_manager_conf.xml, a <table> in the conf). For non-XML loc-file diagnostics (a bad row in a .test/.sample tabular file) there is no lxml node, so the plain LintMessage class ignores any extra kwargs and a SourceLoc dataclass will not auto-render a location — the path (and line, when known) must be embedded in the message string, exactly as _lint_row_shape does above. Machine-readable annotations from these would need a new LintMessage subclass in galaxy-tool-util that carries path/line; that's additional scope, not something the existing bus provides for free.

Planemo side (call site — one insertion)

Planemo builds the model from the RealizedRepository it already has, then adds the entry point into lint_repository, gated like the existing --tools block:

# planemo/shed_lint.py — inside lint_repository, near the --tools block
if kwds.get("data_tables"):                       # new CLI flag, mirrors kwds["tools"]
    from galaxy.tool_util.data.bundles.build import build_repository_data_tables
    from galaxy.tool_util.data.bundles.lint import lint_repository_data_tables

    model = build_repository_data_tables(
        tool_sources=[ts for _, ts in yield_tool_sources(ctx, realized_repository.path, recursive=True)],
        data_manager_conf=_find_dm_conf(realized_repository),
        tool_data_table_confs=_find_ttt_confs(realized_repository),
        repo_root=realized_repository.real_path,
    )
    lint_repository_data_tables(lint_ctx, model)  # its internal lint_ctx.lint(...) calls do per-check naming

The same build_repository_data_tables(...) + lint_repository_data_tables(...) pair is what the issue's "preflight for planemo test" would call from planemo/galaxy/config.py right after _find_tool_data_table resolves the .test/.sample files — reusing discovery planemo already does there.

Why the seam sits here

Concern	Owner	Reason
Which files exist in the repo	planemo	Only planemo has RealizedRepository + shed expansion + .test→.sample discovery.
Macro/token expansion of tool sources	planemo (already done)	yield_tool_sources returns expanded ToolSource; satisfies false-positive req #1 before galaxy-tool-util sees anything.
Cross-file model + all row/loc/schema checks	galaxy-tool-util	Reusable (issue's explicit ask), composing the existing bundles/models.py manager side.
Reporting / skip / exit code	shared LintContext	Already the contract for tool linting; nothing new.

Release sequencing

• The manager-side model (convert_data_tables_xml, output_ref_by_data_table) and the row/loc primitives are already in released galaxy-tool-util 26.0.x, which planemo can consume under its current galaxy-tool-util>=25.1,<26.1 pin.
• _lint_schema_conflict depends on the conflicting-columns / column-merge work currently on galaxy dev (targets 26.1), which is outside planemo's current pin.

So the new repository.py + checks are new galaxy-tool-util code shipped in a galaxy-tool-util release; planemo then bumps its pin and adds the small call site above. The parsing-only P0 checks can start against the current pin; the schema-conflict check waits on the 26.1 release + pin bump.

[jmchilton]

Posted by Claude (AI assistant) on behalf of @jmchilton — not authored personally.

Update: there's now a working implementation of this repository-level linter and it's been validated against the live tools-iuc tree.

Implementation (WIP branch): https://github.com/jmchilton/galaxy/tree/data_validation

• New galaxy.tool_util.data.bundles linters: MissingLocFixture, LocRowShape, ManagerTableConfigured, ConsumerTableDefined, OutputRefValid, DuplicateColumnNames, ConflictingTableSchema.
• Standalone endpoint galaxy-tool-data-lint <repo> plus wiring into Planemo's shed_lint.

Live sweep — ran the linter over all 245 tools-iuc repos with a tool_data_table_conf.xml*. Results (zero crashes, zero false-positive errors):

• 6 real ERRORs:
  • LocRowShape (malformed loc rows — spaces where tabs are required): data_manager_checkm2, data_manager_dram_database_downloader, tools/checkm2, tools/coverm, tools/vsnp
  • MissingLocFixture (dangling loc reference): data_manager_hisat2_index_builder (a bwa_mem_indexes BWA-template leftover)
• 15 ConsumerTableDefined WARNINGs, all correct de-escalations — references to Galaxy-standard tables (all_fasta, fasta_indexes, __dbkeys__) or a table supplied by another repo (clair3_models). These are surfaced as warnings, not errors, exactly because they may be satisfied cross-repo.

Three of the six are already fixed with PRs, each independently confirmed by shed_lint:

• data_manager_checkm2: fix TAB separator in test-data loc tools-iuc#8234 (checkm2 data manager)
• data_manager_dram_database_downloader: fix tab separators in loc test data tools-iuc#8235 (dram)
• data_manager_hisat2_index_builder: drop stray bwa_mem_indexes table tools-iuc#8236 (hisat2 index builder)

The count lines up with the audit's "six loc/configuration row-shape inconsistencies" prediction, which is a decent signal the static checks match the originally-scoped queue.