Skip to content

fix(pipeline): preserve reconstruction and verification evidence - #240

Merged
SamGu-NRX merged 21 commits into
alphafrom
fix/review-debt-bs-pipeline
Oct 14, 2026
Merged

SamGu-NRX merged 21 commits into
alphafrom
fix/review-debt-bs-pipeline

Conversation

@SamGu-NRX

@SamGu-NRX SamGu-NRX commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

What changed

This fixes review comments from already-merged PRs that were never addressed. Reconstruction and verification now refuse the tested invalid evidence, preserve capture inputs and report incomplete runs as incomplete.

#20: recon: handoff to the server team

  • Reconstructed walls retained old facing and overhead measurements when the meter barely moved. The target wall now loses those entries whenever its baseline is replaced; the rotation regression preserves other walls and the input document. Review 1. Review 2.

  • Clearance crossed occupied slabs below the 20% threshold or only one voxel thick. Any occupied sampled voxel now stops the facing or overhead search. Tests cover a thin rail and a one-step obstruction. Review 1. Review 2.

  • Ground samples used a flat datum despite fitting a slope. Sampling now evaluates the fitted plane at each point and uses its normal; synthetic sloped depth supports the surface and rejects the old horizontal samples. Review 1. Review 2.

  • Coverage could extend beyond the fitted wall. Only complete half-foot cells inside the extent are exported; regressions cover partial endpoints and a wall shorter than one cell. Review 1.

  • LiDAR without confidence data entered fusion unfiltered. It now fails before reading depth; mixed-capture fixtures supply actual confidence bytes and the missing-map regression checks the refusal. Review 1. Review 2.

  • Untrusted ZIPs could exhaust disk or allocate unlimited entry metadata. Admission reuses the verifier policy of 2,000 entries and 512 MiB, streams hashes and central records, and publishes extraction only after all members finish. Tiny archives exercise byte/count limits, a false entry count and failed extraction. Review 1. Review 2.

  • Corrupt caches blocked subsequent runs, and concurrent runs could overwrite them. Cache validation and inference run under one advisory lock; malformed archives trigger recomputation and completed NPZ files publish atomically. Tests exercise truncated ZIPs, invalid deflate data, lock exclusion and an interrupted write that preserves the prior cache. Review 1. Review 2.

  • EXIF orientation rotated sensor pixels away from their intrinsics and poses. Image loading ignores EXIF rotation; fixtures with orientation 3 and 6 retain raw pixels. Review 1. Review 2.

  • Failed or no-server reruns left earlier answers beside new outputs. Known outputs now clear at startup, with input/output overlap refused before deletion. Regressions preserve capture inputs and unrelated files. Review 1. Review 2.

  • The export silently sliced observed entries at 500. It now raises a specific error above the existing schema limit; boundary tests retain all 500 entries and refuse 501. Review 1.

  • Follow-up review on this PR found another damaged-cache failure: an unsupported ZIP compression method raised NotImplementedError and survived every retry. The cache-repair block now handles that read failure. The regression writes a damaged cache file, verifies fresh inference, then retries and checks that the repaired cache is reused. Cache-retry review.

  • Deep review found that a later malformed ZIP footer in a comment could bypass the early directory cap. Admission now selects the same footer as Python and refuses malformed tails and ZIP64 locator overrides before constructing ZipFile. Tiny fixtures assert that metadata allocation is never reached; normal comments still extract. Deep review.

Vet follow-ups protect the effective ZIP extraction directory and its ancestors/descendants before startup cleanup. Cached members are verified by name, size and streamed CRC and damaged caches are re-extracted under a lock. Overhead samples include both edges of the existing 0.56 m footprint, catching the missed 1.90 m eave; support for other battery depths still needs the server footprint contract. At df7b75d, twelve new recon cases failed before these fixes; the guarded recon suite passes 125 tests on #240 and 149 on #242.

#13: verification: independent end-to-end checks (S4)

  • Sweep footprint validation trusted endpoints that could lie on opposite sides of a corner. It now checks sampled interior starts, with passing and unsure corner-crossing regressions. Review 1. Review 2.
  • Malformed replay poses could crash accuracy reporting, and oversized integers escaped finite-number validation. Accuracy now runs only after valid format checks, and numeric conversion reports overflow as a format error. CLI and oversized-integer regressions retain the failure report. Review 1. Review 2. Review 3.
  • Simulator runs could clobber app staging, leak copies on preparation failure, race the shared device or omit leading warnings. Copies now have unique paths and cleanup, both verifier runners share a device lock, and warning parsing accepts both diagnostic forms. Mocked preparation failures and a second-process lock test exercise these paths. Review 1. Review 2. Review 3. Review 4.
  • Failed XCTest output could count as clean accessibility evidence. Success now requires zero exit status and the success marker without a failure marker; the regression supplies clean screen records with a failed process. Review 1.
  • Transformed server responses were checked without the bytes actually sent. Each transform now passes its serialized request into hash validation; the regression rejects stale input hashes. Review 1.
  • Ground trimming crashed when reach was omitted, and geometric case assumptions were not compared with active rules. Omitted reach now stays zero; battery width and corner allowance are compared before stale expectations are judged. Both behaviors have targeted regressions. Review 1. Review 2.
  • A suffix match could choose old-scene.json instead of the uploaded scene. Bundles now require exactly one root scene.json; tests cover misleading names, absence and duplicate canonical entries. Review 1.
  • Skipped probes and incomplete runs could establish acceptance. An unexercised archive probe now fails, all_passed requires every record to pass, and S2-8 requires that successful-run gate while retaining independent contract and expectation counts. Report and scoreboard regressions cover skipped and expectation-failed runs. Review 1. Review 2. Review 3.
  • Deleted remote branches and the default 30-PR inventory could leave incomplete scoreboard evidence. Fetch now prunes, and PR listing requests up to 10,000 entries under its existing timeout. Tests verify pruning and a 41-PR inventory. Review 1. Review 2. Review 3.
  • Facade coverage overshot or omitted a non-grid wall tail. The last cell clips to the wall and probes its midpoint; dataset-free tests cover 2.2, 2.3 and 0.05-foot walls, including a visibility predicate that refuses any probe beyond the fitted extent. Review 1.

The shared root dispatcher was the one necessary exception to the folder scope. Makefile now invokes verification, and tests/makefile.sh checks its inclusion, standalone target, missing-folder refusal and failure propagation across all 64 optional-suite combinations. Review 1.

Deep-review follow-ups now require all five hostile probes to have actually executed before S2-8 receives full acceptance; --no-hostile and legacy reports cannot earn that credit. The generator and 43 committed cases declare battery width, with corner allowance on the four relevant fixtures, so changed rules skip stale expectations and cannot establish acceptance. Only assumption metadata changed; geometry and expectations are unchanged. The shared root verification target runs uv sync --locked before its child tests, with dispatch and sync-failure regressions. Hostile evidence review, case assumptions review, locked sync review.

Codex found that a server-reported sweep such as [0, 1e9] could allocate billions of starts. Sweep admission now rejects spans outside the scanned chain before sampling, and the sampler validates eagerly and yields lazily with a 10,000-sample cap including the endpoint. Oversized runs fail with a specific message rather than get truncated. Regressions cover both outcomes, both scan ends, a distant singleton, tiny rule steps and the exact cap boundary. Sweep sampling review.

One runtime meter counts the geometry and comparison work that actually runs, including unsure checks, footprint lookups, ground grids, coverage and corner checks. Identical sweep records are deduplicated, and identical geometry checks are memoized. Structural admission rejects nonfinite numbers, overflowing span lengths, invalid dimensions and unknown wall references before geometry. The ground sampler is lazy; ordering comparisons use an interval index. A failed admission or invariant stops property comparisons, and any check exception becomes a named report problem. Exceeding the runtime limit produces verification work limit exceeded (N samples). Verification ZIP admission still counts directories before metadata allocation, responses remain bounded at 4 MiB, and required simulator states must occur in order.

The runtime limit is now 7,834,000 units, twenty times the remeasured valid peak of 391,700 on a saved ETH3D facade response. After caching, 35 valid public-fixture judgments including property variants peak at 31,649 units. Twenty additional measurements cover an 80-foot surveyed wall written as twenty walls, each subdivided 1, 4, 12, 13 or 20 times, original and mirrored, with full-judge resends. Those peak at 88,999 units. Another 468 distinct saved responses are included in the measurement corpus. These are operational work counts, not physical clearance accuracy.

