Skip to content

validate-contracts: trajectory discovery is location-based, so a recording outside experiments/**/trajectories/ is still silently unvalidated #116

Description

@myselfsiddharth

Follow-up from review of #110.

#110 replaces validate-contracts.mjs's hand-maintained extraTrajectories array with discoverTrajectories(), which walks experiments/ and collects *.json whose parent directory is literally named trajectories. That's a real improvement over the list. But it swaps a hand-maintained list for a hand-maintained location convention — the guarantee still doesn't depend on the data shape, which is the root cause the PR's own description names:

Two holes with one root cause: a guarantee that depends on a specific call path or a hand-maintained list, rather than on the data shape itself.

A recording that lands anywhere else is silently skipped — no warning, no failure, exactly the failure mode #99 set out to remove. Reproduced against discoverTrajectories() directly:

  • experiments/gate-v2/recordings/leaky.json — a plausible sibling directory name. Not discovered. Confirmed with git check-ignore that this path is not gitignored, so it is committable and would sit in the tree permanently unvalidated (including against additionalProperties: false, the mechanism that makes an accidental cookies field unrepresentable).
  • experiments/gate-v1/out/trajectories/leaky.jsonout is in SKIP_DIRS, so even a correctly-named trajectories/ directory is skipped underneath it. Lower severity since experiments/gate-v1/out/ is gitignored, but it means the skip list and the discovery rule can interact in a way that isn't obvious from either one alone.

This is reachable through the supported interface, not just by hand: src/recorder/cli.ts:254 accepts --out to an arbitrary path, defaulting to experiments/gate-v1/trajectories/ only when the flag is absent. Nothing constrains --out to a directory named trajectories.

Why not blocking: only two trajectory files exist in the tree today, both in the canonical location, and both validate — I ran node scripts/validate-contracts.mjs on the branch and it reports clean on exactly the two files the old hand-maintained list named. So #110 is a strict improvement on the status quo and regresses nothing; this is about the residual gap it leaves rather than anything it breaks.

Suggested fix: make discovery shape-based rather than location-based — walk all *.json under experiments/ (minus SKIP_DIRS) and validate any file that looks like a trajectory, e.g. one carrying the trajectory schema's identifying fields (trajectory_id + steps, or a matching $schema/schema_version). Then a recording is checked because of what it is, not where someone put it, and --out to a novel path can't quietly opt out. The existing zero-discovery hard failure should be kept as-is — it's the right instinct and stays useful under either rule.

Not currently exercised by any test — tests/unit/trajectory-guard.test.ts's discovery cases all assert on the canonical location (that the two known files are found, that paths are relative/sorted, that an empty root returns []). Worth adding a case that a trajectory-shaped file outside experiments/**/trajectories/ is picked up, alongside the fix.

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions