Add the PLAN-623 branch-local citation evidence contract and the CT-001 detector-sweep packet - #3444
Add the PLAN-623 branch-local citation evidence contract and the CT-001 detector-sweep packet#3444jslee02 wants to merge 38 commits into
Conversation
…tor sweep Adopt the citation-driven contact-trust evidence contract on release-6.20 as additive tooling only (no library, API, ABI, or default change): - PLAN-623 dashboard entry, branch design doc, and dev-task handoff after the 2026-08-14 audit (PLAN-621 owns CT-018 with #3056/#3428, PLAN-622 and PR #3431 own CT-020, PR #3377 owns exact-Coulomb fixtures). - Branch-local claims-manifest.json (design-doc sidecar owns release-6.20 lane state; claim identity stays owned by the main corpus) and the fail-closed check-citation-evidence gate wired into check-lint, with a permanent intentionally incomplete negative-control packet and 33 pytest cases registered in test-ai-infra. - First complete CT-001 rolling-direction packet: deterministic sphere slide-to-roll launch-angle sweep across fcl, dart, bullet, and ode with the default boxed-LCP solver and per-run requested-vs-readback detector identity. All four detectors break rotational symmetry beyond tolerance (fcl/dart/ode lateral drift up to 2.101e-3 m, bullet 6.696e-4 m), so the bounded claim is disposition reproduced within its recorded boundary; unsupported LCP internals are typed unsupported, never zero.
Address two independent review passes on 1189155: - Close the bypass where a lane could satisfy the evidence check with a file the packet checks never reached: every lane-referenced path is now validated wherever it sits, and prose, non-JSON files, paths outside evidence/, negative controls, and duplicate packet owners are rejected. - Make "unsupported is never silently zero" an enforced rule: every exact zero inside a measured metric group must be typed {status: unsupported, reason} or declared in measured_zero_fields, and spelled placeholders, nulls, NaNs, dangling raw_paths, empty measurement windows, and empty-container metrics now fail. - Type the release-6.20 solver internals honestly: per-solve LCP iteration counts, residuals, and Dantzig-vs-PGS fallback events carry branch-specific unsupported markers instead of prose or zeros. - Correct the CT-001 attribution. bullet exceeds the drift tolerance without the antisymmetric friction-pyramid signature (its antisymmetry residual exceeds its own peak drift and its largest drift sits at 45 degrees, where the mechanism predicts zero), so it is excluded from the reproducing set; the claim reproduces on fcl, dart, and ode. dart and ode are bit-identical at every angle and that is now recorded, so the sweep is not read as four independent measurements. - Gate the disposition on physical validity (rolling speed 5/7 v0, rolling reached, no fall-through, bounded penetration) so a degenerate run can no longer strengthen a "reproduced" verdict. Tests grow to 48 cases covering the bypass and each new rule. Still additive tooling only: no library, API, ABI, default, or packaging change.
Round-2 verification refinements on release-6.20: - Assert detector type, timestep, and gravity against the request per run and across repeats, so the resolved-provenance string describes what the writer actually checks rather than more than it checks. - Derive deterministic_repeats_identical from the comparison instead of hardcoding it. - Record the round-2 findings in the task verification log.
…cord An independent review of afc6d7a found two false statements in this branch's own records: - The round-2 verification block described recordSolverDiagnostics, an AVBD fall-through, and a solver_iterations_by_method helper. None of that exists on release-6.20 -- it was DART 7 text imported by mistake into the artifact that certifies what was checked here. Replaced with the branch-local record of what was actually verified, and the error is noted rather than hidden. - bullet's largest lateral drift is at 75 deg, not 45 deg. Its 45 deg drift is 90% of its largest, which still fails the pyramid signature, and the exclusion rests on the 1.79 antisymmetry ratio either way. Corrected in the packet limitation, the writer, and the verification record. Also: the disposition validity gate now fails a run whose per-step kinetic gain exceeds a thousandth of the launch energy, so a blow-up cannot pass the gate while a symmetry verdict is drawn (measured: bullet 1.9e-4 J against a 5.0e-4 J threshold, fcl/dart/ode ~3.0e-8 J). The overclaim in the previous commit message is recorded in verification.md.
Final independent review of d4f95f1 found: - A genuine bypass: a non-string entry in a lane's evidence list was silently dropped, so a lane could be closed by [{"path": ...}] with nothing validated. Non-string entries now fail. - raw_paths accepted a directory, negative controls in a subdirectory were enumerated by neither loop, and scene.digest was format-checked but never recomputed against the parameters the packet publishes. All fixed. - The packet attested to assertions and a target.commit its recorded run predated, because writer changes had landed without regenerating. The packet is regenerated on the current writer; its raw_rows are byte-identical because the fixture is deterministic, which the packet evidences through its per-repeat trajectory hashes. - Writers now carry forward recorded review passes instead of erasing them on every regeneration, so the two-review floor can be met without hand-editing a generated file. Tests grow to 52 cases. Still additive tooling only.
…-citation-contact-trust
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7314b4476c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| claim_id, lane_name = linked | ||
| if packet.get("claim_id") != claim_id: | ||
| errors.append( | ||
| f"{packet_path}: claim_id {packet.get('claim_id')} does " | ||
| f"not match manifest lane {claim_id}.{lane_name}" | ||
| ) |
There was a problem hiding this comment.
Match closed-lane dispositions to their packets
When a lane is marked closed, this cross-check verifies the packet's claim ID and branch but never compares the lane's disposition with packet.result.disposition. Consequently, a manifest lane declared fixed or reproduced can reference an otherwise valid unresolved packet and check-citation-evidence still succeeds, allowing the manifest's authoritative result to contradict its evidence.
Useful? React with 👍 / 👎.
| if rel in closed_lane_packets: | ||
| passes = ( | ||
| packet.get("review", {}).get("passes") | ||
| if isinstance(packet.get("review"), dict) | ||
| else None | ||
| ) | ||
| if not isinstance(passes, list) or len(passes) < 2: | ||
| errors.append( | ||
| f"{packet_path}: a packet closing a lane needs at " | ||
| "least two recorded review passes" |
There was a problem hiding this comment.
Require independent review passes before closing a lane
For a closed lane, checking only len(passes) >= 2 lets two identical entries from the same reviewer satisfy the gate; I verified that duplicating {"reviewer": "same", "summary": "same"} makes validate_tree() return no errors. Reject duplicate passes or record and validate distinct reviewers/roles so the advertised independent-or-role-separated review floor cannot be bypassed.
Useful? React with 👍 / 👎.
| raw_paths = evidence.get("raw_paths") | ||
| raw_rows = evidence.get("raw_rows") | ||
| has_paths = isinstance(raw_paths, list) and bool(raw_paths) | ||
| has_rows = isinstance(raw_rows, list) and bool(raw_rows) | ||
| if not (has_paths or has_rows): | ||
| errors.append("evidence must carry raw_rows inline or non-empty raw_paths") |
There was a problem hiding this comment.
Validate the contents of inline raw rows
The inline evidence check accepts any non-empty list, including raw_rows: [null] or raw_rows: ["prose"]; I verified that such a packet can still close a lane with no validation errors. Since raw_rows is the only required underlying evidence when no paths are supplied, require each entry to be a non-empty row object (and reject null/scalar placeholders) rather than treating list presence as raw data.
Useful? React with 👍 / 👎.
| ## Initial DART 6 rows | ||
|
|
||
| Prioritize: | ||
|
|
||
| 1. Audit/guard the completed `3k_shapes` and sleeping/collision performance |
There was a problem hiding this comment.
Move the initial-row priorities to the roadmap
This durable design document introduces an ordered “Prioritize” list, creating a second mutable priority source alongside docs/plans/dashboard.md; as PLAN-623 advances, these two lists can diverge and route later work differently. Keep only the durable row rationale here and place the ordering/cap in the roadmap or active task state.
AGENTS.md reference: docs/design/AGENTS.md:L17-L21
Useful? React with 👍 / 👎.
| @@ -0,0 +1,171 @@ | |||
| # DART 6.20 Citation-Driven Contact Trust | |||
There was a problem hiding this comment.
Add this design document to the design index
The new direct docs/design/*.md document is not listed in docs/design/README.md, so readers using the directory's documented index cannot discover this durable owner. Add it to the Files table with the other design documents.
AGENTS.md reference: docs/design/AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-6.20 #3444 +/- ##
================================================
+ Coverage 75.39% 75.44% +0.04%
================================================
Files 449 449
Lines 53705 53705
================================================
+ Hits 40492 40517 +25
+ Misses 13213 13188 -25
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7314b4476c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| review = existing.get("review") | ||
| if isinstance(review, dict) and isinstance(review.get("passes"), list): | ||
| return {"passes": review["passes"]} |
There was a problem hiding this comment.
Clear reviews when regenerating a packet
When an existing packet already has two review passes and the writer is rerun after its scene, calculations, or output changes, this helper blindly copies those passes into the regenerated packet. validate_tree() then treats the stale passes as reviews of the new evidence and can close the lane without the required post-change review; clear the passes on regeneration or bind each pass to the reviewed packet digest/target state.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| "evidence.visual object form must be " | ||
| "{'status': 'not-applicable', 'reason': ...}" | ||
| ) | ||
| elif not (isinstance(visual, list) and visual): |
There was a problem hiding this comment.
Validate each visual artifact entry
When a packet claims applicable visual evidence, this accepts any non-empty list, including visual: [null] or visual: ["prose"]. Such a packet can therefore close a simulation lane without an assessable capture even though this is the only structural check for its visual artifacts; require every entry to have a valid artifact structure and resolvable evidence rather than checking list length alone.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| if previous_energy is not None: | ||
| max_energy_gain = max(max_energy_gain, kinetic - previous_energy) |
There was a problem hiding this comment.
Include the first step in the energy-gain gate
Because previous_energy is initially None, the transition from the launched state to the state after the first world.step() is omitted from max_energy_gain_j. If the initial contact solve injects excessive energy and later steps dissipate it before the final-speed checks, the physical-validity gate can still label the run valid; initialize the comparison from the pre-step kinetic energy so every simulated step is covered.
Useful? React with 👍 / 👎.
| corpus_reference = manifest.get("corpus_reference") | ||
| if ( | ||
| not isinstance(corpus_reference, dict) | ||
| or not _is_nonempty_str(corpus_reference.get("path")) | ||
| or not _is_nonempty_str(corpus_reference.get("branch")) | ||
| ): |
There was a problem hiding this comment.
Pin the corpus reference to its canonical owner
The manifest contract says claim identity remains owned by the DART 7 corpus on main, but this check accepts any two non-empty strings, such as {"path":"wrong","branch":"release-6.20"}. A future manifest can therefore silently point at another corpus while the gate still passes, defeating the stated protection against claim-ID/title forks; validate the canonical branch and corpus path for this schema.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed5074fdff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if rel in lane_evidence: | ||
| owner = lane_evidence[rel] |
There was a problem hiding this comment.
Canonicalize lane evidence paths before indexing
When two lanes reference the same packet using evidence/./packet.json and evidence/packet.json, the raw strings become separate keys even though both resolve to one file. I verified that this lets a closed CT-001 lane point to a zero-review packet owned by an in-progress CT-002 lane while validate_tree() returns no errors, bypassing the one-owner, claim-ID, and closed-lane review checks. Normalize and validate each path before storing it in lane_evidence and closed_lane_packets.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| elif base_dir is not None and not any( | ||
| (root / raw_path).is_file() for root in (base_dir, REPO_ROOT) | ||
| ): |
There was a problem hiding this comment.
Keep raw evidence paths inside the repository
When a packet supplies an absolute path, root / raw_path resolves to that absolute location, so a machine-local file such as /etc/hosts satisfies the raw-evidence requirement; I verified that an otherwise valid packet with only that raw_paths entry produces no errors. This allows a lane to close using unrelated host data rather than reproducible repository evidence, so resolve each entry and reject paths outside the permitted repository evidence roots.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| has_sweep = isinstance(sweep, list) and len(sweep) >= 2 | ||
| has_seeds = isinstance(seeds, list) and len(seeds) >= 2 | ||
| if not (has_repeats or has_sweep or has_seeds): |
There was a problem hiding this comment.
Require distinct valid ensemble samples
When deterministic_repeats is absent, any two list entries satisfy the ensemble gate without being validated or distinguished; I verified that both sweep: [null, null] and two identical sweep objects pass packet_errors(). Such a packet can represent only one run while claiming the required ensemble evidence, so require sweep/seed entries to have the expected structure and at least two distinct samples.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
| elif ( | ||
| isinstance(leaf, str) and leaf.strip().lower() in UNSUPPORTED_SENTINELS | ||
| ): | ||
| errors.append( |
There was a problem hiding this comment.
Require an actual measured metric leaf
When a measured metric group contains only arbitrary prose, the string check rejects only a short placeholder blacklist, so {"method": "m", "measurement": "totally unmeasured prose"} passes with no numeric measurement or typed unsupported value; I verified this through packet_errors(). This contradicts the measured-with-method contract and permits a packet to close without the promised metrics, so require at least one finite numeric measurement or formally typed metric value rather than accepting arbitrary strings as the entire group.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed5074fdff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not isinstance(value, dict) or not value: | ||
| errors.append(f"configuration.{side} must be a non-empty object") |
There was a problem hiding this comment.
Validate requested and resolved configuration fields
A packet whose configuration.requested and configuration.resolved are both {"placeholder": null} passes this check because only object non-emptiness is validated. With the other superficial string fields populated, such a packet can eventually close a lane without recording the promised solver, detector, thread, or default identities, undermining the fail-closed evidence contract; require the expected identity fields and validate their value shapes.
Useful? React with 👍 / 👎.
| "signature; bullet exceeds the drift tolerance without that " | ||
| "angular structure and is excluded. Says nothing about other " |
There was a problem hiding this comment.
Derive detector conclusions from the actual sweep
When Bullet is not compiled—as explicitly supported by available_detectors()—or when a different build produces different Bullet results, the regenerated packet still asserts that Bullet exceeded the drift tolerance and was excluded; the limitations similarly hard-code Bullet statistics and a DART/ODE equivalence. This makes generated evidence contradict its recorded detector set or computed findings, so build these statements from detectors, anisotropy_findings, and identical_detector_groups rather than embedding results from one run.
Useful? React with 👍 / 👎.
Addresses the 15 findings from the PR #3444 Codex reviews (validator rules mirrored from the main-branch contract): Validator: closing-lane disposition must equal the packet's result.disposition; distinct reviewers for the two-review floor; review passes bind to a content_digest of the packet minus its review block, so regeneration invalidates stale reviews; raw_rows entries must be non-empty structured records; raw_paths/visual paths must be relative, non-escaping, and resolve inside an approved root; sweep/seed ensembles need valid distinct entries; string metric leaves only under semantic keys; requested/resolved configurations must carry a recognizable identity key and no nulls; NaN/Infinity rejected at load; lane evidence paths normalized before indexing; corpus_reference pinned to the canonical DART 7 corpus path and branch; target.fetch_hint required (PR head refs keep target commits fetchable after squash-merge). preserve_review() keeps only digest-bound passes; record_review_pass() added. CT-001 writer: the energy-gain physical-validity gate now starts from the launched pre-step state so the first contact solve is covered, and the claim boundary/limitations are derived from the computed detector sets (anisotropic, nonconforming, identical-hash groups) instead of hardcoding one historical run's bullet statistics -- a build without bullet, or a bullet that starts conforming, now regenerates consistent conclusions. Packet regenerated: disposition unchanged (reproduced); bullet still excluded, now by computed criteria. Design docs: the ordered Prioritize list moved out of the durable design document (ordering is mutable task state owned by the PLAN-623 task home; the document keeps the row set and rationale), and the document is now listed in the docs/design/README.md index. Tests: 52 -> 65 validator cases; check-citation-evidence passes on the real tree; the permanent negative control keeps failing.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae4f3b5f5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif isinstance(scene.get("parameters"), dict): | ||
| # When the packet publishes the parameters the digest was taken | ||
| # over, recompute it. A digest that cannot be reproduced from the | ||
| # packet's own scene description binds nothing. |
There was a problem hiding this comment.
Require digest-bound scene content
When scene.parameters is absent or is not an object, this branch skips recomputation and accepts any syntactically valid SHA-256 value. A packet containing only an ID and a fabricated digest can therefore close a lane without reproducible scene content. Require inline parameters or a referenced scene artifact whose bytes are actually checked against the digest.
Useful? React with 👍 / 👎.
| elif isinstance(leaf, (int, float)) and not isinstance(leaf, bool): | ||
| if not math.isfinite(leaf): | ||
| errors.append(f"metrics.{name}.{path} contains a non-finite number") | ||
| elif leaf == 0: | ||
| observed_zero_fields.append(path) |
There was a problem hiding this comment.
Reject non-measurement scalar metric leaves
Fresh evidence after the prior prose-metric fix: boolean and other unsupported scalar leaves still fall through this type chain without an error. For example, a measured group containing only {"method": "m", "measurement": true} passes because the boolean increments leaf_count but is neither validated as a number nor rejected, allowing a closed lane to claim measured metrics without a measurement.
Useful? React with 👍 / 👎.
| reviewers = { | ||
| entry.get("reviewer") | ||
| for entry in passes | ||
| if isinstance(entry, dict) | ||
| and _is_nonempty_str(entry.get("reviewer")) | ||
| } |
There was a problem hiding this comment.
Normalize reviewer identities before counting them
Fresh evidence after adding the distinct-reviewer check: the set uses unnormalized reviewer strings, so passes from reviewer-a and reviewer-a count as two independent reviewers even though _is_nonempty_str() strips whitespace only for validation. Normalize identities before inserting them into the set, or bind reviews to stable reviewer IDs, so trivial spelling variations cannot satisfy the closure gate.
Useful? React with 👍 / 👎.
| if _is_nonempty_str(item): | ||
| item_path = item | ||
| elif ( | ||
| isinstance(item, dict) | ||
| and _is_nonempty_str(item.get("path")) | ||
| and _is_nonempty_str(item.get("description")) | ||
| ): | ||
| item_path = item["path"] |
There was a problem hiding this comment.
Require visual paths to identify visual artifacts
Fresh evidence after the prior visual-entry fix: a nonempty string is now treated as an artifact path without checking its media type, and _evidence_path_issue() only checks that some file exists. Consequently, visual: ["CHANGELOG.md"] passes when validation uses the repository design directory, allowing a simulation lane to claim applicable visual evidence using an ordinary text file; validate an accepted visual/media format and its artifact metadata rather than existence alone.
AGENTS.md reference: AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
Mirrors the main-branch round-2 fixes (adversarial re-tests of round 1): scene.parameters is required so every digest binds to published content; the metric-leaf type chain is exhaustive (booleans accepted explicitly as measured findings, all other leaf types rejected); reviewer identities are normalized (strip + casefold) before the distinct-reviewer closure count; visual evidence entries must carry a recognized media suffix. Tests: 65 -> 69 validator cases. The CT-001 packet is regenerated at this commit in the next commit so its recorded command runs the current writer at its recorded target.
target.commit now names a commit that contains the current writer, so the recorded command plus fetch_hint reproduce this packet from a clean checkout -- the round-2 CT-007 finding on main, applied to this branch's packet. Disposition unchanged (reproduced); bullet remains excluded by the computed criteria.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e96ea71db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| window = ensemble.get("measurement_window") | ||
| if "measurement_window" not in ensemble: | ||
| errors.append("ensemble.measurement_window is required") | ||
| elif not window or (isinstance(window, str) and not window.strip()): |
There was a problem hiding this comment.
Validate measurement-window structure
When a packet uses measurement_window: true, 1, [null], or another truthy placeholder, this condition accepts it as an actual window, so a lane can close without recording when its measurements were collected. Require a supported window shape, such as finite ordered start/end values or another explicitly defined representation, rather than testing truthiness alone.
Useful? React with 👍 / 👎.
| "cannot stand in for visual evidence" | ||
| ) | ||
| continue | ||
| if PurePosixPath(item_path).suffix.lower() not in VISUAL_MEDIA_SUFFIXES: |
There was a problem hiding this comment.
Verify visual artifacts by content
Fresh evidence after the prior visual-path fix: the new suffix check still accepts a prose file renamed to capture.png; I verified that a file containing not an image produces no packet errors. Consequently, applicable visual evidence can still be satisfied without a readable capture, so verify the media signature or parse the artifact and bind appropriate metadata instead of trusting its filename extension.
Useful? React with 👍 / 👎.
| f"configuration.{side} carries null values at {null_keys}; " | ||
| "record the identity or omit the key" | ||
| ) | ||
| if not _has_identity_key(value): |
There was a problem hiding this comment.
Reject empty configuration identity values
Fresh evidence after adding recognizable identity-key checks: _has_identity_key() treats any non-null value as an identity, so both configuration.requested and .resolved can be {"solver": ""}, {"solver": []}, or {"solver": false} and pass validation. Such packets still omit the promised solver/detector identity while being eligible to close a lane; require identity values to have a valid non-empty scalar or explicitly supported structured shape.
Useful? React with 👍 / 👎.
Mirrors the round-3 fixes: structural measurement_window validation (non-empty string or non-empty object of finite numeric values with start_s <= end_s); visual artifacts verified by media signature rather than filename suffix; configuration identity values must be non-empty strings; review passes require an explicit verdict 'pass' (record_review_pass stamps it). Tests: 69 -> 73 validator cases.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 034b1cc7a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not any( | ||
| isinstance(row, dict) | ||
| and any( | ||
| "seed" in str(key).lower() and row[key] == seed for key in row | ||
| ) | ||
| for row in raw_rows |
There was a problem hiding this comment.
Consume a distinct raw row for each declared seed
When a seed ensemble contains multiple seeds, each independent any(...) search can be satisfied by the same row. For example, seeds [7, 11] pass with rows [{'seed': 7, 'other_seed': 11, 'metric': 1}, {'metric': 2}], even though the second row records neither seed. This lets a lane close without a distinct observation for every declared seed; build match sets and enforce a one-to-one assignment as the sweep path does.
Useful? React with 👍 / 👎.
| start = window.get("start_s") | ||
| end = window.get("end_s") | ||
| if _is_finite_number(start) and _is_finite_number(end) and start > end: | ||
| errors.append( | ||
| "ensemble.measurement_window start_s must not exceed end_s" |
There was a problem hiding this comment.
Require a non-empty time measurement window
For time-based windows, this rejects only start_s > end_s, so both {'start_s': 1.0, 'end_s': 1.0} and entirely negative bounds pass packet_errors(). A zero-duration window cannot contain the measurements used to close a lane, while the step-based form already requires at least one continuation step; require a valid non-empty interval and define whether negative bounds are supported.
Useful? React with 👍 / 👎.
| "source", | ||
| "target", | ||
| "scene", | ||
| "configuration", | ||
| "ensemble", | ||
| "metrics", | ||
| "evidence", | ||
| "result", | ||
| "review", | ||
| "host", |
There was a problem hiding this comment.
Enforce the provenance fields promised by the design contract
The packet contract in docs/design/dart6_citation_driven_contact_trust.md:74-86 says every packet records source model/license/conversion provenance and Gazebo relevance, but these are neither required here nor validated later; the committed CT-001 packet consequently omits all of them and still passes. This allows future evidence to close without stating whether imported material is licensed or converted, or whether the result is downstream-relevant, so add structured required fields (including explicit not-applicable forms).
Useful? React with 👍 / 👎.
Declared seeds need DISTINCT observed rows; time windows must be non-empty intervals with non-negative start; measured performance is forbidden while host.performance_valid is false; parquet footer metadata-length sanity; reproduction command lists begin with the build step (writer emits it; the packet was migrated in place); the design doc's packet-contract list is aligned with the enforced schema (model/license provenance applies when external assets are used; procedural scenes carry their construction in scene.parameters). Tests: 117 -> 121.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62e08627e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for index, command in enumerate(commands): | ||
| if not COMMAND_RE.match(command.strip()): |
There was a problem hiding this comment.
Require a build step before evidence execution
When a future packet lists only a command such as pixi run test, this loop accepts it and packet_errors() returns no errors even though the evidence contract requires a build step. Such a packet can close a lane using an absent or stale build from the host rather than artifacts produced from target.commit; require a recognized build command and ensure it precedes commands that execute the evidence harness.
Useful? React with 👍 / 👎.
| if not ( | ||
| isinstance(header, dict) | ||
| and {"descr", "shape", "fortran_order"} <= set(header) | ||
| and isinstance(header["shape"], tuple) |
There was a problem hiding this comment.
Validate that NPY dtypes contain measurements
Fresh evidence after the structural NPY fixes: this condition requires the descr key but never checks its dtype, so a valid NPY containing only a string array such as ['not a measurement'] passes _raw_data_content_issue() and can be the packet's sole raw artifact with no validation errors. Restrict descr to numeric/boolean dtypes and verify the payload size matches the declared shape so raw binary evidence cannot close a lane without measurements.
Useful? React with 👍 / 👎.
The validator requires a recognized build step in evidence.commands; NPY descr dtypes are restricted to numeric/boolean/complex (structured record dtypes are a recorded boundary); a measured-with-method group whose every quantity is typed unsupported is rejected -- an unmeasured group must be typed unsupported as a whole. Tests: 121 -> 123.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08ff92bb48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| performance_group = ( | ||
| packet.get("metrics", {}).get("performance") | ||
| if isinstance(packet.get("metrics"), dict) | ||
| else None |
There was a problem hiding this comment.
Validate allocation measurements against host validity
Fresh evidence after host provenance became mandatory: when host.performance_valid is false, this branch checks only metrics.performance, so a packet with measured metrics.allocation values still passes validation. Allocation counts are also explicitly described here as requiring a controlled host, so an uncontrolled run can close a lane with invalid allocation evidence; apply the host-validity gate to both performance and allocation groups.
Useful? React with 👍 / 👎.
| for index, row in enumerate(raw_rows) | ||
| if isinstance(row, dict) | ||
| and all(row.get(key) == value for key, value in point.items()) |
There was a problem hiding this comment.
Reject absent coordinates when matching sweep rows
Fresh evidence after the one-to-one sweep matcher was added: row.get(key) == value treats an absent field as matching a declared null coordinate. For example, a sweep of [{"angle_deg": null}, {"other_angle_deg": null}] and two unrelated measurement rows passes packet_errors() with no errors, allowing a lane to claim configurations that were never observed; require coordinate values to be valid and each key to be present in the matched row.
Useful? React with 👍 / 👎.
| if isinstance(row, dict) | ||
| and any( | ||
| "seed" in str(key).lower() and row[key] == seed for key in row | ||
| ) |
There was a problem hiding this comment.
Match seeds only through actual seed fields
Fresh evidence after enforcing distinct rows per seed: the substring check accepts any key containing seed, including unrelated fields such as unseeded_metric. I verified that seeds [7, 11] with rows [{"unseeded_metric": 7}, {"unseeded_metric": 11}] produce no packet errors, so the lane can still claim an unperformed seeded ensemble; restrict matching to the supported seed field names or an explicit seed schema.
Useful? React with 👍 / 👎.
Allocation measurements also require a performance-valid host; sweep points may not declare null coordinates; seed fields match by whole token; the build step must precede the evidence command. Tests: 123 -> 127.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c96974730
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| build_indices = [ | ||
| index | ||
| for index, command in enumerate(commands) | ||
| if re.match(r"^pixi run build\b", command.strip()) |
There was a problem hiding this comment.
Recognize environment-prefixed build commands
When a packet uses the command form explicitly accepted above, such as DART_PARALLEL_JOBS=4 pixi run build, COMMAND_RE accepts it but this anchored match does not classify it as a build step. The validator then reports that no build command exists, so valid evidence using an allowed environment prefix cannot pass; strip the accepted assignments before identifying and ordering build commands.
Useful? React with 👍 / 👎.
Env-prefixed build commands are recognized (a genuine false positive in the round-17 gate); NPY payloads verified against the declared array size; artifact digests no longer count as repeat verification; bookkeeping keys matched case-insensitively by whole token. Tests: 127 -> 131.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0562b1a911
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "checkout from target.fetch_hint has no artifacts, and " | ||
| "an existing checkout may hold stale ones" | ||
| ) | ||
| elif non_build_indices and min(build_indices) > min(non_build_indices): |
There was a problem hiding this comment.
Require an evidence execution command
Fresh evidence after the build-order fix: a packet whose entire evidence.commands list is ['pixi run build'] still produces no validation errors because non_build_indices may be empty. The repository's build task only compiles the project, so such a packet can close a lane without providing any command that runs the simulation or regenerates its claimed measurements; require at least one valid non-build command after the build.
Useful? React with 👍 / 👎.
| BUILD_COMMAND_RE = re.compile( | ||
| r"^(?:[A-Za-z_][A-Za-z0-9_]*=[^\s;|&`$]+[ \t]+)*pixi run build\b" | ||
| ) |
There was a problem hiding this comment.
Because the build regex ends at build\b, it also classifies pixi run build-nothing as a build command even though no such task exists in the inspected pixi.toml. I verified that ['pixi run build-nothing', 'pixi run python scripts/example.py'] passes packet_errors(), allowing a lane to close with a reproduction sequence that never builds anything; match an explicitly supported build task rather than any name beginning with build-.
Useful? React with 👍 / 👎.
| and isinstance(header["descr"], str) | ||
| and re.match(r"^[<>|=]?[bifuc][0-9]+$", header["descr"]) |
There was a problem hiding this comment.
Reject zero-width NPY dtype descriptors
Fresh evidence after the current dtype and payload checks: the regex accepts a descriptor such as <i0, making itemsize zero and allowing a header-only array with shape (10,) to pass as raw measurement evidence. I reproduced a 74-byte file that yields no packet errors while numpy.load() rejects it with descr is not a valid dtype descriptor; validate legal positive-width dtypes before calculating the required payload.
Useful? React with 👍 / 👎.
| for index, entry in enumerate(sweep): | ||
| if isinstance(entry, dict) and entry: | ||
| canonical_points.append(json.dumps(entry, sort_keys=True)) |
There was a problem hiding this comment.
Deduplicate numerically equivalent sweep points
Fresh evidence after the distinct-point checks: JSON serialization treats {"angle_deg": 1} and {"angle_deg": 1.0} as different strings, although they represent the same numeric configuration and the row matcher considers them equal. Two rows for that single setting therefore satisfy both the distinct-point and distinct-row gates with no errors, allowing one configuration to masquerade as a sweep; canonicalize JSON numbers by value before testing uniqueness.
Useful? React with 👍 / 👎.
Build-only command lists are rejected (the sequence must also run the evidence command); the build match requires the exact task name (arguments allowed, invented suffixes not); zero-width NPY dtypes are rejected; numerically equivalent sweep points deduplicate. Tests: 131 -> 135.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc6e8a4cf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"configuration.{side} carries null values at {null_keys}; " | ||
| "record the identity or omit the key" | ||
| ) | ||
| if not _has_identity_key(value): |
There was a problem hiding this comment.
Require every promised configuration field
A packet whose configuration.requested and configuration.resolved contain only {"detector": "fcl"} passes this identity check even when solver, iterations, threads, and defaults are omitted. This lets a lane close without the configuration provenance explicitly required by docs/design/dart6_citation_driven_contact_trust.md:84-85, so validate each required field individually and require typed-unsupported values where appropriate.
Useful? React with 👍 / 👎.
| non_build_indices = [ | ||
| index for index in range(len(commands)) if index not in build_indices |
There was a problem hiding this comment.
Require the non-build command to execute evidence
Fresh evidence after the build-only fix: ['pixi run build', 'pixi run lint'] now produces no packet errors because every command not classified as a build is assumed to execute the harness. The repository defines lint as formatting tasks, so this sequence can close a lane without running or regenerating any claimed measurements; recognize an evidence-producing command rather than accepting any non-build task.
AGENTS.md reference: AGENTS.md:L14-L22
Useful? React with 👍 / 👎.
| try: | ||
| float(cell.strip()) | ||
| except ValueError: | ||
| continue | ||
| has_numeric_cell = True |
There was a problem hiding this comment.
Reject non-finite CSV measurement cells
For CSV/TSV artifacts, float() accepts NaN, Infinity, and -Infinity, after which has_numeric_cell is set without checking finiteness. I verified that a digest-bound file containing metric,other\nNaN,text can be the sole raw measurement artifact for a hash-bearing repeat packet with no validation errors, so require math.isfinite() before treating a parsed cell as measurement content.
Useful? React with 👍 / 👎.
| footer_len = int.from_bytes(footer[:4], "little") | ||
| file_size = path.stat().st_size | ||
| if not (0 < footer_len <= file_size - 12): | ||
| return mismatch |
There was a problem hiding this comment.
Decode Parquet metadata before accepting the artifact
Fresh evidence after the Parquet-stub fix: these checks accept any positive-length bytes as the footer without decoding Parquet's metadata or confirming that a measurement row exists. I constructed a 26-byte file containing PAR1, arbitrary junk, an arbitrary ten-byte “footer,” its length, and trailing PAR1; _raw_data_content_issue() accepts it and a digest-bound repeat packet using it as its sole raw artifact has no errors. Parse the container and require non-empty numeric/boolean data, or stop accepting .parquet artifacts.
Useful? React with 👍 / 👎.
Case-insensitive identity suffix exclusion; configurations must name a solver/method/integrator identity (detector alone is not a configuration; requested method sweeps count as identity lists); a non-build command must run a repository harness; CSV numeric cells must be finite; Parquet dropped from supported raw formats (stdlib cannot decode it -- fail-closed means unsupported). Tests: 135 -> 139.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8b56e5fab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue | ||
| if not math.isfinite(cell_value): | ||
| continue | ||
| has_numeric_cell = True |
There was a problem hiding this comment.
Exclude bookkeeping-only CSV cells from measurements
When a repeat-only packet cites a digest-bound CSV such as seed,note\n7,control, this branch treats the finite seed value as measurement content because it never associates cells with their column headers. I verified that this artifact can be the packet's sole raw_paths entry with no validation errors, allowing a lane to close using only run metadata; parse the header and apply the existing bookkeeping-key exclusion before setting has_numeric_cell.
Useful? React with 👍 / 👎.
| if total_size < header_start + header_len + expected_payload: | ||
| # Truncated payload: the declared array does not fit in the file. | ||
| return mismatch | ||
| return None |
There was a problem hiding this comment.
Reject NPY artifacts containing no finite measurements
When the sole raw artifact is a valid NPY or NPZ numeric array containing only NaN or infinity, the header and payload-size checks return success without inspecting any element values. I verified that a digest-bound numpy.save(..., [nan]) artifact produces no packet errors, unlike equivalent JSON and CSV evidence whose non-finite values are rejected; decode the supported numeric/boolean payload and require at least one finite measurement before accepting it.
Useful? React with 👍 / 👎.
Target commits must exist in the repository object store (platform CI check-lint checkouts now fetch full history; shallow clones error explicitly), evidence commands may only reference harness scripts that exist, NPY/NPZ payloads must carry at least one finite element (unsupported float widths are rejected as unverifiable), and CSV finite-cell scanning classifies columns by header so bookkeeping-only tables cannot close a lane. Tests: 139 -> 149.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 175eefa5b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # arbitrary placeholder object. Keys that merely mention an identity word in | ||
| # a metadata role (method_note, backend_reason, ...) do not count. | ||
| FETCH_HINT_RE = re.compile( | ||
| r"^git fetch origin pull/3444/head && git checkout ([0-9a-f]{40})$" |
There was a problem hiding this comment.
Accept fetch hints from future evidence PRs
When CT-002/CT-003 are produced on the new branch explicitly planned in docs/dev_tasks/dart6_citation_contact_trust/RESUME.md:44-45, their target commits will be reachable through a new PR ref rather than PR #3444. This hard-coded regex rejects that valid fetch command, while retaining pull/3444/head cannot check out the new commit, so the required check-citation-evidence lint gate makes the next evidence slice impossible to land; accept and validate the packet's actual numeric PR ref instead of pinning this PR.
Useful? React with 👍 / 👎.
| for dim in header["shape"] | ||
| ) | ||
| and isinstance(header["descr"], str) | ||
| and re.match(r"^[<>|=]?[bifuc][1-9][0-9]*$", header["descr"]) |
There was a problem hiding this comment.
Restrict NPY headers to valid dtype widths
Fresh evidence after the zero-width hardening: an NPY header declaring <i3 with shape (1,) and three payload bytes passes _raw_data_content_issue(), although numpy.load() rejects <i3 as an invalid dtype descriptor. With a matching artifact digest and trajectory hash, this file can be the packet's sole raw_paths artifact and packet_errors() returns no errors, so constrain each dtype kind to legal widths rather than accepting every positive decimal width.
Useful? React with 👍 / 👎.
| elif not any( | ||
| re.search(r"python[ \t].*scripts/", commands[index]) | ||
| for index in non_build_indices |
There was a problem hiding this comment.
Identify the actual evidence-producing harness
Fresh evidence after the harness-execution check: a packet using ['pixi run build', 'pixi run python scripts/parallel_jobs.py'] produces no validation errors. The inspected parallel_jobs.py only prints a build-job count and neither runs the experiment nor regenerates its measurements, so stale or fabricated rows can still close a lane; bind the command to the packet's declared generator or otherwise recognize an artifact-producing harness rather than any existing file under scripts/.
Useful? React with 👍 / 👎.
Fetch hints accept any evidence PR ref (future slices were impossible to land under the pinned regex), NPY dtype widths are whitelisted per kind, the harness command must invoke an evidence writer, packets are source-bound to the manifest's canonical citation, and --fetch-target-refs restores squash-merged target commits before the platform CI object-store checks. Tests: 149 -> 155.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 312114d234
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else: | ||
| if len(head) < 12: | ||
| return mismatch | ||
| (header_len,) = struct.unpack("<I", head[8:12]) | ||
| header_start = 12 |
There was a problem hiding this comment.
Reject unsupported NPY format versions
Fresh evidence after the dtype-width hardening: every NPY major version other than 1 is parsed as though it used the v2/v3 four-byte header length. A digest-bound file marked version 9.0 with a valid-looking <f8 header and payload therefore passes _raw_data_content_issue(), even though numpy.load() rejects the unsupported version, allowing a lane to rely on bytes that are not a valid NPY artifact; accept only the supported version tuples before parsing the header.
Useful? React with 👍 / 👎.
| repeat_hash_sources = [evidence_for_repeats, ensemble] | ||
| if has_repeats and not any( | ||
| _has_hash_leaf(source) for source in repeat_hash_sources | ||
| ): |
There was a problem hiding this comment.
Bind repeat hashes to the repeated trajectories
Fresh evidence after excluding artifact_digests: _has_hash_leaf() still accepts any hash-named field in the ensemble. A repeat packet with no trajectory digest passes when given only ensemble.unrelated_hash = "aaaa...", so deterministic_repeats_identical: true can close a lane without any hash representing the repeated runs; require a supported trajectory/per-repeat hash field rather than searching arbitrary keys.
Useful? React with 👍 / 👎.
| re.search( | ||
| r"python[ \t].*scripts/write_citation[A-Za-z0-9_]*\.py", | ||
| commands[index], | ||
| ) | ||
| for index in non_build_indices |
There was a problem hiding this comment.
Bind the writer command to the packet it regenerates
Fresh evidence after restricting execution to write_citation*.py: the regex accepts any such writer, regardless of the packet being validated. For example, a future CT-002 packet can cite scripts/write_citation_ct001_rolling_direction_packet.py and pass, although that script hard-codes claim_id: CT-001 and writes the CT-001 experiment, so the recorded command cannot reproduce the packet; validate the invoked writer and output arguments against the packet or record an explicit generator identity.
Useful? React with 👍 / 👎.
| if suffix == ".json": | ||
| try: | ||
| parsed = json.loads(path.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
Reject non-standard raw JSON artifacts
When a raw .json artifact contains both a finite measurement and a non-standard constant, such as {"metric": 1, "bad": NaN}, Python's permissive json.loads() accepts it and the finite leaf makes the artifact pass. Duplicate object keys are accepted similarly, despite the packet loader explicitly rejecting both cases because strict consumers can fail or interpret them differently; load raw JSON through the same strict constant and duplicate-key hooks before allowing it to serve as evidence.
Useful? React with 👍 / 👎.
| def _canonical_value(value: object) -> object: | ||
| # 1 and 1.0 are the same coordinate; the row matcher's == | ||
| # treats them as equal, so the distinct-point check must too. | ||
| if isinstance(value, int) and not isinstance(value, bool): | ||
| return float(value) |
There was a problem hiding this comment.
Canonicalize nested sweep coordinates recursively
Fresh evidence after numeric-equivalent point deduplication: _canonical_value() normalizes integers only when they are direct values of the sweep object. Thus {"config": {"x": 1}} and {"config": {"x": 1.0}} are considered distinct, while the row matcher treats their nested dictionaries as equal; two duplicate rows for that single configuration then satisfy the distinct-row assignment and let one setting masquerade as a sweep. Apply numeric canonicalization recursively, or make matching type-sensitive in the same way as deduplication.
Useful? React with 👍 / 👎.
NPY format versions are whitelisted, repeat binding requires the supported trajectory/state_sha256 key family, the writer command must regenerate THIS claim's packet, raw JSON loads through the strict NaN/duplicate-key hooks, sweep points canonicalize recursively, and sweep coordinates are excluded from row-measurement checks. Tests: 155 -> 161.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1da3966e38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "tests/test_sync_ai_commands.py", | ||
| "tests/test_ai_infrastructure.py", | ||
| "tests/test_install_git_hooks.py", | ||
| "tests/test_check_citation_evidence.py", |
There was a problem hiding this comment.
Fetch packet targets before running AI infrastructure tests
In the Ubuntu ai-infrastructure job, .github/workflows/ci_ubuntu.yml still uses the default shallow checkout and then runs this expanded test-ai-infra task. The newly included suite executes test_repository_tree_validates, which rejects the CT-001 historical target commit when it is absent from a shallow object store; I reproduced the resulting cannot be verified in a shallow clone failure with a depth-one clone. Add the full-history checkout and target-ref fetch to this job as well as the separate build job, otherwise this required CI job fails on the committed packet.
Useful? React with 👍 / 👎.
| re.search( | ||
| r"python[ \t].*scripts/write_citation_" | ||
| + re.escape(claim_id.lower().replace("-", "")) | ||
| + r"[a-z0-9_]*\.py", | ||
| commands[index], |
There was a problem hiding this comment.
Require the writer filename to be the executed script
Fresh evidence after the claim-specific matcher was added: ['pixi run build', 'pixi run python -c pass scripts/write_citation_ct001_rolling_direction_packet.py'] still produces no packet errors. Here Python executes pass and treats the writer filename only as an inert argument, because this regex searches for the filename anywhere after python; consequently a packet can pass with a command that never regenerates its evidence. Parse the command tokens and require the matching writer to be Python's script operand rather than an arbitrary later argument.
Useful? React with 👍 / 👎.
Summary
tooling only: a branch-local claims manifest, the fail-closed
pixi run check-citation-evidencegate wired intocheck-lint, apermanent negative control, 65 pytest cases, and the first
release-6.20evidence packet — a CT-001 rolling-direction sweep across the fcl, dart,
bullet, and ode collision detectors.
the diff touches only
docs/,scripts/,tests/,pixi.toml, andCHANGELOG.md, verified by independent review.Motivation / Problem
reproducible dispositions on the maintained LTS line. PLAN-623
(docs/design/dart6_citation_driven_contact_trust.md, dashboard entry
added here) gives them stable rows and fail-closed packets while
preserving the compatibility contract; claim identity stays owned by the
maincorpus viacorpus_reference, so rows cannot fork across branches.Changes / Key Changes
claims-manifest.json(singledart6lane per row) underthe design-doc sidecar, so evidence survives dev-task cleanup.
scripts/check_citation_evidence.py— the same fail-closed packetcontract as
main, adapted torelease-6.20lane ownership; incheck-lint; permanent negative control; 65 tests intest-ai-infra(the pinned task list in
scripts/check_ai_infrastructure.pyis extendedin the same change, which is how that guard is designed to grow).
default boxed-LCP solver, per-run detector readback asserted, two
bit-identical repeats per cell. The claim reproduces on fcl, dart, and ode
(lateral drift 2.1e-3 m with the antisymmetric friction-pyramid
signature: nulls at 0/45/90 deg, antisymmetry residual ~1e-15 of peak);
bullet exceeds the drift tolerance without that angular structure
(residual 1.79x its own peak, largest drift near 45 deg where the
mechanism predicts zero) and is excluded from the reproducing set rather
than counted as a fourth corroboration. dart and ode are bit-identical at
every angle and the packet says so.
not exist as public API on this branch; they are typed
{status: unsupported, reason}rather than reported as zeros.Testing
pixi run check-lint(includes the new gate) andpixi run test-all—158/158 tests pass on the branch head.
pixi run python -I scripts/run_pytest.py tests/test_check_citation_evidence.py -q— 65 passed.pixi run -e gazebo test-gznot triggered: the diff has no collision,constraint,
World::step, parser, package, or downstream-sensitivechange (documented in the dev-task verification log with the file list).
in-branch and recorded, including two corrections to this branch's own
records (an imported wrong-branch verification block and a wrong
largest-drift angle).
Breaking Changes
Related Issues / PRs (backports)
mainPR: branchfeature/citation-trust-foundation(independent; owns the corpus, PLAN-123, and the DART 7 packets).
Checklist