Guarded unit suites pass on this repair: 494 verification tests and 126 recon tests. All checks used run-guarded.py --min-free-gib 8 after the 10 GiB startup threshold was met. Fourteen judge regressions fail on 2d959c7, including oversized integers and unknown wall references. Bounded old-head controls also reproduce repeated unsure geometry, quadratic interval lookup and rejection of the valid direct-coverage result.

Null rules_assumed and null expectation query arrays now produce bad input records before counting. A regression saves both failed cases and a subsequent valid case in one report. The matching-public wrong-width and malformed-case controls fail on a7b3169. Public-policy review. Case metadata review.

Repeated sweep and check expectation failures now produce one message per expectation, with a failure count, the reason once and at most three examples. A shared report budget retains at most 1,000 diagnostic entries and 262,144 serialized diagnostic characters, including duplicate JSON fields, and emits one omitted-count summary. Contract counts and acceptance are computed before display truncation; raw response payloads remain separate. Markdown-visible explanations receive budget priority over their duplicate JSON classifications. Both 1,000-error regressions retain the first explanation in the Markdown row and stay within the shared limits; both fail on c32ee97. Numeric cache and interval-map keys use exact rational strings, preserving equal integer/float values while preventing cheap modulus collisions. Regressions cover 1,000 runs with a 4,096-character reason, oversized reasons, JSON escaping and 2,000 colliding integers across four key paths.

The sampler count API now explicitly counts starts before the final endpoint; run_starts yields those starts and then the endpoint. Pinned db1e4c2 controls confirm that the old implementation already checked the last pre-end start, checked a sub-step run's first start and charged each yielded sample. The new count-API assertions fail there; the functional sampling controls already pass. Sampler review 1. Sampler review 2.

#22: packet: point to the server's capture packet 0.4 and propose 0.5 additions

  • The 0.9/24-inch figures omitted their subgroup and reference scale. The proposal now identifies in-plane ETH3D electro edges scaled using laser truth, matching the committed study methods and results. Review 1.
  • Estimated-depth uncertainty had no malformed-map validation requirements. The unapproved proposal now requires the matching float32 sample count, finite nonnegative values and reported read failures. Review 1.
  • A mesh could claim classification while omitting classes. The proposed layout now requires the property when mesh.classified is true. Packet contains only proposal documents; no retired parser was restored. Review 1.

#179: Ship beta to main: the app, server and experiments as submitted

The malformed-mesh finding refers to the retired Packet 1.1 parser. Commit 6a12700 removed the parser, schema and tests; the current alpha tree contains only the two packet proposal documents. The finding is obsolete.

Why

32 repaired findings were already marked resolved while their defects remained on alpha. The full 137-row triage is kept in the assigned external triage file. Existing fixes and deleted packet 1.1 code were checked against current source.

Verification

Scene/rule derivatives, wall lookups and ground plans are cached per judgment; collinear vertices within one wall no longer multiply geometry work. Direct coverage builds no ground fallback strips. Full judgments at 13 and 20 subdivisions fail at the old work limit before this repair and pass now with costs within 20% of the one-segment representation. Distinct evidence IDs and duplicate diagnostics use hash lookups; 12,000-ID and 4,000-width controls fail on the old head after 100,001 comparisons and pass with linear counts.

Metadata admission precedes schema validation, copies and resend comparison. It bounds walls to 50, vertices per wall to 500, runs to 30,000, checks and requests to 16,000 each, numeric leaves to 1,000,000, JSON nodes to 2,000,000, nesting to 32 and string/key characters to 4 MiB per document. Local case rules have 64 queries. Boundary controls verify clean admission and specific over-limit diagnostics.

Reported widths and corner allowances now share the same handling in both PRs. A case mismatch uses the response's reported value. Geometry adopts a validated reported override only for a different policy fingerprint. Matching public fingerprints retain the loaded rules and contract-check all reported battery dimensions, route height and corner allowance, including transformed responses. An unreported value is unverified when policy sources include private rules or the normalized rule fingerprint differs from the tested public rules, even if the assumed numeric value equals the public default. Four private-source/fingerprint controls fail on 30489f8 before this repair. Encrypted NPZ flags now trigger locked cache repair; the damaged-file control fails before the repair with a password-required RuntimeError.

The public-solver compatibility fixture has an 80-foot wall and 683 candidates. It is accepted with 29,760 actual visits and no ground-grid calls, while 2d959c7 rejects it under its predicted budget. Both missing-evidence branches now compare one unsure check with 10,000 duplicates and use 130 units either way; the old branches invoke geometry 10,000 times. The remeasured valid peak is 391,700 units.

  • Verification: 454 passed. Recon: 126 passed. Both used the existing Python 3.12 interpreter with -m pytest -q through run-guarded.py --min-free-gib 8 from the component folder.
  • The unsupported-compression regression failed with NotImplementedError on b5da197 before the repair; it passes with the fix.
  • Cached Ruff 0.16.10 passed check . and format --check . in verification and recon; git diff --check passed.
  • bash tests/makefile.sh passed all stub-based dispatch cases. It invoked no real build tools.
  • The reconstruction regression file failed 23 cases against an isolated original-alpha snapshot, with 2 controls passing, before the final compressed-cache case was added. An independent read-only review found that additional decompression gap; the repair and new regression were rechecked.

Risk

The reused environment has OpenCV 5.0.0 while recon declares <5. No dependencies were installed. These unit results do not establish the declared OpenCV range or real dataset acceptance. Model inference, ETH3D regeneration, iOS app builds, simulators and private backend behavior were not exercised. The companion live-dots app builds and all 45 Swift tests pass on #242 commit b6bc4ad. Both commands ran through run-guarded.py --min-free-gib 8 with two build jobs; Metal pixels and native interaction were not inspected.

The archive admission limits reuse existing verifier policy, not measured capture maxima. Split archives and ZIP64 end-record metadata are explicitly refused. Cache and device locks are advisory POSIX locks; unrelated manual tools do not participate. Packet edits clarify unapproved proposals and do not change the upload contract.

Checklist

  • The scope is focused on the assigned review findings and the required shared dispatcher
  • I explained what changed and why
  • N/A: no visible UI changes or motion

Verification accepts root scene.json or a scene under one shared top-level capture folder, including the public API Finder fixture. Unlike the public API's root precedence, it refuses both forms together so the verifier has one unambiguous input. Every non-metadata entry must lie beneath the shared folder for the wrapped form. All entries still count toward archive limits. macOS metadata is ignored after the archive limits count every entry; duplicate and ambiguous scene paths still fail. The wrapped-capture regression fails at 68cb685 and passes here. Wrapped bundle review.

Merge order

Merge #240 first, then #242. #242 contains #240 at a7b3169 through merge commit daa36a2. Both include alpha 3c33ad7, retain the same runtime work meter and ingestion bounds, and preserve the shared Makefile dispatch repairs. Neither branch was rebased or force-pushed.

Companion fixes and remaining decisions

#242 carries the remaining executable repairs from this assignment: two-view free-space evidence (#20), rules-bound facade headroom, HTTP limits/deadlines, ordered simulator flow, monotonicity comparisons and accessibility identity (#13), and the anchor-local ground proposal (#22). It also fixes the follow-up review findings on those repairs. Historical replies remain drafted for the coordinated posting pass.

