fix(nav): tighten SVG geometry contract evidence (#8449) - #8450
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. |
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #8450 at exact head 7ac518f3ff57950f774caf7d06e9ae805b41d05d against
origin/main 5f476625eba9c4ae2fa5770781852b78c865fd8b.
Intended design and evidence boundary
This is a research-adjacent SVG geometry-contract follow-up to merged PR #8373. It narrows
legacy/corrected wording, applies corrected-mode path translations consistently, and adds parser,
real-map route, cache, and scenario-loader coverage. The change is diagnostic implementation
evidence only: it does not replace frozen legacy artifacts, launch a benchmark, or claim planner,
safety, ranking, causal, release, or paper results.
Validation
- Focused navigation and scenario-loader selection — 102 passed.
- Ruff 0.16.5 check/format,
git diff --check, and clean
git merge-tree --write-tree origin/main HEAD— passed. - The PR body records a broader focused suite and docs/optional-dependency checks; I did not
independently treat its historical full-readiness attempt as current-head proof.
Findings and disposition
No actionable P0–P3 implementation finding was identified in this exact-head slice. The parser
change is localized, corrected-mode translation is applied only through the corrected extraction
path, and the new cache/contract tests exercise the stated boundary. The remaining admission gaps
are explicit and material: the PR is intentionally draft, the canonical full readiness run is not
clean, independent SVG-geometry/domain approval is absent, and corrected-contract scenario
revalidation remains deferred. Those gaps prevent merge or stronger evidence admission but do not
constitute a new code defect in this review slice.
Decision: hold at draft/diagnostic. Complete a clean canonical readiness run and obtain independent
domain-aware approval before promoting or merging. Do not apply merge-ready.
gate-verdict: blocked @ 7ac518f
pr-metadata: reconciled @ 99d84bed452c7d471f3437086de338379817d4656c02e8b9e9ac48070ff15526
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #8450 at exact head 7ac518f3ff57950f774caf7d06e9ae805b41d05d against
origin/main 5f476625eba9c4ae2fa5770781852b78c865fd8b.
Intended design and evidence boundary
This is a research-adjacent SVG geometry-contract follow-up to merged PR #8373. It narrows
legacy/corrected wording, applies corrected-mode path translations consistently, and adds parser,
real-map route, cache, and scenario-loader coverage. The change is diagnostic implementation
evidence only: it does not replace frozen legacy artifacts, launch a benchmark, or claim planner,
safety, ranking, causal, release, or paper results.
Validation
- Focused navigation and scenario-loader selection — 102 passed.
- Ruff 0.16.5 check/format,
git diff --check, and clean
git merge-tree --write-tree origin/main HEAD— passed. - The PR body records a broader focused suite and docs/optional-dependency checks; I did not
independently treat its historical full-readiness attempt as current-head proof.
Findings and disposition
No actionable P0–P3 implementation finding was identified in this exact-head slice. The parser
change is localized, corrected-mode translation is applied only through the corrected extraction
path, and the new cache/contract tests exercise the stated boundary. The remaining admission gaps
are explicit and material: the PR is intentionally draft, the canonical full readiness run is not
clean, independent SVG-geometry/domain approval is absent, and corrected-contract scenario
revalidation remains deferred. Those gaps prevent merge or stronger evidence admission but do not
constitute a new code defect in this review slice.
Decision: hold at draft/diagnostic. Complete a clean canonical readiness run and obtain independent
domain-aware approval before promoting or merging. Do not apply merge-ready.
gate-verdict: blocked @ 7ac518f
pr-metadata: reconciled @ 99d84bed452c7d471f3437086de338379817d4656c02e8b9e9ac48070ff15526
|
Independent targeted verification of head
This is targeted regression evidence, not full canonical readiness, independent domain approval, or permission to mix historical/corrected campaign rows. The PR remains draft. Existing full-readiness/domain stops are unchanged; no duplicate repair was opened. |
|
review-claim: codex-goal-autopilot-20260905 @ 7a55403 until 2026-09-05T00:10:20Z |
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed draft PR #8450 at exact head 7a55403246e2c236cd8a3423859ae3cb7f7960ab against current origin/main e7b6f9bfde9f80ca65d62bb1d85c681946e2a3dd in a dedicated linked review worktree.
Findings and disposition
No actionable P0-P3 implementation finding was verified in the scoped review. The diff is limited to the SVG geometry-contract follow-up, scenario-loader contract/cache coverage, documentation wording, and focused tests; it does not replace frozen artifacts or claim benchmark, ranking, causal, safety, release, or paper-facing evidence.
Focused proof at this exact head passed:
tests/nav/test_svg_transform_contract.pyandtests/training/test_scenario_loader.py: 113 passed;- scoped Ruff check and format check: passed;
git diff --check origin/main...HEAD: passed;git merge-tree --write-tree origin/main HEAD: conflict-free.
The PR remains intentionally draft and blocked for admission. The live hosted checks still have required jobs pending, the PR body records that the prior optional CUDA-safe serial lane was interrupted without a terminal result, and the required independent domain-aware approval is not present. The body correctly limits its existing domain-approval waiver to implementation smoke and does not treat it as merge or evidence authorization. I did not run a benchmark or the unbounded optional lane locally.
base-policy: ordinary-cas @ 7a55403246e2c236cd8a3423859ae3cb7f7960ab
pr-metadata: reconciled @ f2d61c29cf4b5290765ea7fd72f54b682dab316c7d255542df98c201470f1925
gate-verdict: blocked @ 7a55403246e2c236cd8a3423859ae3cb7f7960ab
merge-ready: no @ 7a55403246e2c236cd8a3423859ae3cb7f7960ab
|
review-claim: released @ 7a55403 Exact-head review published as COMMENTED review 5118367130. Disposition: no actionable implementation finding; keep draft and do not merge while hosted checks, independent domain-aware approval, and the clean optional readiness lane remain incomplete. |
Exact-head follow-up statusVerified after the proof update at head
The PR remains draft/blocked. The local all-extras environment is available, but a gate-verdict: blocked @ 7a55403 |
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #8450 at exact head 03998d67c4d7686e81c9f797e12ba0ab2fc9a67c against fresh
origin/main 1cb110f36cbcc96f781c3e5e8f99b13e60a0cb3e in linked worktrees.
Current-head repair
- The prior P2 coverage gap is fixed with a regression that writes a path translated by
translate(5e-13,-7e-13), verifies corrected mode applies both components, and verifies legacy
mode preserves the historical coordinates. - No new P0-P2 implementation finding was found in this bounded exact-head review slice.
Validation
- Exact-branch SVG transform contract suite:
65 passed. - Ruff 0.16.5 check/format and
git diff --checkpassed. - The PR remains diagnostic implementation evidence only: no benchmark, ranking, causal, safety,
release, fallback-success, or paper-facing conclusion is made.
The PR should remain draft/blocked. A clean canonical readiness run, independent domain-aware
approval for SVG geometry and historical replay, and corrected-contract scenario revalidation are
still required before merge or evidence use. The prior optional readiness attempt is not current-head
proof.
Decision: hold at draft; the reviewed P2 is repaired, but the declared domain/readiness gates remain
open.
gate-verdict: blocked @ 03998d6
base-policy: ordinary-cas @ 03998d6
pr-metadata: reconciled @ ef612bf9b911b596d80bb53f47356f3624315ddab043fdfdb2f566e5155aa835
Summary
Follow up the now-merged PR #8373 by tightening the explicit SVG geometry-contract boundary:
remove overbroad compatibility wording, apply all corrected-mode path translations consistently,
and add focused coverage for parser edge cases, exact legacy state, real-map routes, scenario/cache
propagation, and sub-threshold path translations.
The strict parser and non-SVG fail-closed implementation are already in the base through #8373; this
follow-up makes those guarantees executable and keeps the remaining claims precise.
Linked Issues
Stack / Dependency
main.maincontains the parent implementation.What Changed
near-zero threshold used only for paths.
1.e2, whitespace/comma forms, adjacenttranslations, exponent forms, malformed numeric input that must not be partially parsed, and a
path whose translation components are both below
1e-12.MapDefinitionstate field on all fivetransformed maps under both default and explicit legacy parsing; the additive provenance field
is intentionally excluded.
cache identity, unknown contract names, and corrected contracts on non-SVG maps.
supported historical-geometry and explicit-contract guarantees.
Why It Matters
The parent implementation changes the loader contract, so its evidence must distinguish exact
supported behavior from stronger guarantees that the generic downstream aggregation layer does not
enforce. These tests and wording corrections make the compatibility boundary reviewable without
changing frozen results or launching a benchmark campaign.
Research / Evidence Notes
outputs, and one authored bottleneck route.
loading and its cache; this commit adds regression coverage and consistent path shifting.
without an explicit compatibility decision.
or paper-facing conclusion.
corrected-contract scenario revalidation if that work is approved.
Validation / Proof
164 passed; the new sub-thresholdregression file was then validated at the exact PR branch with
65 passed.git diff --check: passed for the changed implementation/tests.while new hosted checks for this exact head are pending.
CUDA-safe serial optional lane ran for more than 30 minutes without a terminal result and was
interrupted cleanly; its termination receipt verified process-group cleanup. The base advanced
afterward, so that receipt is not current-head proof. Full readiness is not claimed here and this
PR remains intentionally draft.
Risks / Rollback
Docs / Provenance
23d07d6cf32a6b88027c767d3861e1f00aa1eecc; they prove exact pre-existing-field equivalence,not generic whole-object byte serialization including additive provenance.
dependency of this draft.
Downstream Propagation
MapDefinition.svg_geometry_contractremains the recorded compatibility label.pooling legacy and corrected executions; this PR does not add a generic aggregation guard.
Follow-Up / Residual Scope
draft to ready.
Reviewer Notes
translate(1.e2)and malformedseparators/numbers.
additive
svg_geometry_contractmetadata.translate(5e-13,-7e-13)while legacy mode preservesthe historical path coordinates.