Skip to content

test(dataset): assert the provenance sidecar payload whole - #402

Merged
kstonekuan merged 1 commit into
Hebbian-Robotics:mainfrom
chiruu12:test/391-dataset-sidecar
Sep 4, 2026
Merged

test(dataset): assert the provenance sidecar payload whole#402
kstonekuan merged 1 commit into
Hebbian-Robotics:mainfrom
chiruu12:test/391-dataset-sidecar

Conversation

@chiruu12

@chiruu12 chiruu12 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Closes #391.

The dataset provenance sidecar had five keys no test read back: dataset_manifest_version, created_at, total_episodes, the coverage block, and workspace_id. test_the_manifest_and_its_provenance_land_together already loaded the file, so this widens that test rather than adding a second one.

Following #388, the top level is asserted as a whole dict. That pins absence as well as presence. pipeline is popped and compared separately, since it is json.loads(manifest.to_json()) and pinning it whole would mean restating every check on every pipeline change.

Two things the fixture needed:

  • created_at is now passed explicitly, so the assertion pins that the sidecar records the caller's stamp rather than a second now() taken inside _sidecar_payload.
  • The workspace identity is minted in the test. Ingest does not mint one, so workspace_id was None in this fixture, which is also what dropping the key would look like.

coverage gets real numbers: one entry for duration, 1 of 1. Not merely a non-empty list.

Mutations run against the assertions, each one alone:

mutation result
drop dataset_manifest_version fails
set the version to a literal 2 fails
drop total_episodes fails
replace coverage with [] fails
drop the coverage key fails
write episodes_ran as 0 fails
force workspace_id to None fails
stamp created_at inside the payload fails

tests/test_dataset.py is 13 passed, 1 skipped. The full suite matches main exactly here, 31 failed and 1194 passed either way, from hflow_server not being installed locally.

Copilot AI lite review requested due to automatic review settings September 4, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merging. Your table reproduces, all eight, and two more I added for good measure (dropping name and row_count) fail too, so the whole-dict assertion covers the three keys that were already checked as well as the five that were not.

Minting the workspace identity is the part that took judgment, and your reasoning in the comment is exactly right: asserting workspace_id is None would have been indistinguishable from asserting the key was dropped. A test that passes for both the correct and the broken behavior is worse than no test, and you spotted that the fixture handed you one.

Passing created_at explicitly rather than letting it default does the same job for a different reason, and the mutation proves it: stamping a fresh now() inside _sidecar_payload fails now, so the field is pinned as the caller's value rather than merely as a plausible timestamp.

Popping pipeline and comparing it above is the right call. A whole-dict assertion that swallowed the entire pipeline manifest would fail on every unrelated check change, and a test that cries wolf gets edited into uselessness.

Gate on the merged result: ruff check, ruff format --check, ty check clean, 1526 passed / 6 skipped. Your 31 local failures are hflow-server not being installed; uv sync --locked --all-extras picks it up if you want them quiet.

Closes #391.

@kstonekuan
kstonekuan merged commit 5ca985a into Hebbian-Robotics:main Sep 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The dataset provenance sidecar records a schema version and coverage block that no test reads back

3 participants