feat(research): add CALF/LegNav comparator diagnostic - #7357
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Contract Check SummaryOverall Status: 🟢 PASSED
ℹ️ Info
This check is mechanized. Please resolve any blockers to pass CI. |
Maintainer Decision RequiredWhat is complete:
What is missing for merge:
|
Exact-Head Review Verdict
Findings Disposition
Validation Executed (exact head)
base-policy: ordinary-cas @ 28e5214 |
Exact-head self-review — PR #7357
Local proof
Gate disposition
|
ll7
left a comment
There was a problem hiding this comment.
Exact-head CALF comparator review — PR #7357
- Reviewed head:
c680b6811b6b8d1b8f750ec9c1892c7d433d9901. - Reviewed base:
462032df2abc3e086655935288c806b9df8bda2b(origin/main). - Rebased/merged cleanly onto current
main. - Local validation: 4 tests passed in
tests/dev/test_issue_7331_benchmark_namespace_inventory.py. Ruff check and format passed.git diff --checkpassed. - Domain-aware approval: pending maintainer decision.
- Reconciled PR metadata:
pr-metadata: reconciled @ afe9c4c61d65be7f42e06ce3f607e8ad0dc1dba4bb78716ce6830073e68b9fc4.
gate-verdict: accepted @ c680b6811b6b8d1b8f750ec9c1892c7d433d9901
merge-ready: no (domain-aware approval pending)
Domain-aware review — paired observation-contract validityReviewed head: Not author-reservedThe diff adds a comparator module, two schemas, a runner, a frozen smoke config, a docs page, and Pairing verified empirically, not from the bodyLive paired smoke on the committed config, both rows executed, no fallback/degraded execution:
Same candidate One fail-open found (tracked: #7454)
A validated fix exists on branch Why this is parkedThe branch received a base refresh from the autonomous factory a few minutes before this review, and RecommendationMerge Hosted CI at this head was still fully pending when this review was written. |
|
Self-review disposition (2026-08-18; not an approval): current head The comparator is explicitly diagnostic-only and its body correctly keeps domain-aware approval pending before promotion. The implementation/schema/tests may proceed through independent exact-head review, but no claim-eligible comparison, campaign, evidence admission, publication, compute, scheduler, or issue-state mutation is authorized by this PR. |
|
#7357: approve-diagnostic-contract-after-fail-closed-binding Approve the paired observation-contract direction and its narrow claim boundary: one Robot SF candidate, scenario, and seed may be compared under the fixture's ideal-state and bounded perception-limited observations as a diagnostic plumbing smoke. It is not a CALF reproduction, sensor-realism result, transfer result, safety result, benchmark ranking, or paper-facing finding. The committed smoke's observed pair is valid, but two enforcement conditions remain before this head can receive Domain-Aware Approval. First, land the prepared #7454 hardening: bind each fixed slot to its derived observation condition, block swapped/degenerate pairs, and include Automatic consequence: remove Decision evaluated at head |
PR reconciliation — comparator contract incompleteThis PR must not merge in its current form. Issue #7454 identified a fail-open: the report does not bind each condition slot to the observed evidence class and does not include Keep in draft until:
The one-seed fixture remains diagnostic and cannot support transfer or policy-ranking claims. |
…7508) (#7519) ## Summary Make `scripts/dev/pr_loop_policy.py` machine-recognize the two workflow markers that PR #7500 introduced to `goal-pr-review` (docs-only): the advisory `review-claim: <lane> @ <head> until <UTC>` comment marker and the `author_decision` parking state (`decision-required` label + `### Decision packet` comment at the live head). PRs parked by either are classified `active_writer` / `author_decision` with recommended action `no_action`/stop, distinct from `blocked_preflight`, so reviewer lanes and the factory stop racing on the same branch. ## Linked Issues - Closes `#7508` - Refs `#7500` (defines the marker formats), `#7491` (not-ready-sentinel half, intentionally NOT implemented here) ## Stack / Dependency - Base dependency: none - Required prior PRs: #7500 (merged, defines the marker contract) - Stack follow-up issues: none - Safe to review independently: yes - Review dependency reason, if any: the marker format is defined by #7500's docs. ## What Changed - `scripts/dev/pr_loop_policy.py`: - `VALID_STATES` gains `active_writer` and `author_decision`. - New marker parsing: `_REVIEW_CLAIM_RE` (`review-claim: <lane> @ <sha> until <UTC>`), `_REVIEW_CLAIM_RELEASED_RE` (`review-claim: released @ <sha>`), `_DECISION_PACKET_HEADING_RE` (`### Decision packet`), plus a `ReviewClaim` dataclass. - New pure helpers: `_parse_review_claim_marker`, `_review_claim_released_shas`, `_has_decision_packet_heading`, `_trusted_marker_comments` (OWNER/MEMBER/COLLABORATOR only, reading both `comments` and `reviews` bodies), `active_review_claim` (unexpired + unreleased + head-bound, `now=None` -> `datetime.now(UTC)`), `has_author_decision_packet` (live-head binding; review-body packets must name the live head). - `classify_pr_state` takes an explicit `now` kwarg; draft/error/closed still return `no_action` first; `active_writer`/`author_decision` are checked before `blocked_preflight` and the rest of the pipeline (precedence per issue). - `recommend_action` maps both new states to `no_action` with named reasons; flow decision is `stop` for both. - Module docstring documents the lane-identity limitation (snapshot cannot distinguish lanes, so any unexpired unreleased trusted marker parks) and links **#7491 as explicitly out of scope** — no not-ready-sentinel implementation was added. - `tests/dev/test_pr_loop_policy.py`: 24 new tests with fixed timestamps (no monkeypatching): park on unexpired trusted claim, untrusted does not park, released clears, expiry, `now == until` is expiry, head mismatch, unparseable timestamp fails closed, claim in review body, parser round-trips, author_decision with/without label and packet, live vs stale head, untrusted packet, precedence over `blocked_preflight` and over ready-to-merge, draft still `no_action`, VALID_STATES contract, queue-level evaluation. - `CHANGELOG.md`: Unreleased -> Changed entry. ## Why It Matters - Added value: the loop tooling now parks PRs with an active review-claim or author-reserved decision instead of racing the branch, which was the observed 2026-08-18 failure mode (6 PRs had exact-head evidence voided by mid-run factory rebases; #7357 and #7347 raced within minutes). - Expected impact: workflow reliability only; no benchmark/planner/evidence semantics change. - Why this is worth merging now: closes the machine-recognition gap left by #7500 so the advisory markers become enforceable. ## Research Result Guidance - Target claim / hypothesis / blocker this should affect: NA - support/tooling-only; no research claim. - Comparator or baseline, if applicable: NA - Evidence tier: NA - support helper. - Result classification: NA - Decision or stop rule, if applicable: NA - Parent issue, claim map, registry, context note, or synthesis surface to update: NA - New research/benchmark/metric/paper-facing analysis tool, if any: NA - support helper. ## Domain-Aware Approval - Required for this PR: no - workflow/tooling classification change; no evidence classification, comparison methodology, figure eligibility, benchmark interpretation, or paper-facing claim change. - Domains reviewed: NA - Status: not required - Approver/review source or waiver: NA - Validity checklist (machine-detected labels unchanged): - Target claim/hypothesis: NA - Comparator or split/evidence validity: NA - Fallback/degraded exclusions: NA - Claim boundary: NA - Implementation integrity vs experimental validity: implementation integrity only. ## Falsification / Non-Transfer Check - Did the mechanism activate? NA - Did the intervention change command source, selected command, trajectory, or route progress? NA - Did the scenario actually contain the targeted failure mode? NA - Result route: NA - Follow-up question or issue for weak, negative, or non-transfer results: NA ## Next Empirical Action - Rerun needed: no - Extractor or analysis tool needed: no - Artifact missing or unavailable: none - Stop / revise / continue decision: NA - Proposed child issue or existing follow-up: none ## Validation / Proof - Commands run (worktree root): - `uv run pytest tests/dev/test_pr_loop_policy.py -q` — 174 passed (150 pre-existing + 24 new). - `uv run pytest tests/dev/test_snapshot_pr_queue.py -q` — 55 passed (companion). - `uv run ruff check scripts/dev/pr_loop_policy.py tests/dev/test_pr_loop_policy.py` — clean. - `uv run ruff format --check scripts/dev/pr_loop_policy.py tests/dev/test_pr_loop_policy.py` — clean. - Smoke: real `snapshot_pr_queue --active | pr_loop_policy --stdin --json` — exit 0, no crash; synthetic snapshot emits `active_writer` and `author_decision` states via the CLI. - Evidence that the change works here: 24 focused tests cover the marker grammar, expiry/release semantics, live-head binding, precedence, and queue-level behavior with fixed timestamps. - Benchmarks or smoke tests, if applicable: none (workflow tooling). ## Performance Evidence - (Not a `perf` change; section deleted per template guidance.) ## Risks / Rollout - Compatibility risks: additive states and an explicit `now` kwarg (default preserves current behavior); existing classifications re-tested (draft still no_action, ready_to_merge precedence preserved). - Failure modes: unparseable claim timestamps fail closed (documented); lane identity is not distinguishable in the snapshot, so any unexpired unreleased trusted marker parks (documented limitation). - Rollback or fallback plan: revert the single commit; the new states are purely additive. ## Docs / Provenance - Updated docs: module docstring in `pr_loop_policy.py` documents the marker grammar, precedence, lane-identity limitation, and the #7491 out-of-scope boundary; CHANGELOG entry. - Relevant design or provenance notes: marker grammar comes from the merged #7500 goal-pr-review docs. - Any assumptions that need to be preserved: the #7491 not-ready-sentinel half must remain tracked there, not duplicated here. ## Downstream Propagation - Parent issue updated (yes/no/NA): yes - issue #7508 will be closed by this PR. - Claim map / benchmark report updated (yes/no/NA): NA - Leaderboard / artifact catalog updated (yes/no/NA): NA - Registry or config index updated (yes/no/NA): NA - Context index / memory note updated (yes/no/NA): NA - Follow-up issue opened for deferred propagation (yes/no/NA): no - Not applicable because: workflow/tooling-only change with no research-facing downstream surface. ## Follow-Up Issues - Deferred work: the #7491 not-ready-sentinel machine check (tracked there). - Issues opened for follow-up: none. ## Reviewer Notes - Anything a reviewer should verify closely: the marker regex grammar matches the #7500 docs exactly (`review-claim: <lane-id> @ <head-sha> until <UTC>`); precedence of `active_writer`/`author_decision` over `blocked_preflight`; the live-head binding for author_decision packets. - Any known limitations: snapshot does not carry lane identity, so the active-writer check cannot exclude the claiming lane itself; documented in the module docstring. - Shared-helper migration: NA - single-file CLI helper change.
|
review-claim: goal-autopilot-7454 @ c680b68 until 2026-08-18T16:17:00Z I am taking the bounded #7454 landing/review lane for PR #7357. The intended action is to refresh |
ll7
left a comment
There was a problem hiding this comment.
Exact-head review after landing the prepared #7454 hardening.
- reviewed head:
ec2f61aca6db146345471537663a146d2d4202fb - reviewed base:
a2d9279cd9da9740a7011f25ebe96860d95617dc - intended design: keep the CALF/LegNav comparator diagnostic-only while making slot-to-observed-
condition binding and paired horizon identity fail closed - changed landing: prepared commit
5949b7024fbddfc1e021bc49db5f87e0e2132be9, plus current-main
namespace lock refresh from 296 to 297 direct children - validation: 28 focused comparator/namespace/diagnostic tests passed; Ruff check/format and
git diff --checkpassed; docs/proof consistency passed for 12 changed files; committed smoke
returnedstatus: available, both bindingsmatched,runner_error_count: 0, and
evidence_status: diagnostic-only - findings: the prepared fail-closed binding and horizon checks are present; no new actionable
implementation defect was found - domain-aware approval: still pending, as required by the maintainer ruling; the smoke remains
diagnostic plumbing only and does not establish a CALF reproduction, transfer, safety, ranking,
or paper-facing result - readiness boundary: repository readiness recognizes the domain section as structurally complete
but remains blocked until status isapprovedorwaived; nomerge-readylabel is applied - merge boundary: hosted CI, authorized domain decision, independent review, maintainer acceptance,
and guarded merge preflight remain outstanding
gate-verdict: blocked @ ec2f61a
pr-metadata: reconciled @ 10dfe4c44f0f7e175fd7b58b1595dc41f0a189a32e08c3f13725bc6b103a3e91
ll7
left a comment
There was a problem hiding this comment.
Exact-head research review — PR #7357
Reviewed draft PR #7357 at exact head 441534346c5a4f81826267af10d3a6ef88588590 against current
origin/main 51df8965b0ceff300e15f88fc248cd3217e23ff1.
The current-main refresh exposed and repaired one real integration gap: the new direct benchmark
module was missing from the fail-closed namespace classification manifest. The effective diff now
contains that registration alongside the comparator, its schema/config, the observation-contract
adapter, docs, and focused tests.
Local exact-head proof:
- Comparator, diagnostic-runner, and namespace tests: 31 passed.
- Focused Ruff check, format check, Python compilation, and
git diff --check: passed. - Config-first dry run: passed.
- Actual two-condition smoke:
status=available,evidence_status=diagnostic-only, zero runner
errors; one paired episode remains insufficient for uncertainty or a research conclusion.
The implementation remains explicit that it does not execute CALF/LegNav or establish sensor,
embodiment, safety, transfer, benchmark, or paper-facing validity. This is a comment-only
exact-head self-audit under the documented single-account waiver; it is not an approval. Domain
review, draft/dependency state, and hosted checks remain open.
base-policy: ordinary-cas @ 4415343
pr-metadata: reconciled @ 65a747e9f65baf81b1c265d8ad02759505f701164aa2a6d0b95b17879d6e2db7
gate-verdict: blocked @ 4415343 (domain review, draft/dependency state, and hosted checks pending)
merge-ready: no
Exact-head research review — comparator plumbing passes; transfer gate remains open
Exact-head proof passed:
The implementation records observation/sensor boundaries, timeout/runner-error states, and provenance without silently converting the CALF/LegNav preprint setup into Robot SF benchmark evidence. No policy promotion, universal ranking, real-world safety claim, or paper-facing result is established. The PR remains draft and blocked on maintainer/domain review of transferability and the dependency gate. The source is TurtleBot/indoor-simulator research; any later comparison must retain sensor, embodiment, perception, and proxy boundaries. No compute or benchmark campaign is authorized by this review.
|
ll7
left a comment
There was a problem hiding this comment.
Exact-head research/comparator review — blocked
Reviewed PR #7357 at exact live head
e8b15f911cd76751328a8537eb1c2c6185f615cf against current origin/main
88b7188644adb05629551c94d70353d2dce3302f in the dedicated linked worktree.
Findings
- P1 — observation-contract integration blocker (inherited root cause).
scripts/validation/run_policy_search_step_diagnostics.pyreturns simulator/world-frame
pedestrian velocities from_pedestrian_state_from_simand injects them directly into the
policy observation in_apply_observed_pedestrians_to_policy_obs. The canonical observation
contract requires robot-frame velocities and closest-first ordering. The sidecar reproduced
the mismatch at nonzero heading (0.056rad): simulator/world velocity
[-0.1717296, 0.6269043]versus required ego-frame velocity
[-0.1363721, 0.6355334], while the injected value remained world-frame. This predates the
PR, but the new paired comparator relies on the path, so it must repair/gate this contract and
add nonzero-heading and multi-pedestrian ordering regressions before stronger comparison claims. - P2 — executable provenance is incomplete.
_input_refshashes the comparator config,
scenario matrix, and candidate registry, but not the effective candidate/base configuration;
runtime metadata also records a null checkpoint digest. A changed effective policy or model
could therefore alter a diagnostic without changing its recorded input identities. Hash the
resolved config and checkpoint receipt, or preserve an explicit unavailable/blocked status.
No additional P0-P3 defect was confirmed in the comparator metric fail-closed logic, schemas,
fixed-shape overflow handling, fallback/degraded gating, or namespace registration.
Evidence and boundary
- Luna/max exact-head advisory review: 60 focused tests passed; current-main merge tree and
git diff --checkpassed; no changed-code hosted failure was reported (35 successful checks,
2 expected draft-lane skips). - Current
origin/mainis88b7188644adb05629551c94d70353d2dce3302f; the PR declares older base
895d396abb47757b0742cab6b2d677dd00eb80aeand is not rebased. - PR body metadata was reconciled to the current main reference and dedicated linked worktree;
digest:8ab8c2341cefe9986e5b5e17ac60211376457e38fe3e21f576ed28d96ded71ed. - The comparator remains diagnostic-only: one local candidate/situation/seed, no external CALF
or LegNav parity, no calibrated sensor or TurtleBot runtime, no multi-seed uncertainty, and
no benchmark, safety, transfer, ranking, or paper-facing claim. - The PR remains open/draft with
state:blocked, dependency/domain gates, and no merge-ready
admission. No source branch, label, draft-state, or merge status was changed.
base-policy: ordinary-cas @ e8b15f9
gate-verdict: blocked @ e8b15f9
merge-ready: no @ e8b15f9
pr-metadata: reconciled @ 8ab8c2341cefe9986e5b5e17ac60211376457e38fe3e21f576ed28d96ded71ed
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed pushed head f544b6881a2e8129ec9ad9b047e7f7cf1c0ac5f3 after the observation-contract
repair. The new policy-input path keeps observed pedestrian position/velocity/ID rows aligned,
orders complete rows closest-first in world coordinates, preserves world-frame positions, rotates
velocities into the robot ego frame, and rejects malformed row counts. The focused diagnostic and
observation-perturbation suites pass (68 tests), and the change is limited to the intended runner
and tests.
No P0/P1 implementation finding remains from the prior review. The PR is still blocked by the
research evidence boundary:
- P2 provenance:
scripts/benchmark/run_calf_legnav_comparator_issue_7318.pycurrently hashes the
comparator config, scenario matrix, and candidate registry, but does not fully bind the resolved
candidate/base configuration lineage or a verified runtime checkpoint digest. The trace runtime
metadata still reportscheckpoint_sha256: null; a declared registry digest alone would not prove
the checkpoint actually loaded. Stronger benchmark or paper-facing admission must remain blocked
until this lineage is executable and verified, or is explicitly waived by the research owner.
The PR also remains a draft, requires trusted domain approval, and has stale/undeclared non-main
ancestry relative to current origin/main; those gates are not overridden by local test success.
gate-verdict: blocked @ f544b68
merge-ready: no @ f544b68
pr-metadata: reconciled @ 688935256f8d6970a8c362e33af50a703ec37763fd68b4b95cbfe2b2166da0e7
ll7
left a comment
There was a problem hiding this comment.
Exact-head research repair review
Reviewed exact PR head 6a66300f68a330719b84a162350adbbf730d414c.
- The observation-contract repair remains scoped to paired local diagnostic traces: closest-first
actor ordering, world-frame positions, ego-frame velocities, fixed-capacity validation, and
fail-closed malformed-row handling. - The new provenance repair resolves candidate/base/model-registry lineage, records the declared
release digest, computes the resolved PPO checkpoint digest at runtime, and blocks unless both
paired conditions expose the same loaded digest matching the declaration. - Focused proof: 118 comparator/provenance/observation tests and 70 PPO/map-runner/foresight tests
passed; scoped Ruff 0.16.4 andgit diff --checkpassed. - No benchmark, paper-facing, transfer, safety, or planner-ranking claim is made. The PR remains a
draft and requires trusted domain approval; the declared base is stale and ancestry remains an
undeclared stack relative to currentorigin/main.
No new finding on the exact head.
gate-verdict: blocked @ 6a66300
merge-ready: no @ 6a66300
pr-metadata: reconciled @ e962d7e3a292325b7f153bae1807b7a6d389ef6e3749eadca4f08c478e0f2fb4
ll7
left a comment
There was a problem hiding this comment.
Exact-head research implementation review
Reviewed PR #7357 at exact head 6a66300f68a330719b84a162350adbbf730d414c from a fresh linked
worktree against current origin/main=d262a93092acb392692fd0a1a627258d4a45b3a3.
Disposition
Blocked and diagnostic-only. The CALF/LegNav comparator’s observation-frame handling, fixed-capacity
padding, distance completeness, contradiction handling, and diagnostic-only claim boundary are
covered by its focused tests, but no research result or benchmark evidence is admitted.
Findings
- P1: the PR base and current-base evidence are stale (
895d396/body88b7188; current main is
d262a930), so the exact-head proof must be refreshed after a rebase. - P1: the exact-head hosted changed-coverage gate fails at 76.9% (10/13 changed lines) in
robot_sf/benchmark/map_runner/map_runner.py; add focused branch coverage after the refresh. - P1: independent domain approval is absent; the PR is draft and its parent #7318 remains open.
- P2: nested predictive-model expected/observed digests are recorded but not compared before the
comparator accepts provenance. Require equality or fail closed, with a regression test.
Local focused proof reported 77 passed, Ruff/format/diff checks passed, and no registry-local model
artifacts or CALF/LegNav runtime was available. The smallest next action is current-main rebase,
coverage repair, nested-digest regression coverage, and non-author domain review. No merge-ready
verdict or evidence promotion is issued.
review-verdict: blocked @ 6a66300
gate-verdict: blocked @ 6a66300
base-policy: stale-current-main-refresh-required @ 6a66300
domain-approval: pending @ 6a66300
merge-ready: no @ 6a66300
Bind each paired condition slot to the observation contract actually observed in its trace. A perfect_perception slot whose trace reports perception_limited (or vice versa) now marks the condition unavailable/blocked instead of reporting a mislabelled contrast as available evidence. Also add horizon to the paired identity check so the two rows cannot differ in replay length. Refs #7318 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit f9a3f399967e64fb491383e5501216c7c3519cf9)
Exact-head review — held
The current-base focused suites pass (84 tests), the config dry-run passes, and a one-seed local smoke is available with The independent exact-head review found these admission blockers:
Smallest safe next slice: add fail-closed provenance/perturbation/space validation and an end-to-end stub test, then rerun exact-base readiness. Until that exists, this remains held despite the local diagnostic smoke. Review confidence: 0.96 for the implementation-integrity findings; 0.99 that no external/domain evidence was established. |
6a66300 to
5e5a855
Compare
Exact-head review — held
The current-base focused suites pass (85 tests), the config dry-run passes, and a one-seed local smoke is available with The independent exact-head review found these admission blockers:
Smallest safe next slice: add fail-closed provenance/perturbation/space validation and an end-to-end stub test, then rerun exact-base readiness. Until that exists, this remains held despite the local diagnostic smoke. Review confidence: 0.96 for the implementation-integrity findings; 0.99 that no external/domain evidence was established. |
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #7357 at exact head d1d25966c7cd32ee24ba391fdb2ffa366d38c65a against
origin/main 5f476625eba9c4ae2fa5770781852b78c865fd8b.
Intended design and latest repair
This PR is a bounded, diagnostic-only local CALF/LegNav-inspired comparator. It explicitly does
not claim external CALF policy parity, LegNav simulator parity, calibrated sensor validity,
benchmark evidence, safety, ranking, transfer, or a paper result. The latest commit correctly
adds separate predictive-foresight checkpoint provenance: when enabled, both paired conditions must
report loaded, non-fallback diagnostics whose requested and observed digests match the model
registry declaration.
Validation
pytest -q tests/benchmark/test_calf_legnav_comparator.py tests/benchmark/test_checkpoint_provenance_issue_4970.py tests/validation/test_run_policy_search_step_diagnostics.py— 83 passed.- Worktree-local Ruff 0.16.5 check/format, Python compilation,
git diff --check, and clean
git merge-tree --write-tree origin/main HEAD— passed. - Hosted CI at this exact head was not complete: the guarded status showed one pending changed-
coverage gate andmutation-authorized: false; no hosted success was inferred.
Findings and disposition
The nested predictive-checkpoint gap is repaired and focused-tested, but the current PR contract
still identifies implementation blockers for trustworthy comparator output: the selected scenario
and complete include/asset graph are not admission-bound, perturbation settings/positions are not
fully proven within policy-space bounds, complete outcome flags are not yet guaranteed before
metric interpretation, and optional track IDs can detach from sorted rows. The PR also still lacks
independent domain approval and the external-data/embodiment/multi-seed evidence required for any
stronger research claim. These are blockers for the current diagnostic contract, not evidence of a
benchmark result.
Decision: hold at state:blocked/draft. No P0–P3 repair is safely isolated in this review slice;
the smallest next proof is to close the remaining local contract gaps, then rerun current-head
coverage/CI and obtain independent domain approval while preserving the diagnostic-only boundary.
Do not apply merge-ready or merge this head.
gate-verdict: blocked @ d1d2596
pr-metadata: reconciled @ 808da774a0164bc05f7f752b82b4e547cea2834c1869cff5be01c7ce2f5546a0
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #7357 at exact head 0d961382b63783f580b55deab46a26d77ea0c9fb against
origin/main 5f476625eba9c4ae2fa5770781852b78c865fd8b.
Intended design and latest repair
This PR is a bounded, diagnostic-only local CALF/LegNav-inspired comparator. It explicitly does
not claim external CALF policy parity, LegNav simulator parity, calibrated sensor validity,
benchmark evidence, safety, ranking, transfer, or a paper result. The latest commit repairs the
independent review finding that predictive checkpoint bytes were not enough to bind model
identity: when enabled, both paired conditions now expose requested/runtime model IDs and must
match the configured model-registry identity in addition to loaded, non-fallback diagnostics and
matching requested/observed checkpoint digests.
Validation
pytest -q tests/benchmark/test_calf_legnav_comparator.py tests/benchmark/test_checkpoint_provenance_issue_4970.py tests/validation/test_run_policy_search_step_diagnostics.py— 84 passed.- Worktree-local Ruff 0.16.5 check/format, Python compilation,
git diff --check, and clean
git merge-tree --write-tree origin/main HEAD— passed. - Hosted CI at this exact head is not complete: the guarded status currently has queued and
in-progress checks, withmutation-authorized: false; no hosted success is inferred.
Findings and disposition
The model-identity gap is repaired and focused-tested, but the current PR contract still
identifies implementation blockers for trustworthy comparator output: the selected scenario and
complete include/asset graph are not admission-bound, perturbation settings/positions are not
fully proven within policy-space bounds, complete outcome flags are not yet guaranteed before
metric interpretation, and optional track IDs can detach from sorted rows. The PR also still
lacks independent domain approval and the external-data/embodiment/multi-seed evidence required
for any stronger research claim. These are blockers for the current diagnostic contract, not
evidence of a benchmark result.
Decision: hold at state:blocked/draft. No P0–P3 repair is safely isolated in this review slice;
the smallest next proof is to close the remaining local contract gaps, then rerun current-head
coverage/CI and obtain independent domain approval while preserving the diagnostic-only boundary.
Do not apply merge-ready or merge this head.
gate-verdict: blocked @ 0d96138
pr-metadata: reconciled @ 1a9a33b25f00f60b1b6d9581e1d2ccd160e896419ceb12621529c1dbf59a01cd
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #7357 at exact head 3dc3c08fb30e23afef56d178437030b849dbfb64 against
origin/main 5f476625eba9c4ae2fa5770781852b78c865fd8b.
Intended design and latest repair
This PR is a bounded, diagnostic-only local CALF/LegNav-inspired comparator. It explicitly does
not claim external CALF policy parity, LegNav simulator parity, calibrated sensor validity,
benchmark evidence, safety, ranking, transfer, or a paper result. The latest commits repair the
independent review findings that predictive checkpoint bytes were not enough to bind model
identity and that malformed IDs could violate the report schema: when enabled, both paired
conditions now expose normalized requested/runtime model IDs and must match the configured
model-registry identity in addition to loaded, non-fallback diagnostics and matching
requested/observed checkpoint digests.
Validation
pytest -q tests/benchmark/test_calf_legnav_comparator.py tests/benchmark/test_checkpoint_provenance_issue_4970.py tests/validation/test_run_policy_search_step_diagnostics.py— 86 passed.- Worktree-local Ruff 0.16.5 check/format, Python compilation,
git diff --check, and clean
git merge-tree --write-tree origin/main HEAD— passed. - Hosted CI at this exact head is not complete: the guarded status currently has queued and
in-progress checks, withmutation-authorized: false; no hosted success is inferred.
Findings and disposition
The predictive model-identity and malformed-provenance gaps are repaired and focused-tested, but
the current PR contract still identifies implementation blockers for trustworthy comparator
output: the selected scenario and complete include/asset graph are not admission-bound,
perturbation settings/positions are not fully proven within policy-space bounds, complete outcome
flags are not yet guaranteed before metric interpretation, and optional track IDs can detach from
sorted rows. The PR also still lacks independent domain approval and the external-data/embodiment/
multi-seed evidence required for any stronger research claim. These are blockers for the current
diagnostic contract, not evidence of a benchmark result.
Decision: hold at state:blocked/draft. No P0–P3 repair is safely isolated in this review slice;
the smallest next proof is to close the remaining local contract gaps, then rerun current-head
coverage/CI and obtain independent domain approval while preserving the diagnostic-only boundary.
Do not apply merge-ready or merge this head.
gate-verdict: blocked @ 3dc3c08
pr-metadata: reconciled @ 0c3970809f206e7ae70d121574c34f63e70b6d7f67491575c7948b5541a44000
Summary
Implements #7318 as a bounded CALF/LegNav-inspired Robot SF comparator diagnostic. It runs the
same local PPO candidate, scenario, and seed under ideal and perception-limited observation
contracts, records paired local trace metrics, and preserves explicit blocked, unavailable, and
qualified-proxy fields.
The comparator does not execute the external CALF policy, LegNav simulator, calibrated leg sensor,
TurtleBot 4 deployment, or source training recipe. Its output is diagnostic-only and is not
benchmark, safety, transfer, or paper-facing evidence.
Research / Evidence Boundary
perception-limited observations for one local policy smoke.
contracts; no external policy parity or scientific comparator is asserted.
estimate uncertainty or support a research conclusion.
distance fields; incomplete distance coverage is unavailable rather than silently reduced to a
partial denominator.
done_info.successcannot be overridden by a true row-levelis_success; contradictory outcomes are unavailable.missing traces, runner errors, schema mismatch, missing or contradictory fallback/degraded
verdicts, or unknown source fields remain blocked or unavailable.
Exact Current-Base Refresh
origin/main=5f476625eba9c4ae2fa5770781852b78c865fd8b.3dc3c08fb30e23afef56d178437030b849dbfb64.origin/main;this repair adds no default runtime behavior or benchmark promotion.
research/issue-7318-calf.Repair Scope
as unavailable when any executed row lacks both distance fields.
row success flag instead of combining them with an unconditional
or.and reject actor-capacity overflow rather than truncating evidence.
planner statistics over an adapter-only summary.
coordinates; positions stay in world coordinates and velocities are rotated into the robot ego
frame for policy inputs. Nested and flat observation layouts fail closed on malformed rows.
registry, and runtime artifact; paired conditions block unless their computed digest matches the
registry-declared release digest.
must report loaded, non-fallback runtime diagnostics whose requested model IDs and
requested/observed checkpoint digests match the nested model-registry identity; missing,
malformed, or mismatched values block.
Prior Diagnostic Smoke (Historical Context Only)
The one-seed local smoke completed with
status=available,runner_error_count=0, and bothconditions loading the same registry-matching checkpoint without fallback. It produced 12-step
paired traces: success rate
0.0, timeout rate1.0, and collision rate0.0for bothconditions. The sensor-limited minus ideal deltas were
-231.187 rad/s^3for the qualifiedangular-jerk proxy and
-0.524 action_units/stepfor local action smoothness. This is onedeterministic paired episode; uncertainty is unavailable and these values are not a CALF/LegNav,
benchmark, safety, ranking, or transfer result.
This smoke predates the current-head runtime model-identity repairs, so it is retained only as
historical diagnostic context—not exact-head integration or merge evidence. No new model execution
was attempted on this head because the required local model cache was unavailable; focused contract
proof and clean merge-tree checks are the current local proof.
Domain-Aware Approval
execution handling, current-base integration, and CALF/LegNav claim boundaries.
the PR remains held as
draft,state:blocked, anddomain-review-required.domain approval or a waiver.
same-seed traces with per-action distance completeness and explicit metric availability.
verdicts remain blocked or unavailable.
paper-facing claim.
external-policy parity, multi-seed uncertainty, and experimental validity remain unproved.
Validation / Proof
86 passed.origin/main=5f476625eba9c4ae2fa5770781852b78c865fd8b:passed for the affected suites, scoped Ruff check/format, Python compilation,
git diff --check,and a clean merge tree. Full repository readiness and hosted checks for this new head remain
pending.
ID and missing/non-string IDs; the comparator records paired requested/runtime IDs and fails
closed without putting non-string values into schema-bound input references.
and unavailable/fallback runtime diagnostics.
recipe, benchmark campaign, or uncertainty estimate was produced.
hosted CI and changed-line coverage are pending.
Remaining Blockers / Follow-Up
embodiment, real-manifest, multi-seed, multi-scenario, and uncertainty decisions.
external CALF/LegNav policy, sensor, embodiment, multi-seed, and transfer validity remain
unproved and cannot be inferred from this diagnostic.
perturbation settings, policy-space bounds, complete outcome flags, and nonempty runtime
provenance as not yet admission-bound; optional actor IDs can also detach from sorted rows.
These remain implementation blockers for trustworthy comparator output.
Gate
The bounded diagnostic contracts are locally exercised, including the runtime predictive model
identity guard. Independent domain approval, remaining contract blockers, external parity, current-
head hosted checks, and the parent research decision still keep this PR draft and blocked. No
merge-ready admission or merge is requested.
gate-verdict: blocked @ 3dc3c08fb30e23afef56d178437030b849dbfb64merge-ready: no @ 3dc3c08fb30e23afef56d178437030b849dbfb64compute: local diagnostic smoke only; no external computePR Metadata
5f476625eba9c4ae2fa5770781852b78c865fd8b.3dc3c08fb30e23afef56d178437030b849dbfb64.