The genuine decisions remain ground reach, overhead footprint and photo-clearance uncertainty (#20), and per-capture meter yaw uncertainty (#22), as recorded in DECISIONS.md and the Opus decisions file. The app's evidence-replay policy (#18) is also still a decision on #242. The packet ground representation remains an unapproved proposal; #242 clarifies its coordinate frame without adopting a wire contract.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of capture archives, including clearer rejection of oversized or malformed files.
    • Improved reconstruction consistency for ground-aware coverage, wall measurements, and repeated runs.
    • Improved depth-data validation and recovery from invalid cached results.
    • Strengthened verification so incomplete runs and failed checks are reported accurately.
  • Documentation

    • Clarified validation requirements for depth uncertainty data and classified meshes.
    • Updated measurement guidance to reflect calibrated photo scaling and reported error ranges.

@codemetrics-ai

Copy link
Copy Markdown

Note

Review skipped

Codemetrics is installed on this repository, but it hasn't been added to your team yet.

To enable AI code reviews for this repo, go to your dashboard and add BaseScanning to your team's repositories.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ba2bccda-381d-4cb2-9b1b-6d9fc710a1c3
📥 Commits

Reviewing files that changed from the base of the PR and between cde8450 and 1194523.

📒 Files selected for processing: 30
  • Makefile
  • packet/README.md
  • packet/validator-checks.md
  • recon/HANDOFF.md
  • recon/models/moge_depth.py
  • recon/recon/capture.py
  • recon/recon/coverage.py
  • recon/recon/depth.py
  • recon/recon/pipeline.py
  • recon/recon/scene.py
  • recon/tests/test_review_debt.py
  • recon/tests/test_worker.py
  • tests/makefile.sh
  • verification/hsverify/a11yaudit.py
  • verification/hsverify/e2e.py
  • verification/hsverify/gitref.py
  • verification/hsverify/replaycheck.py
  • verification/hsverify/resultcheck.py
  • verification/hsverify/scoreboard.py
  • verification/hsverify/simrun.py
  • verification/scenes/eth3d-facade/build_scene.py
  • verification/scoreboard/metrics.yaml
  • verification/tests/test_a11yaudit.py
  • verification/tests/test_build_scene.py
  • verification/tests/test_e2e.py
  • verification/tests/test_gitref.py
  • verification/tests/test_replaycheck.py
  • verification/tests/test_resultcheck.py
  • verification/tests/test_scoreboard.py
  • verification/tests/test_simrun.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

This pull request updates reconstruction inputs, depth caching, coverage calculations, and generated outputs. It also changes verification checks and simulator execution, integrates the verification suite into make check, and revises packet documentation.

Changes: Reconstruction pipeline

Layer / File(s) Summary
Bounded capture archive extraction
recon/recon/capture.py, recon/tests/test_review_debt.py
ZIP handling enforces entry and byte limits, validates archive records, and extracts into staging before publishing the cache. Tests cover rejection limits, cache reuse, and cleanup after extraction failure.
Depth inputs and cache writes
recon/recon/depth.py, recon/models/moge_depth.py, recon/tests/test_review_debt.py, recon/tests/test_worker.py
LiDAR loading requires confidence data, image loading ignores EXIF orientation, and MoGe cache reads validate depth data under a lock. Depth writes use a temporary archive and replace the target after writing. Tests cover cache recovery, locking, and input requirements.
Coverage geometry and scene outputs
recon/recon/coverage.py, recon/recon/pipeline.py, recon/recon/scene.py, recon/HANDOFF.md, recon/tests/test_review_debt.py, recon/tests/test_worker.py
Coverage uses an optional fitted ground plane and revised cell and clearance bounds. Pipeline runs reject input-output overlap and remove owned outputs before loading the capture. Scene builds replace target-wall measurements and reject more than 500 observations. Regression tests cover these behaviors. The handoff updates its defect statements and describes the added tests.

Verification suite

Layer / File(s) Summary
Audit and simulator execution
verification/hsverify/a11yaudit.py, verification/hsverify/simrun.py, verification/tests/test_a11yaudit.py, verification/tests/test_simrun.py
Audit success now requires a zero process return code, a success marker, and no failure marker. Simulator app copies use unique directories, are cleaned up on failure or completion, and run under a device lock. Tests cover result checking, cleanup, and locking.
End-to-end and result validation
verification/hsverify/e2e.py, verification/hsverify/resultcheck.py, verification/scenes/eth3d-facade/build_scene.py, verification/tests/test_e2e.py, verification/tests/test_resultcheck.py, verification/tests/test_build_scene.py
End-to-end checks require one canonical scene.json and pass status for every scene. Assumption checks receive configured rules. Result checks sample sweep starts and compare geometric assumptions with rule values. Facade coverage records exact wall intervals. Tests cover the updated checks.
Replay checks and reporting
verification/hsverify/replaycheck.py, verification/hsverify/gitref.py, verification/hsverify/scoreboard.py, verification/scoreboard/metrics.yaml, verification/tests/test_replaycheck.py, verification/tests/test_gitref.py, verification/tests/test_scoreboard.py
Replay validation handles numeric overflow and calculates pose accuracy only after format validation succeeds. Git fetch prunes stale refs. PR listing requests a larger result limit, and S2-8 requires all_passed for validity. Tests cover these changes.
Root suite integration
Makefile, tests/makefile.sh
make check conditionally includes verification when its Makefile exists. A verification target runs the suite tests, and dispatch tests cover suite presence and failure propagation.

Packet documentation

Layer / File(s) Summary
Proposal and validator wording
packet/README.md, packet/validator-checks.md
The estimated-depth proposal specifies sigma-map validation. The photo-span proposal updates its ETH3D measurement description and figures. The mesh validator wording requires classification when mesh.classified is true.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 11945

The ZIP probe behaves as required, and no identified issue prevents merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 17.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 110 functions across 25 files. (5 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed The description links the related pull requests and review findings, including PRs #13, #20, #22, #179, #240, and #242.
Out of Scope Changes check Passed The changes match the stated objectives. The shared Makefile and packet documentation updates are explicitly justified as related work.
Title check Passed The title clearly summarizes the primary changes: preserving reconstruction and verification evidence through pipeline repairs.
Description check Passed The description is detailed and covers the required What changed, Why, Verification, Risk, and Checklist sections. It reports completed checks, limitations, risks, and non-applicable UI evidence. The …
Full details: Docstring Coverage

Explanation

Docstring coverage is 17.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 110 functions across 25 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@github-actions github-actions Bot added the size:L 100-499 effective changed lines. label Oct 13, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T21:46:13.415721Z 1428d04 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: b5da197b97

ℹ️ 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".

@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The latest changes appear safe to merge and fix the missing Markdown explanations.

Summary

This PR repairs reconstruction and verification evidence checks. The latest change fixes missing explanations in Markdown reports.

  • Capture archives and depth caches keep their data within clear limits.
  • Reconstructed scenes keep measurements tied to the rebuilt wall.
  • Verification checks reported rules and case assumptions before judging results.
  • The verifier counts geometry work and avoids repeating expensive checks.
  • Verification reports distinguish a full successful run from partial evidence.
  • Verifier runs serialize simulator use and require complete operator evidence.
  • Packet proposals spell out uncertainty-map and mesh-class requirements.

Reviews (12) · Last reviewed commit: "fix(verification): preserve Markdown exp..." · Reviewed by Greptile

Comment thread recon/recon/depth.py
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 1194523580

ℹ️ 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".

SamGu-NRX added a commit that referenced this pull request Oct 14, 2026
SeenFromTwo.observed rebuilt the frames-by-frames distance matrix on every
slab query, and free_space makes thousands. It now builds it once per
reconstruction and passes it to two_positions. On a synthetic 20 m clear
wall with 150 frames the free-space pass drops from 16.0 s to 10.7 s with
identical clearances. The rest is projecting each slab into every frame;
batching that means reshaping free_space's march, which #240 rewrites.

#242

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@greptileai review this at apex, and go beyond the diff. Read the full files this PR touches, plus the code that calls into them and the code they call, and flag real bugs you find anywhere in that wider area, not only in the changed lines. Include problems that were already there before this PR. Skip style nits.

Guard the clipped midpoint retained when combining #240 and #242; a visibility predicate rejects any probe beyond the fitted wall.
Comment thread recon/recon/capture.py Outdated
Comment thread verification/scoreboard/metrics.yaml
Comment thread verification/hsverify/resultcheck.py Outdated
Comment thread Makefile
Bind ZIP admission to the footer stdlib will parse and refuse ZIP64 overrides before metadata allocation. S2-8 requires execution of all five hostile probes, and geometric fixtures declare the public rule values their expectations depend on.

The root verifier target now uses the locked dependency sync from CI. Tiny archive controls, shortened CLI reports, real-case overrides and dispatch failures cover the new review findings.
@github-actions github-actions Bot removed the size:L 100-499 effective changed lines. label Oct 14, 2026
SamGu-NRX added a commit that referenced this pull request Oct 14, 2026
Keep complete-cell and clipped-span repairs from #240 with two-view coverage and active-rule headroom from #242. Merge #240 first, then #242; #242 also contains #240 if merged first.
@github-actions github-actions Bot added the size:XL 500-999 effective changed lines. label Oct 14, 2026
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10f0fb1463

ℹ️ 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".

Comment thread verification/hsverify/resultcheck.py
Comment thread verification/hsverify/resultcheck.py Outdated
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68cb6859ea

ℹ️ 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".

Comment thread verification/hsverify/e2e.py Outdated
Comment on lines +166 to +169
scenes = [i for i in infos if i.filename == "scene.json"]
if len(scenes) != 1:
raise SystemExit(
f"{path}: expected exactly one canonical scene.json, found {len(scenes)}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Accept scene.json inside a single top-level folder

When --scene or --app-export points to a valid Finder-style bundle such as capture-2026-09-26/scene.json, this exact-root check exits before the bundle reaches the server. The API explicitly accepts this layout in server/api.py::_scene_folder, and server/tests/test_api.py::test_zip_with_scene_inside_one_top_level_folder protects it, so the verifier should select either the root file or the exact file inside one shared top-level folder while still rejecting ambiguous suffix matches.

AGENTS.md reference: AGENTS.md:L22-L22

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 717e6da8ff

ℹ️ 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".

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 67b10e5684

ℹ️ 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".

Comment on lines +962 to +966
valid_widths = [(source, width) for source, width in widths if finite(width) and width > 0]
if valid_widths and any(
abs(width - valid_widths[0][1]) > NUMBER_TOL_FT for _, width in valid_widths
):
problems.append("reported battery widths disagree")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Compare reported battery width with the active rules

When the server under test uses a stale or hard-coded battery width but reports that same value in both the spot and wall_backing check, this block verifies only internal agreement and never compares it with rules.width_ft loaded from the tested ref. The new rules_assumed.battery_width metadata also compares the fixture with those rules rather than with the response, so a server can pass the verifier while evaluating an undersized battery; include the active rule width in this comparison.

AGENTS.md reference: AGENTS.md:L32-L34

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread recon/recon/depth.py
Comment on lines +220 to +224
# ZIP corruption can name an unsupported compression method.
NotImplementedError,
zipfile.BadZipFile,
zlib.error,
):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recompute caches that claim encryption

If corruption flips the encrypted flag in an NPZ central-directory entry, np.load raises RuntimeError (for example, that a password is required), but this repair block does not catch it. The damaged cache therefore remains in place and every subsequent reconstruction fails instead of deleting it and rerunning inference; handle this ZIP read failure alongside the other corruption exceptions.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

Comment thread verification/hsverify/resultcheck.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7b3169fef

ℹ️ 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".

Comment on lines +1792 to +1793
if values := reported_assumption_values(key, result):
changes[field] = values[0][1]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Verify reported dimensions against matching public rules

When --scene or --app-export supplies no rules_assumed metadata and the response claims the public policy with policy.rules_sha256 matching the tested ref, this unconditional replacement trusts a stale or hard-coded reported width instead of checking it against rules.width_ft. The server can therefore use and report the same incorrect width and still earn contract acceptance. Fresh evidence after a7b3169 is that reported_rules still overrides the active public rules without consulting the policy fingerprint; restrict overrides to private/different policies and compare matching-public responses with the loaded rules.

AGENTS.md reference: AGENTS.md:L33-L33

Useful? React with 👍 / 👎.

@SamGu-NRX SamGu-NRX Oct 14, 2026 •

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep loaded dimensions when the response claims the tested public fingerprint, including transformed responses. Reject malformed local case fields without losing the report.

Reviews: #240 (comment) #240 (comment)
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: db1e4c2f93

ℹ️ 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".

Aggregate expectation failures with bounded examples and cap serialized diagnostic text across the report. Use exact string keys instead of adversarial numeric hashes. Make the pre-end sampling count explicit while preserving sampled starts.

Reviews: Astra resource addendum; #242 (comment) #242 (comment)
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: c32ee975f1

ℹ️ 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".

Comment thread verification/hsverify/diagnostics.py
Prioritize the Markdown-visible problems array before duplicate JSON classifications. Regressions retain the first explanation for 1,000 contract or expectation failures without exceeding the shared diagnostic budget.

Review: #240 (comment)
@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@codex review

SamGu-NRX added a commit that referenced this pull request Oct 14, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 1428d04732

ℹ️ 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".

@SamGu-NRX
SamGu-NRX merged commit 31e37f2 into alpha Oct 14, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant