-
Notifications
You must be signed in to change notification settings - Fork 0
qa: registration instrument — per-room paint-vs-collision agreement score + honest baseline (#1680) #1681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
qa: registration instrument — per-room paint-vs-collision agreement score + honest baseline (#1680) #1681
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,6 +234,29 @@ jobs: | |
| # per-cell classifier + tri-state gate + the VQA adjudicator exercised with an injected stub (no LLM / | ||
| # box / player). Shares this lane's Pillow+numpy stack; single-process. | ||
| run: python -m pytest qa/test_paint_coherence.py -q -p no:xdist | ||
| - name: Registration instrument cores (#1680 — per-room paint<->collision agreement; deterministic) | ||
| # The SHIP RULER (qa/registration_score.py): for each room, the screen-space agreement between the | ||
| # PAINTED-object mask (paint_coherence's per-cell coverage machinery) and the COLLISION set | ||
| # (walkmask + boxes-sidecar footprints, projected through the contract camera). Names the two | ||
| # navigation-truth defects per cell — invisible walls (painted open, collides) and walk-through | ||
| # furniture (painted blocked, walkable). Deterministic synthetic-plate + pure-math cores; no LLM / | ||
| # box / player. Shares this lane's Pillow+numpy stack; single-process. | ||
| run: python -m pytest qa/test_registration_score.py -q -p no:xdist | ||
| - name: Registration BASELINE over every manifest room (#1680 — the honest paint<->collision score) | ||
| # The instrument run over ALL plates_manifest rooms. NON-BLOCKING (continue-on-error) like the | ||
| # coherence-freshness lint below: the baseline is EXPECTED RED today (per #1680 the owner's '98% | ||
| # mismatch' — every existing room is below the 0.99 ship bar; that is the finding, not a | ||
| # regression). The uploaded summary + per-room heatmaps are the acceptance ruler for the #83 | ||
| # 3D-first rebuild chain; a room only clears this once rebuilt. | ||
| continue-on-error: true | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In this CI context, Useful? React with 👍 / 👎. |
||
| run: python qa/registration_score.py score-rooms --evidence-dir "$RUNNER_TEMP/registration" -o "$RUNNER_TEMP/registration/summary.json" | ||
| - name: Upload registration baseline (summary JSON + per-room heatmaps) | ||
| if: always() | ||
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: registration-baseline | ||
| path: ${{ runner.temp }}/registration | ||
| if-no-files-found: warn | ||
| - name: Room-manifest DERIVATION from greybox geometry (regeneratable + loop-closed on forest_road) | ||
| # Owner playtest #5: manifests are DERIVED from greybox geometry (single source of truth for | ||
| # footprint + occlusion + walkable) via tools/derive_room_manifest.py. Pins the derivation math, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: continue-on-error masks harness-ERROR (exit 2) indistinguishably from expected-red (exit 1)
The 'Registration BASELINE' step has
continue-on-error: trueto accommodate the intentionally-red baseline. That is correct for the below-bar case (exit 1). But qa/registration_score.py defines a tri-state CLI where exit 2 means an indeterminate/harness ERROR (a room's plate/geometry/ortho/boxes could not be measured) and is explicitly 'never a pass'. With continue-on-error, GitHub marks the step yellow/x-passed-with-warning for BOTH exit 1 and exit 2, so a future regression that turns a manifest room unmeasurable (e.g., a cameraPin.ortho lost to 0, a renamed geometry stem, a missing boxes sidecar) would be indistinguishable from the expected-red baseline in the job's pass/fail signal. Consider gating the harness-ERROR path separately — e.g., have score_manifest_rooms write anerroredflag into the uploaded summary and add a tiny blocking follow-up step (if [ -f .../summary.json ]; then python -c "import json,sys; sys.exit(1 if json.load(open('...'))['errored'] else 0)") so an indeterminate batch is loud while the expected-red agreement score stays non-blocking. The blocking pytest step (test_score_manifest_rooms_smoke) mitigates a total measurement break, but only asserts at least one room is measurable, so a partial harness break (some rooms error, others still score red) would still slip through.Category: Flaky test risk
Why this matters: The whole point of the tri-state design is that a broken harness must never read green OR red — it must read ERROR. continue-on-error collapses that distinction at the CI-status layer, so the exit-2 guarantee the docstring promises is not actually visible to a reviewer looking at the check run.