fix(context): adopt contextgraph 2.0.0's attestation fields - #6504
fix(context): adopt contextgraph 2.0.0's attestation fields#6504macanderson wants to merge 1 commit into
Conversation
…tQueryResult
context-graph-protocol's Downstream Canary builds stella against CGP main
and fails to compile: ContextQueryResult gained frame_attestations and
result_attestation, and stella's construction sites did not list them
(E0063). CGP has not published this version to crates.io yet (no release
token/environment for its publish workflow), so the workspace pins move
from a registry version to a git rev pinned to CGP main HEAD instead of
bumping the version string.
- Cargo.toml: contextgraph-{types,host,trace,conformance} now pin
{ git, rev } at CGP main HEAD (d446bb55dc48604b9e664546dc30ce8cb7531920)
instead of "=0.1.2". Comment records why this git pin does not repeat
this workspace's past git-pin incident: the earlier pin named a commit
reachable from no branch and eligible for GC; this one is an ancestor of
an actively-merged branch.
- deny.toml: allow-git widened to the CGP repo, with matching reasoning.
- stella-context/src/provider.rs, stella-context/src/retrieval/outcome.rs,
stella-cli/src/contextgraph.rs: every ContextQueryResult construction
site (5 production, 4 test doubles) sets frame_attestations: Vec::new()
and result_attestation: None, each commented with why empty is correct
(every in-tree provider holds no signing key and mints no commitment).
- stella-cli/src/contextgraph/tests.rs: added a serde round-trip witness
proving an unattested result serializes with both fields omitted from
the wire, with a genuinely-attested result as the control. Updated
pinned_protocol_version_is_a_conformance_verified_wire_version: CGP's
wire name is still contextgraph/1.0, so the 0.1.x draft spelling is
dropped from the verified set.
Refs #4359
There was a problem hiding this comment.
Sorry @macanderson, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 2 days and 3 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Reviewer's GuideThe PR updates Stella to CGP’s new ContextQueryResult shape by pinning the unreleased CGP 2.0.0 crates to a reachable main revision, initializing attestation fields as unattested across all construction sites, and adding serialization/conformance coverage to preserve the existing wire behavior. Sequence diagram for ContextQueryResult wire round tripsequenceDiagram
participant Test as Conformance test
participant Result as ContextQueryResult
participant Serde as Serde JSON
Test->>Result: construct with unattested fields
Test->>Serde: serialize
Serde-->>Test: JSON without frame_attestations or result_attestation
Test->>Serde: deserialize
Serde-->>Test: equivalent ContextQueryResult
Test->>Result: construct attested control
Test->>Serde: serialize
Serde-->>Test: JSON containing both attestation keys
Flow diagram for unattested ContextQueryResult constructionflowchart LR
A[Provider or retrieval pipeline] --> B[ContextQueryResult]
B --> C["frame_attestations = Vec::new()"]
B --> D["result_attestation = None"]
C --> E[Serde serialization]
D --> E
E --> F[Attestation keys omitted from wire JSON]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
This reverses #819, deliberately, and that should be visible from both sides — so this comment is mirrored there. What #819 decided. It moved these four crates off Why the reproducibility half does not apply here. #819's pin named a commit on a history line the upstream had re-rooted away from: reachable from no branch and no PR, and a garbage collection on a cold cache away from breaking every build. This pin is The audit half is a real residual cost, and worth naming plainly. Why not simply wait for the registry release. CGP #102 cannot be done from a session: the repository has zero Actions secrets and no One correction to the record while I am here. The canary script The exit condition is already written into both files. |
|
Converting this to a draft and taking auto-merge off. I was wrong, and the tree said so.
Its assertion message is written for exactly this change:
I argued for this change from a stale comment, and that was the error. I told the author that the canary script's header — which describes stella as consuming these crates "as a pinned git dependency" — meant the registry pin was drift from the intended design. It is the reverse. That comment is the stale artefact; the registry pin is the design. The history is worse than the one issue I checked. #819 was the supply-chain and reproducibility risk. #878 was its residual, and it is the one that matters here: each of So the bar for reversing it is not "write a good comment explaining why this git pin is safer than the last one". It is "delete a guard that two incidents paid for". That is a different decision and not one to make from a session, especially not one I reasoned into from the wrong document. What is still good here, and why this is a draft rather than closed. The substantive work stands: the two new What it is actually waiting for. context-graph-protocol#102, the crates.io publish. Once I have posted the same correction on stella#4359 and on the CGP issue, because I put the wrong conclusion on both of those earlier today and they should not be left standing. |
What & why
context-graph-protocol's Downstream Canary builds stella against CGPmainand now fails at compile time: CGP 2.0.0 addedframe_attestations: Vec<FrameAttestation>andresult_attestation: Option<ProvenanceAttestation>toContextQueryResult, and stella's construction sites did not list them (E0063).CGP has not published 2.0.0 to crates.io — the release workflow's publish token and environment do not exist yet (context-graph-protocol#102) — so there is no registry version to pin. This PR moves the four
contextgraph-*workspace pins from"=0.1.2"to a git revision,d446bb55dc48604b9e664546dc30ce8cb7531920(CGPmainHEAD at the time of this change), and wires the two new fields through every construction site.This also turns CGP's Downstream Canary green once merged — see https://github.com/macanderson/context-graph-protocol.
On the git pin
Stella dropped a git pin for exactly these crates once before (#819), after an earlier
revwent stale: it named a commit reachable from no branch, eligible for garbage collection on a cold cache. This pin is not that risk — the revision is an ancestor of CGP's actively-mergedmain, so it stays reachable as long as that branch does.Cargo.toml's comment above the pin anddeny.toml's comment aboveallow-gitboth record this reasoning; revert both to a registry pin ("=2.0.0",allow-git = []) in the same change once CGP publishes.The witness
main, passes here)crates/stella-cli/src/contextgraph/tests.rs:unattested_context_query_result_round_trips_without_the_attestation_fields_on_the_wireconstructs aContextQueryResultwith all five fields — the literal that does not compile onmainagainst CGP 2.0.0 — then proves the wire claim behind invariant 4 (an unattested answer is byte-identical to a pre-attestation one): the struct serializes withframe_attestations/result_attestationentirely absent from the JSON, not as[]/null, and round-trips. A genuinely-attestedContextQueryResultis the control, proving both keys appear when populated.The gate
cargo fmt -p stella-context -p stella-clicargo clippy -p stella-context -p stella-cli --tests -- -D warnings(clean)cargo test -p stella-context --lib(188 passed) andcargo test -p stella-cli --bin stella contextgraph::(38 passed)./scripts/check-lockfile-sync.sh,./scripts/check-file-size.sh,python3 scripts/check-prose.py— all OKcargo deny check—sources ok(the newallow-gitentry is accepted),advisories ok, bans ok, licenses okrust-versionis1.90, matching stella's declared MSRV.Fix over file
Ground-rule check
stella-coreed25519-dalek/curve25519-dalek/der/pkcs8/etc. as new transitive dependencies (CGP's own attestation-adjacent crypto stack), not a direct dependency stella added.cargo deny check banspasses;multiple-versionsstays atwarnperdeny.toml's existing policy.Anything reviewers should know?
This PR does not close #4359. Its DoD asks for the registry pin (
"=2.0.0") and the crates.io publish itself, which stays blocked on context-graph-protocol#102 — a human repo-settings action outside this session's reach. It also asks therecord_hashcoupling comment inCargo.tomlbe confirmed against the published 2.0.0 artifact; I re-confirmedserde_json_canonicalizer = "0.3.2"and the canonicalizer/feature routing against CGPmainat the pinned revision (the same commit that will become 2.0.0), but not against a published crate, since none exists. And a manual re-run of CGP's Downstream Canary needs a merge here first, plus CGP-side access this session does not have.What's done: every construction-site and conformance-test item in the DoD checklist, the lockfile, and the wire round-trip witness. What's left, tracked by #4359: the crates.io publish, moving these four pins back to a registry version once it lands, and the post-merge canary confirmation.
Refs #4359
Summary by Sourcery
Adopt Context Graph Protocol 2.0 result attestation fields and track its unreleased crates from a stable git revision.
Bug Fixes:
Enhancements:
Build:
Tests: