Add the PLAN-123 citation-trust foundation: fail-closed evidence contract, six first-wave packets, and resolved-configuration bindings - #3445
Conversation
Land the citation-driven simulation-trust foundation on main: - PLAN-123 plan, durable contact-trust design, citation-claim corpus with the 2026-08-14 current-state audit, and the dev-task handoff. - Machine-checked claims-manifest.json (20 rows, corpus-consistent) and the fail-closed check-citation-evidence gate wired into check-lint: missing target commit, scene digest, requested/resolved solver identity, command, ensemble, disposition, claim boundary, or typed-unsupported metrics fail validation, and a permanent intentionally incomplete negative-control packet must keep failing. - First complete CT-001 rolling-direction packet: deterministic sphere slide-to-roll launch-angle sweep under SEQUENTIAL_IMPULSE and BOXED_LCP with per-method requested/resolved readback; friction-pyramid lateral drift up to 2.1e-3 m with the 0/45/90-degree symmetry nulls, so the bounded claim is disposition reproduced within its recorded boundary. - 53 pytest cases proving field-by-field fail-closed behavior, registered in test-ai-infra.
Address two independent review passes on d0cb640: - Close the bypass where a manifest 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 rather than a promise: 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. - Fix the packets that violated it. StepMetrics.last_step_residual is structurally zero on the rigid contact path (recordSolverDiagnostics takes residual = 0.0 and no rigid call site passes one) and the boxed-LCP branch records no iteration count at all, so both are now typed unsupported with their source citations in scripts/citation_packet_utils.py instead of being published as measurements. - Distinguish mechanism from deviation in CT-001: record which isotropy criterion fired, test the antisymmetric friction-pyramid signature, and gate the disposition on physical validity so a degenerate run cannot strengthen a "reproduced" verdict. - Add CT-002 dense inelastic contact (reproduced: sequential impulse at dt=4 ms is still moving at the 2 s horizon) and CT-003 dense elastic contact (unresolved: no energy injection and no solver failure, recorded as an honest negative result rather than a refutation of the original report). Tests grow to 68 cases covering the bypass and each new rule.
Round-2 verification found the first dense-contact packets still overclaimed: - CT-002's verdict rested on one cell crossing a hardcoded settle-speed threshold, but that speed is bit-exactly linear in dt (speed/dt identical to 17 digits across timesteps), which is converging integrator residual rather than instability. The packet now computes the dt-linearity and refuses to count a dt-linear excess as instability. - In its place CT-002 gains a real oracle: total mechanical energy must not rise once the pile has settled. Sequential impulse gains 1.0e-3 J (2 ms) and 1.3e-3 J (4 ms) per step against a 1.8e-4 J tolerance while boxed LCP stays at or near zero, so the reproduced signal is now specific, small, non-divergent, and attributed to a solver rather than to a threshold. - configuration.resolved_provenance claimed per-cell assertion of four fields while only the contact solver was checked, and the recorded timestep was the first cell's for every cell. Resolved identity is now keyed per cell with contact solver, timestep, and gravity each asserted, in all three packets and on release-6.20. - solver_iterations_by_method() keyed "unsupported" off the method name, so a genuinely recorded count would have been laundered as unsupported; it now keys off the observed value. - CT-002 cited an energy-monotonicity oracle it did not have, and CT-003's envelope maximum was its own seed. Both are now transparent.
Passive 4-link pendulum chain with no contact and no control, released from rest and swept over four timesteps for both multibody integration families. A passive chain conserves total mechanical energy exactly, so the oracle is solver-neutral: relative energy drift versus timestep, summarized by the least-squares log-log slope. Measured: SEMI_IMPLICIT drift falls 5.68e-1 -> 6.47e-2 and VARIATIONAL 8.70e-2 -> 1.05e-2 as dt goes 4 ms -> 0.5 ms, observed slopes 1.04 and 1.02. Both families stay finite and converge, so the convergence half of the row is reproduced. The packet explicitly does not cover the row's 'at matched cost' half: no timing methodology was applied, performance is typed unsupported, and no ranking between the families is claimed. Angular momentum is reported as an observed envelope, not a conservation oracle, because gravity exerts a torque about the world origin.
Final independent review found two majors:
- A genuine gate bypass: a non-string entry in a lane's evidence list was
silently dropped, so evidence: [{"path": ...}] on a closed lane validated
with zero errors, skipping the missing-packet, not-JSON, outside-evidence,
negative-control, and two-review checks. This is the same shape as the
subdirectory bypass an earlier round fixed. Non-string entries now fail.
- CT-004's disposition was reproduced on an oracle unrelated to the cited
claim. The corpus defines reproduced as "the claimed behavior is observed";
CT-004's claim is a methodological prescription about comparison at matched
cost, and this packet measures no cost, so none of its content was observed.
The row is now unresolved, following CT-003, and the convergence result it
did establish is published as a finding that explicitly does not promote it.
Also from that review: raw_paths accepted a directory, negative controls in a
subdirectory were enumerated by neither loop, and scene.digest was
format-checked but never recomputed from the parameters the packet publishes
(a hand-edited scene used to pass). CT-004's relative drift is disclosed as
gauge-dependent, CT-001's claim boundary now leads with its observed outcome,
CT-002 publishes the basis of its energy tolerance, and writers carry forward
recorded review passes instead of erasing them on every regeneration.
Tests grow to 72 cases.
A 4-link chain hanging under gravity tracks a sinusoidal joint reference, swept over four timesteps for both multibody integration families. The fixture needed four iterations, and the reason is worth recording: a fixed-gain diagonal PD saturated its torque limit in every cell and diverged at the coarser timesteps, so it measured the controller rather than the integrator. A direct probe showed why -- the root joint of a serial chain sees a far smaller articulated inertia than the composite rigid-body inertia used to size its gains, so gains stiff enough for the root were unstable while the tip tracked fine. Replaced with a computed-torque controller built on Multibody.compute_inverse_dynamics, which gives every configuration the same closed-loop bandwidth: no saturation, max torque 143.7 Nm, RMS error about 1% of the reference amplitude. The result is substantive. Refining the timestep eightfold does NOT reduce tracking error (2.14e-3 rad at 4 ms versus 2.41e-3 rad at 0.5 ms) while control work rises monotonically, so the error is controller-limited rather than integration-limited and the row's premise -- a whole-step speed/accuracy tradeoff -- is not the simple 'smaller timestep is more accurate' shape. Disposition is unresolved: the cited claim is about a speed/accuracy tradeoff and this packet measures no step cost. constraint_error is typed unsupported because the scene has no constraints to violate.
The docs-policy gate caught the dashboard entry growing to 17 lines against its own 15-line Next step budget, which is exactly the drift that budget exists to prevent: each landed packet had been appended to the operating view instead of to a history owner. Per the dashboard's own rule, the accumulated WS0/WS1/WS2 narrative and the review record now live in a Progress log section of the owner plan, and the dashboard keeps the current action plus a History pointer.
Every packet had been recording resolved solver identity by reading back the option it had just set, which cannot distinguish a method that ran from one the World silently substituted. Three review rounds named this as the gap behind the typed-unsupported markers. World::getResolvedConfiguration() already existed in C++ but was unreachable from Python. This binds ResolvedConfigurationNote and ResolvedSolverConfiguration, adds the World.resolved_configuration property, and threads it through every packet: each run now records, per domain, what was requested, what actually resolved, why, and whether it was a substitution. Verified on a live World -- empty before bake, four domains after, and the recorded rigid-contact resolution follows the requested contact solver. Stub scope: pixi run generate-stubs rewrote 1990 lines across eight files because the committed stubs were already stale relative to the build. That churn is reverted and only the 37 lines this change needs were added by hand; the pre-existing drift is left for its owner rather than swept in. The remaining WS4 gap is a comparable per-solve residual, which still does not exist and stays typed unsupported in every packet.
…lure Establishes the baseline arm the WS5 exact-cone GO/NO-GO needs: a two-box stack at rest, upper mass swept over four decades, both contact solvers, two deterministic repeats per cell. The row stays unresolved -- the cited claim is comparative and no exact-cone arm exists on this branch -- but the baseline is not a null result. SEQUENTIAL_IMPULSE, the World's default contact solver, holds the stack at mass ratios 1 and 10 (relative closure 1.4e-4 and 1.0e-3) and fails completely at 100 and 1000: the heavy box descends 0.20002 m, exactly one box height, while the light box beneath it moves by microns, and the pair comes to rest fully interpenetrated at near-zero velocity. BOXED_LCP holds the same stack across all four decades (3.3e-5 to 3.0e-4). Both outcomes are bit-identical across repeats, and the scene starts with exactly zero overlap, so the closure is produced by the solve. This is consistent with iterative Gauss-Seidel contact under a fixed iteration budget, which is what makes it credible rather than a fixture artifact, but the packet does not claim the mechanism: confirming it needs the per-solve residual WS4 has not exposed. It needs a maintainer decision on the default path and is the strongest WS5 GO input so far. The gate rejected an untyped null for the 'no failure onset' case on the first attempt; that is now a typed unsupported marker.
Working copy of the GitHub issue for the sequential-impulse high-mass-ratio failure, with a standalone repro script (verified against the current build: the heavy box descends to full interpenetration in about one second at ratio 100 while boxed LCP holds the gap at 1e-5 m) and the sweep table from the CT-007 packet. Posting needs maintainer approval; the draft lives in the dev-task folder until that decision is made.
…s covered The RobotDART row's restore-equivalence half, measured with a seven-arm protocol over a contact-rich five-sphere pile, both contact solvers, every continuation hashed bit-exactly over the full state vector, and the whole protocol repeated twice and required identical. Finding: World.state_vector restore is not a function of the restored state once the world has contact history. In-place restores diverge from the original continuation at the FIRST post-restore step and depend on how much history preceded the restore -- two restores of the same snapshot differ from each other. Meanwhile fresh-world restores of the same vector are bit-exact and repeatable, identical histories agree, pre-contact history is harmless, and the ballistic control is exact. Everything is deterministic given full history: this is hidden result-affecting contact state surviving the restore, not nondeterminism. Deactivation was ruled out by probing (nothing asleep; disabling it changes nothing), and update_kinematics() changes nothing. Both solvers are affected identically, pointing at shared contact-pipeline state. This is the design doc's 'reset semantics must explicitly choose whether solver/contact history is preserved' requirement observed unmet, the concrete motivation for WS3 contact ordering/identity, and the second maintainer-decision finding of the campaign (issue draft included alongside the CT-007 one). Disposition stays unresolved: the corpus row is a requirements claim, and reset cost, allocation, and concurrency are typed unsupported. The gate rejected first_divergent_step: 0 until it was declared a measured zero -- which it genuinely is.
…design The WS3 mechanism hunt closed in three probes. At the identical restored state only local_point_a differs between in-place and fresh worlds, and the in-place value is a body-frame anchor of a rotated sphere. World::getStateVector() stores position and linear velocity only per rigid body (the header documents the dense order as 'dynamic rigid-body translations', built for the differentiable path), and the decisive probe shows it directly: 30 entries for the five-sphere scene, and after restoring the same vector the in-place sphere keeps a rolled orientation and ~7 rad/s spin while the fresh sphere sits at identity with zero spin. So the CT-011 divergence is a partial restore by design, not hidden contact state: orientation and angular velocity are never captured, and friction couples the retained rotational state into the continuation. Contact history mattered only because contact is what makes spheres spin. The packet, manifest, verification log, and the posted issue #3443 (body updated with approval, as part of the approved posting workflow) now state the identified mechanism; the earlier contact-pipeline hypothesis is recorded as ruled out rather than silently replaced.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4980a9ee78
ℹ️ 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".
| }, | ||
| "target": { | ||
| "branch": "main", | ||
| "commit": "ca78b0980e88617b98bdbcb84cfeb01931238d05", |
There was a problem hiding this comment.
Record a reproducible commit for each packet
In a clean checkout containing this reviewed commit, ca78b098... is not an ancestor of either c132cb4d... or its parent; the other committed packets similarly reference feature-only SHAs. The recorded command also runs the current checkout rather than checking out this target, and this packet explicitly says its writer exists only in a child commit, so another user cannot reproduce the claimed result at the recorded branch/commit after the feature history disappears. Regenerate the packets against a reachable commit or provide tracked instructions/artifacts that actually combine the target library with the writer.
Useful? React with 👍 / 👎.
| f"{packet_path}: target.branch {branch!r} does not match " | ||
| f"lane {lane_name} branch {expected_branch!r}" | ||
| ) | ||
| if rel in closed_lane_packets: |
There was a problem hiding this comment.
Reconcile closed lanes with packet dispositions
When a lane becomes closed, this block checks only the packet's review count and never compares the manifest lane disposition with packet.result.disposition. For example, a lane marked fixed with a valid referenced packet whose result is unresolved passes validate_tree() with no errors, allowing the manifest to publish a conclusion that its machine evidence does not support. Require the closing evidence to substantiate the lane disposition before accepting closure.
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.
Reject absolute and escaping raw evidence paths
For packets using raw_paths, an absolute path discards both candidate roots, so a host-local entry such as /etc/passwd satisfies this check; ../ paths can likewise escape the intended evidence tree. Such a packet passes locally but its evidence is neither tracked nor portable and will fail or resolve to unrelated contents on another host. Resolve each path and require containment in an approved repository/artifact root.
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: 4980a9ee78
ℹ️ 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 isinstance(review, dict) and isinstance(review.get("passes"), list): | ||
| return {"passes": review["passes"]} |
There was a problem hiding this comment.
Bind preserved reviews to the generated packet
When any writer regenerates an existing packet after its target commit, scene, metrics, or conclusion changes, this unconditionally copies the old review passes into the new output. Because the closure validator only counts those entries, reviews of an earlier packet can satisfy the requirement for reviews of the current post-fix evidence; retain them only when they are bound to an unchanged packet digest, or invalidate them on regeneration.
Useful? React with 👍 / 👎.
| if isinstance(packet.get("review"), dict) | ||
| else None | ||
| ) | ||
| if not isinstance(passes, list) or len(passes) < 2: |
There was a problem hiding this comment.
Require independent reviewers before closing a lane
For a closed lane, two identical pass entries from the same reviewer satisfy this length check even though the task requires two independent or role-separated reviews. This lets a duplicated review record close and publish a lane without the required second review; validate distinct reviewer identities or explicitly distinct roles rather than only the list length.
Useful? React with 👍 / 👎.
| "both contact solvers. Established: restore is NOT a " | ||
| "function of the restored state once the world has contact " | ||
| "history -- the in-place continuation differs from the " | ||
| "original at the first step, and two in-place restores of " | ||
| "the same snapshot differ from each other when different " |
There was a problem hiding this comment.
Generate the CT-011 boundary from current measurements
When this writer is rerun after restore behavior changes, the packet still unconditionally claims that restore is not a function of restored state and describes specific failing arms, even if the newly computed findings report exact restoration. Since regeneration updates the target commit and raw measurements without failing validation, it can publish a directly contradictory conclusion; derive this boundary from the current findings or abort when the expected result changes.
Useful? React with 👍 / 👎.
| restore_is_state_function = all( | ||
| row["findings"]["inplace_restore_matches_continuation"] | ||
| and row["findings"]["inplace_restores_match_each_other"] | ||
| for row in rows |
There was a problem hiding this comment.
Include every restore arm in the state-function verdict
If both in-place checks pass but a fresh-world or pre-contact-history restore still diverges, this aggregate becomes true even though worlds with the same restored vector have different continuations. That is precisely a counterexample to the stated “function of restored state alone” property; include the cross-history/fresh-world comparisons in the aggregate instead of considering only the two in-place booleans.
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: cebf5a8679
ℹ️ 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 requested. Shepherd both PRs through CI and review (re-request | ||
| '@codex review' after every push, iterate until clean). After merge, the |
There was a problem hiding this comment.
Require approval before every Codex re-trigger
When an agent follows this handoff after a later push, the instruction to re-request @codex review unconditionally causes another PR comment without confirming that the push addressed Codex feedback or obtaining explicit approval for that mutation. Limit the instruction to an approved post-fix review round; otherwise it violates the repository's external-mutation policy and can trigger the documented review throttling.
AGENTS.md reference: AGENTS.md:L84-L86
Useful? React with 👍 / 👎.
| 4. Start WS4's first slice in parallel where it unblocks packets: expose | ||
| `ResolvedSolverConfiguration` to Python and a comparable per-solve | ||
| residual, both currently typed unsupported in every packet. |
There was a problem hiding this comment.
Remove the already-landed binding from the next steps
A resumed agent is told to expose ResolvedSolverConfiguration to Python even though this same commit already adds the classes and World.resolved_configuration binding in module_compute.cpp and module_world.cpp. The handoff also incorrectly says the binding remains typed unsupported, so following it can duplicate or churn completed API work; mark the exposure complete and leave only the still-missing per-solve residual as WS4 work.
AGENTS.md reference: docs/AGENTS.md:L39-L40
Useful? React with 👍 / 👎.
| elif not (isinstance(visual, list) and visual): | ||
| errors.append( | ||
| "evidence.visual must list visual artifacts or be typed " | ||
| "not-applicable with a reason" |
There was a problem hiding this comment.
Validate each listed visual artifact
For a packet whose claim requires visual evidence, any non-empty list—including [null] or ['/missing/frame.png']—passes this branch because neither entry types nor artifact resolution are checked. Such a packet can therefore satisfy the fail-closed validator and later close a lane without the tracked, source-bound visual evidence required by the evidence contract; validate every list entry and require it to resolve to an approved artifact or supported structured reference.
Useful? React with 👍 / 👎.
|
|
||
| def _load_json(path: Path, errors: list[str]) -> object | None: | ||
| try: | ||
| return json.loads(path.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
Reject non-standard constants in all packet fields
Python's default json.loads accepts NaN, Infinity, and -Infinity, while the later finite-number checks cover only metric leaves. A packet containing one of these constants in evidence.raw_rows, host, or another unvalidated nested field is reported as valid even though strict JSON consumers cannot parse the supposedly machine-readable packet. Configure parse_constant to reject these values at load time so the whole artifact remains portable JSON.
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: cebf5a8679
ℹ️ 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".
| 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.
Reject vacuous inline raw evidence
When a packet uses inline evidence, any non-empty list is accepted, so raw_rows: [null] passes packet_errors() despite containing no trajectory or metric data. Such a packet can subsequently satisfy the evidence requirement for a closed lane with no inspectable raw evidence; validate that every row is a non-empty structured record with the required data shape.
Useful? React with 👍 / 👎.
| has_repeats = ( | ||
| isinstance(repeats, int) and not isinstance(repeats, bool) and repeats >= 2 | ||
| ) | ||
| has_sweep = isinstance(sweep, list) and len(sweep) >= 2 | ||
| has_seeds = isinstance(seeds, list) and len(seeds) >= 2 |
There was a problem hiding this comment.
Require valid distinct ensemble entries
When an ensemble relies on sweep or seeds, only list length is checked, so values such as seeds: [null, null], duplicate seeds, or two empty sweep points are accepted as multi-run evidence. This lets a packet pass the explicit single-run guard without defining two meaningful ensemble members; require correctly typed, distinct seeds or valid distinct sweep points.
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 metric leaves
When a measured value is replaced by arbitrary prose, the validator rejects only a small sentinel vocabulary and otherwise lets the value fall through; for example, max_penetration_m: "not measured yet" passes with no errors. This permits missing measurements to masquerade as measured fields and close a lane, so measurement leaves need an allowed finite numeric/boolean type or an explicit typed-unsupported marker, with prose metadata confined to known metadata fields.
Useful? React with 👍 / 👎.
One genuine documentation bug fixed: the dashboard overclaimed that all six first-wave families have a main packet -- CT-006 heel-strike has none (audit-required, gated on WS3); corrected to five of six. Validator: allocation measurements also require a performance-valid host; sweep points may not declare null coordinates (absent row fields would spuriously match them); seed fields match by whole token; the build step must PRECEDE the evidence command. Tests: 143 -> 147 validator cases.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9086f3ede0
ℹ️ 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 len(head) <= header_start + header_len: | ||
| # Header-only file: a declared array with no payload bytes. | ||
| return mismatch | ||
| return None |
There was a problem hiding this comment.
Verify NPY payload size against the declared array
Fresh evidence after adding the structural NPY parser is that a 75-byte file declaring shape: (1000000,) and descr: '<f8' passes when it contains only one payload byte, because this check merely requires some byte after the header. Once its artifact digest is recorded, the complete packet validator accepts this truncated, undecodable artifact as raw evidence; compute the expected payload size from the shape and dtype and reject truncated or inconsistent files.
Useful? React with 👍 / 👎.
| if has_repeats and not _has_hash_leaf(packet.get("evidence")): | ||
| errors.append( | ||
| "ensemble.deterministic_repeats is asserted but the " | ||
| "evidence carries no *hash*/sha256 field binding the " | ||
| "repeats to recorded trajectories; an unverifiable repeat " | ||
| "claim is not an ensemble" | ||
| ) | ||
| has_repeats = False |
There was a problem hiding this comment.
Bind two-repeat claims to repeat-level hashes
Fresh evidence after the hash-format hardening is that removing the trajectory hash from raw_rows and adding an unrelated valid value under evidence.artifact_digests still makes the complete two-repeat packet pass. _has_hash_leaf therefore treats any artifact digest as proof of deterministic repeats, even when no repeated run is represented; require two repeat-level trajectory digests and verify that they agree.
Useful? React with 👍 / 👎.
| if not any( | ||
| (_is_finite_number(leaf) or isinstance(leaf, bool)) | ||
| and re.sub(r"(\[\d+\])+$", "", path.rsplit(".", 1)[-1]) | ||
| not in NUMERIC_BOOKKEEPING_KEYS | ||
| for path, leaf in _metric_leaves(row) |
There was a problem hiding this comment.
Normalize bookkeeping keys before counting measurements
Fresh evidence after excluding lowercase seed is that a complete packet still passes with a raw row whose only numeric leaf is {"Seed": 1} (or {"run_id": 1}), because the bookkeeping comparison is case-sensitive and recognizes only exact terminal names. These remain metadata-only records but can satisfy the raw-measurement gate and support lane closure; normalize key case and classify common tokenized identifier fields before counting measurement leaves.
Useful? React with 👍 / 👎.
One genuine false positive fixed: env-prefixed build commands (DART_PARALLEL_JOBS=4 pixi run build) passed COMMAND_RE but were not recognized as the build step, rejecting valid evidence -- fixed with a prefix-aware BUILD_COMMAND_RE and a regression test. NPY payloads are verified against the declared array size; artifact digests no longer count as repeat verification (repeat claims bind to trajectory hashes in rows or the ensemble; digests prove file identity only); bookkeeping keys are matched case-insensitively by whole token. Tests: 147 -> 151 validator cases.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ee6d30aeb
ℹ️ 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_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.
Match only the registered build task
When validating a packet from a clean checkout, this word-boundary match treats invented task names such as pixi run build-not-a-task as the required build step. I verified that substituting this command into CT-001 leaves packet_errors() with no non-review error, even though the repository defines no such task, so a lane can pass the check-lint evidence gate with a reproduction sequence that never builds its artifacts. Require the task token to end or be followed by whitespace rather than accepting any build-* entry point.
AGENTS.md reference: AGENTS.md:L87-L87
Useful? React with 👍 / 👎.
A commands list consisting only of build steps is rejected -- the reproduction sequence must also RUN the evidence command; the build match requires the exact task name (arguments allowed, invented suffixed tasks like build-nothing not); zero-width NPY dtypes are rejected; numerically equivalent sweep points (1 vs 1.0) deduplicate in the distinct-point check, matching the row matcher's equality semantics. Tests: 151 -> 155 validator cases.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92d976e8b4
ℹ️ 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".
| max_iterations = 0 | ||
| max_residual = 0.0 | ||
| contact_count_max = 0 | ||
| previous_energy = None |
There was a problem hiding this comment.
Include the initial energy in the injection check
When the first simulation step injects kinetic energy, previous_energy is still None, so the increase from the configured initial state to the first post-contact state is never included in max_energy_gain_j. Because the sphere starts touching the ground and this value gates the packet's physical validity, an erroneous first contact impulse can be missed while the packet still reports reproduced; initialize the baseline from world.compute_step_metrics().kinetic_energy before stepping.
Useful? React with 👍 / 👎.
| if key.endswith(IDENTITY_METADATA_SUFFIXES): | ||
| return False | ||
| tokens = re.split(r"[^a-zA-Z0-9]+", key.lower()) |
There was a problem hiding this comment.
Normalize identity keys before filtering metadata suffixes
When configuration keys use mixed case, the metadata-suffix exclusion runs before lowercasing, so objects such as {"Method_Note": "arbitrary prose"} or {"Backend_Reason": "arbitrary prose"} are accepted as solver identities. Such metadata-only requested and resolved objects can therefore pass packet_errors() and eventually back a closed lane; lowercase the key before both the suffix and token checks.
Useful? React with 👍 / 👎.
One genuine parity gap fixed: main's CT-001 energy-injection gate started from previous_energy = None, skipping the first contact solve -- the fix its release-6.20 twin received in round 2 was never mirrored; now seeded from the configured pre-step state, packet regenerated in the follow-up commit. Also: identity metadata suffixes excluded case-insensitively; configurations must name a solver/method/integrator identity (a 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: 155 -> 159 validator cases.
The first contact solve is now inside the gate; every cell still passes physical validity and the disposition is unchanged (reproduced).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 009e5072c8
ℹ️ 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".
| commit = target.get("commit") | ||
| if not (isinstance(commit, str) and COMMIT_RE.match(commit)): | ||
| errors.append("target.commit must be a 40-hex commit hash") |
There was a problem hiding this comment.
Verify target commits are actually reachable
This check validates only the hash's shape and that the same text appears in fetch_hint; a packet using a nonexistent 40-hex commit passes even though the recorded checkout command cannot reproduce it. Because --freshness is optional and not part of check-lint, a hand-edited packet can still close a lane while attributing its measurements to no source revision; verify that the commit exists and is reachable through the promised PR ref.
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.
Require the recorded evidence harness to exist
When the command contains any text matching python ... scripts/, this branch considers the evidence executed without resolving the script path. For example, pixi run python scripts/does_not_exist.py passes the command checks after pixi run build, so a packet can advertise a reproduction sequence that always fails while still satisfying the lane gate; parse the command and require the referenced repository harness to be an existing file.
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.
Inspect NPY payload values before accepting evidence
Fresh evidence after the structural and payload-size fixes is that a fully sized NPY array containing only NaN values still returns no content issue: I verified this with a valid one-element <f8 NPY payload. Once its artifact digest is recorded, this lets a packet satisfy the raw-measurement gate with no finite observation, unlike inline rows and CSV cells; decode or scan the bounded payload and require at least one finite numeric or boolean value.
Useful? React with 👍 / 👎.
Target commits must exist in the repository object store (CI lint now fetches 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: 159 -> 169.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5fe2472e8
ℹ️ 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".
| url = source.get("url") | ||
| if not (_is_nonempty_str(url) and re.match(r"^https?://\S+$", url.strip())): | ||
| errors.append( | ||
| "source.url must be a retrievable http(s) URL; a placeholder " | ||
| "does not bind the claim to its source" | ||
| ) | ||
| if not _is_nonempty_str(source.get("claim")): | ||
| errors.append("source.claim must be a non-empty string") |
There was a problem hiding this comment.
Bind packet sources to the manifest claim
When a packet is attached to a valid claim ID, these checks accept any non-empty claim text and syntactically HTTP(S) URL without comparing them with the manifest or corpus. I verified that replacing CT-001's source with https://example.com/unrelated and an unrelated assertion, then recomputing its review digests, produces no packet validation errors. A lane can therefore close with evidence for a different cited assertion; validate the packet source and claim against canonical source data owned by the manifest or corpus.
Useful? React with 👍 / 👎.
| with: | ||
| # check-citation-evidence verifies every packet's target commit | ||
| # against the local object store; a shallow clone cannot answer | ||
| # whether a historical commit exists. | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
Fetch packet target refs before checking their objects
On the push-to-main path in this workflow, after a squash merge and deletion of the topic branch, fetch-depth: 0 fetches branch history but not the surviving refs/pull/3445/head namespace named by the packets' fetch hints. The pre-squash target commits can therefore be absent locally, causing _git_object_status() to report missing and making the newly required check-lint fail on main; explicitly fetch the recorded PR ref before validation or otherwise make those target objects available.
Useful? React with 👍 / 👎.
Packets are source-bound to the manifest (claims with lane evidence pin canonical source_url/source_claim and packets must match), --fetch-target-refs restores squash-merged target commits before the CI object-store check, fetch hints may name any evidence PR, NPY dtype widths are whitelisted per kind, and the harness command must invoke an evidence writer. Tests: 169 -> 175.
|
@codex review |
| "Bounded reconstruction: the original SimBenchmark elastic " | ||
| "test assets and parameters are not reproduced exactly.", |
| "The envelope uses aggregate world energy; per-body " | ||
| "restitution-outcome tracking (bounce-height ratios) is " | ||
| "future work for this row.", |
| "The envelope maximum is set by the initial state in every " | ||
| "cell (energy never rose above its starting value), so the " | ||
| "1.8e-4 J tolerance is never exercised; the test is " | ||
| "one-sided by construction and is reported as such in " | ||
| "raw_rows.envelope_set_by_initial_state.", |
| "The cited claim also names solver failure. That limb is " | ||
| "only instrumented as non-finite state: no residual exists " | ||
| "on this path and the boxed-LCP branch records no iteration " | ||
| "count, so a non-diverging solver failure would not be " | ||
| "detected here.", |
| "max_penetration_m is bit-identical across the two solvers " | ||
| "at each timestep while the trajectories diverge, so it is " | ||
| "set during the shared first impact and carries no " | ||
| "solver-discriminating information.", |
| "max_active_contacts is 216 in every cell (a fully " | ||
| "stacked column geometry), so it carries no " | ||
| "discriminating information here.", |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3e987cf08
ℹ️ 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( | ||
| (_is_finite_number(leaf) or isinstance(leaf, bool)) | ||
| and not _is_bookkeeping_key( | ||
| re.sub(r"(\[\d+\])+$", "", path.rsplit(".", 1)[-1]) | ||
| ) | ||
| for path, leaf in _metric_leaves(row) |
There was a problem hiding this comment.
Exclude sweep coordinates from outcome measurements
When a sweep coordinate is numeric, this predicate counts that coordinate itself as the row's required measurement. For example, CT-004's rows can be reduced to integration_family, timestep_s, and the existing hash fields and still pass packet_errors(), even though they contain no energy or momentum outcome for any point. This allows aggregate metrics disconnected from the declared cells to support a lane, so exclude sweep/seed coordinate fields when requiring each row to contain a measured outcome.
Useful? React with 👍 / 👎.
| trajectory.update(np.array([total_energy]).tobytes()) | ||
| trajectory.update(angular_momentum.tobytes()) |
There was a problem hiding this comment.
Hash the articulated state for repeat exactness
When repeated articulated runs differ in individual joint positions or velocities but retain the same aggregate total energy and angular momentum, these updates produce identical trajectory hashes because those two aggregates do not uniquely identify a multi-link state. The writer then sets deterministic_repeats_identical and describes the repeats as bit-identical despite divergent trajectories; hash every joint's position and velocity at each step instead.
Useful? React with 👍 / 👎.
Sweep coordinates are excluded from row-measurement checks, 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, NPY format versions are whitelisted, sweep points canonicalize recursively, and the CT-004 hash covers the full articulated state (CT-004/CT-011 regenerated in the follow-up commit). Tests: 175 -> 181.
CT-004's trajectory digest now covers every joint position and velocity per step; CT-011 rows add the trajectory_sha256 chain over their arm state digests. Dispositions unchanged (unresolved).
|
@codex review |
| "Bounded reconstruction: the original SimBenchmark asset, " | ||
| "material, and timestep grid are not reproduced exactly; " | ||
| "sourcing the exact historical setup is future corpus work.", |
| "The reproduced signal is a small per-step energy gain in a " | ||
| "settled pile, not divergence or failure. It must not be " | ||
| "quoted as 'dense contact fails' or as a solver ranking.", |
| "The settle window is the second half of the run; a pile " | ||
| "that settles later would put free-fall discretization error " | ||
| "inside the window and inflate the energy metric.", |
| "Resolved identity comes from the World's own bake-time " | ||
| "resolution, with contact solver, timestep, and gravity " | ||
| "each additionally asserted against the request per cell.", |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 933433af2a
ℹ️ 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( | ||
| (_is_finite_number(leaf) or isinstance(leaf, bool)) | ||
| and not _is_bookkeeping_key(_terminal(path)) | ||
| and not _is_seed_key(_terminal(path)) | ||
| and _terminal(path) not in coordinate_keys | ||
| for path, leaf in _metric_leaves(row) |
There was a problem hiding this comment.
Exclude nested sweep coordinates from measurements
When sweep coordinates are nested, coordinate_keys contains only the outer key while _metric_leaves() reports the inner terminal key. For example, sweep points {"config":{"dt":1}} and {"config":{"dt":2}} with corresponding raw rows containing only those objects satisfy both coverage and this measurement predicate, even though neither row records an outcome. Recursively collect coordinate leaf paths, rather than only top-level keys, before deciding that each row contains a measurement.
Useful? React with 👍 / 👎.
| elif not any( | ||
| 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.
Require the writer to be the executed script
When an unrelated Python script receives the expected writer path as an argument, these unanchored searches treat that argument as the executed harness. For example, after the build, pixi run python scripts/check_citation_evidence.py scripts/write_citation_ct001_rolling_direction_packet.py passes the command checks because both referenced files exist and the writer name appears after python, although the command only invokes the checker and fails on the extra argument. Parse the Python command and require the executable script operand itself to be this claim's writer.
Useful? React with 👍 / 👎.
| if not ( | ||
| isinstance(header, dict) | ||
| and {"descr", "shape", "fortran_order"} <= set(header) |
There was a problem hiding this comment.
Validate the complete NPY header
When an NPY header has fortran_order set to a non-boolean value or contains extra keys, this subset check still accepts it if the shape, dtype, and payload otherwise look valid. NumPy rejects such files (fortran_order is not a valid bool or Header does not contain the correct keys), so an undecodable artifact can still satisfy the raw-evidence gate. Require exactly the standard header keys and a boolean fortran_order value.
Useful? React with 👍 / 👎.
| and ( | ||
| not row["finite"] | ||
| or (row["relative_gap_closure"] or 0.0) > collapse_threshold | ||
| # Whole-stack fall-through keeps the gap closure near zero while | ||
| # both boxes leave the ground; the sink of the LOWER box catches it. | ||
| or (row["lower_sink_m"] or 0.0) > half |
There was a problem hiding this comment.
Reject ejected boxes as failed stack cells
When the upper box is ejected upward or sideways while the lower box remains near its rest height, the state is finite, relative_gap_closure is non-positive or near zero, and lower_sink_m stays small, so this predicate classifies the cell as holding. The resulting baseline can therefore publish holds_across_swept_range even though the boxes are no longer stacked; check the full relative placement (including excessive separation and lateral displacement) and a settling-speed bound.
Useful? React with 👍 / 👎.
Summary
a fail-closed
pixi run check-citation-evidencegate (wired intocheck-lint), first evidence packets for all six capped first-wavefixture families, and Python bindings for the World's bake-time
ResolvedSolverConfigurationso packets record the method that actuallyran.
SEQUENTIAL_IMPULSEcontactsolver lets a heavy box sink fully through a light one at mass ratios of
100 and above (CT-007), and
World.state_vectorrestore is not a functionof the restored state (CT-011; root-caused during this work: the vector is
translational by design and omits orientation and angular velocity).
Motivation / Problem
criticism, exact-cone papers, Nimble/RobotDART workflow needs) had no
branch-qualified, reproducible dispositions: no owner converted them into
stable rows with source-bound evidence, and benchmarks could name a
requested method without proving it ran.
with its durable design doc docs/design/contact_trust_and_observability.md)
makes every such claim a stable row with a fail-closed evidence packet.
Changes / Key Changes
claims-manifest.json(20 corpus rows, exact-IDagreement with the human corpus enforced), packet schema
dart.citation_claim_evidence/v1, validatorscripts/check_citation_evidence.pyincheck-lint, permanentintentionally incomplete negative control that must keep failing, 85
pytest cases. The gate rejects prose/non-JSON/outside-
evidence//negative-control/non-string lane references, dangling or directory
raw_paths, scene digests that disagree with the published parameters,single-run ensembles, spelled placeholders, and any exact zero not
declared a measurement or typed
unsupportedwith a reason.scripts/, packets under the plan'sevidence/): CT-001 rolling-direction (reproduced: friction-pyramidantisymmetry signature), CT-002 dense inelastic (
reproduced: settled-pileenergy gain under sequential impulse only), CT-003 dense elastic
(
unresolved: no energy injection observed), CT-004 articulated energydrift (
unresolved: converges, but the cited claim is matched-cost),CT-005 PD tracking (
unresolved: controller-limited error), CT-007 highmass-ratio (
unresolvedfor the comparative claim; baseline findingabove), CT-011 restore equivalence (
unresolved; finding above, with the rootcause identified in the packet).
ResolvedConfigurationNote/ResolvedSolverConfiguration, theWorld.resolved_configurationproperty, surgical stub entries, and two Python tests; every packet
records per-domain requested/resolved/reason/substitution from the World
itself.
audit; dev task holds the working state, verification log (three
independent review rounds and their fixes), and the two issue drafts.
Testing
pixi run check-lint(includes the newcheck-citation-evidencegate) andpixi run test-all— all phases pass on the branch head.pixi run python -I scripts/run_pytest.py tests/test_check_citation_evidence.py -q— 85 passed.bit-identical; dispositions computed from recorded data with negative
controls and physical-validity gates.
in-branch and recorded in the dev-task verification log.
Breaking Changes
Related Issues / PRs (backports)
feature/dart6-citation-contact-trust(independent; adopts the same contract without DART 7 APIs).
(CT-011 restore-history dependence); both packets and issue sources are in
this PR.
Checklist
World.resolved_configuration)