contract: align SCHEMA.md and the converter with the runner bundle manifests - #20
Conversation
|
…nifests Documents the campaign bundle manifests (metadata.json, plan.json, leg.json, invocation.json) and aligns the converter with the camelCase invocation.json that stellar-rpc#907 writes. ingest.yml becomes a thin wrapper over scripts/ingest.sh.
7770f9a to
aa24c83
Compare
There was a problem hiding this comment.
Pull request overview
Defines and documents the campaign bundle manifest “contract” (especially invocation.json) and updates the converter + CI ingest path to match that contract and improve resilience/diagnostics when bundles contain failed or legacy-shaped invocations.
Changes:
- Expanded
SCHEMA.mdto document all campaign bundle manifests (including newplan.json/leg.json) and updatedinvocation.jsonto camelCase keys with legacy-compat notes. - Updated
converter/convert.pyto warn on failed-run invocations and normalize binary identity keys across legacy/new spellings. - Refactored the GitHub Actions ingest workflow to delegate end-to-end ingest/convert/PR creation to
scripts/ingest.sh, and extended tests/fixtures to cover the schema changes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/ingest.sh | Updates CI documentation and usage-header range to reflect the workflow delegating full ingest behavior to this script. |
| SCHEMA.md | Documents the full campaign bundle manifest set and aligns the written contract with the runner/converter expectations. |
| converter/tests/test_golden.py | Improves golden-test skipping when local datasets are missing or unreadable. |
| converter/tests/test_campaign.py | Adds tests for legacy snake_case binary keys and failed-run invocation warnings. |
| converter/tests/fixtures.py | Updates invocation fixtures to mirror the camelCase invocation.json schema. |
| converter/convert.py | Adds failed-run warnings for invocation.json and normalizes binary identity key spellings. |
| .github/workflows/ingest.yml | Converts the workflow into a thin wrapper around scripts/ingest.sh, including optional passthrough args. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@claude[agent] review |
Reviewed the PR at aa24c83. No code changes needed — the five prior reviewer threads are all resolved with the author's rationale, and there are no open actionable items. Verified locally: |
This PR is the bottom of the campaign-runner stack. It writes down the data contract between the campaign runner and the converter, and it makes the converter match. The runner writes four JSON manifests into a result bundle.
SCHEMA.mdnow documents all four. This layer documentsplan.jsonandleg.jsonbefore the code that writes them lands in parts 3 and 4. The contract comes first, the implementation goes above it.What is in this PR
SCHEMA.mddocuments the campaign bundle manifests:metadata.json(addsfinished_at,campaign.resumed,status), the newplan.jsonandleg.json(runner-owned, additive, ignored by the converter today), andinvocation.jsonwith the camelCase keys that stellar-rpc#907 writes.converter/convert.pyreads binary identity in both spellings._normalize_binarymapscommitHashandbuildTimestamponto the snake_case names, so pre-#907 bundles still resolve.load_invocationswarns loudly when an invocation carries anerrorfield. A failed run writesinvocation.jsontoo, and its CSVs are partial..github/workflows/ingest.ymlbecomes a thin wrapper overscripts/ingest.sh. The workflow passes the bundle path and--dataset-kind. Run identity comes from the bundle's ownmetadata.json. The run arrives as arun/<run_id>PR, not a push tomain.converter/tests/covers both binary spellings and the failed-run warning.test_golden.pyskips when the dataset directory is unreadable, not only when it is absent.Review focus
invocation.jsonsection ofSCHEMA.md: it is a cross-repo contract with stellar-rpc._normalize_binaryandresolve_binary: the commit cross-check must still fire on a mismatch in either spelling.ingest.ymlword-splitsextra_argson purpose. Theshellcheck disable=SC2086marks the documented limit of that input.Verification
Part 7 adds the
shellcheck.ymlCI gate that coversscripts/.Stack: part 1 of 8 · base
main· replaces a slice of #16