VISUAL JOURNEY: screenshot-verified paint-truth navigation sweep (#1540, M-ALIGN)#1552
Conversation
…-truth navigation sweep Extends qa/journey_click_sweep.py (#1537) one layer up: engine-truth is blind to PAINT truth (the same RECALL.md 0/4 lesson — #1537 reported 133/0 clean the night the owner walked through painted tavern tables). Drives the walkslice 3-room world through every room + door round-trip on a SCRATCH viewer (port >=8767, never the owner's :8766) and verifies paint deterministically: 1. hero-position: contract-camera projection of the hero cell's feet lands in its floor quad AND not on an inverse-coherence-flagged cell (actor-inside-the-object). 2. reciprocal-door: after A->B, arrival must be within Chebyshev-2 of B's door back to A (#1541's red test). 3. inverse-coherence: hard-edge (thr 128) standing-silhouette density robust-z outliers over authored-walkable cells => invented-furniture flags (per-room CLEAN%). Emits report.json + gallery.html + per-room CLEAN% table. First real run (qa/evidence/1540): overall 91.6% (< the 95% bar), 25 invented-furniture flags, 1 reciprocal-door failure — tavern invented tables/stools, crypt sarcophagus drift, camp shelter/log/ruin on walkable cells, tavern->crypt dumped at the wrong door. Red-first: the checks fire on today's known defects. 13 pure-function unit tests (qa/test_journey_visual_sweep.py). Engine = SOLE WRITER (only rest-mode /move intents a real click makes). Additive: new files only.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #1552 - VISUAL JOURNEY: screenshot-verified paint-truth navigation sweep (#1540, M-ALIGN) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1552 Review URL: #1552 (review) |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import statistics | ||
| import subprocess | ||
| import sys | ||
| import time |
| n_furniture = n_hero = n_recip = 0 | ||
| for rec in room_recs: | ||
| loc = rec["room"] | ||
| hero_checks = rec.get("hero_checks", []) |
|
Architect review: APPROVED. New-files-only instrument, 13/13 unit tests, first-run baseline CLEAN% 91.6% (crypt 94.7 / camp 90.8 / tavern 88.3) with every finding class validated against known defects (invented furniture, effigy drift, reciprocal-door). The three deviations (headless composite protecting the owner's live window; calibrated hard-edge threshold + per-room robust-z; honest suspicion-detector framing) are all sound and documented. This is now the M-ALIGN gate: every wave-1/2 fix must move CLEAN% up. |
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #1552 - VISUAL JOURNEY: screenshot-verified paint-truth navigation sweep (#1540, M-ALIGN)
Head: 1b1c652a5298c518bee54782fe89325ac8928f45 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~70 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
qa/evidence/1540/frames/step00_spawn_crypt.png |
added | +0/-0 | Changed file | Low |
qa/evidence/1540/frames/step01_arrive_camp_clearing_night.png |
added | +0/-0 | Changed file | Low |
qa/evidence/1540/frames/step02_return_crypt.png |
added | +0/-0 | Changed file | Low |
qa/evidence/1540/frames/step03_arrive_tavern.png |
added | +0/-0 | Changed file | Low |
qa/evidence/1540/frames/step04_return_crypt.png |
added | +0/-0 | Changed file | Low |
qa/evidence/1540/gallery.html |
added | +30/-0 | Changed file | Low |
qa/evidence/1540/report.json |
added | +780/-0 | Changed file | Moderate: validated P3 finding |
qa/evidence/1540/seed.log |
added | +1/-0 | Changed file | Low |
qa/journey_visual_sweep.py |
added | +765/-0 | Changed file | Moderate: validated P2 finding |
qa/test_journey_visual_sweep.py |
added | +146/-0 | Test coverage | Low |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 1, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Docs only: 2
- Runtime correctness: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - WorldOS repo profile implies Unity runtime risk. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1540, #1537, #1541, #1466.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
Assets/**- Prioritize scene, prefab, save-state, asset-reference, and gameplay regressions. - Path instructions:
ProjectSettings/**- Treat build, platform, input, graphics, and release behavior changes as high risk. - Label suggestions: unity, gameplay, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
| {rows_html}</table> | ||
| <div class=cards> | ||
| {''.join(cards)} | ||
| </div> |
There was a problem hiding this comment.
P2: _cross closure trusts cross_door 'ok' without verifying landed room (unlike journey_click_sweep._cross_door)
The injected _cross closure (run_live) posts walk_to_cell then cross_door and returns {"ok": bool(out.get("ok")), "reason": out.get("reason")}. It does NOT fetch the surface afterward to verify location.id == door.to, unlike the proven qa/journey_click_sweep.py::_cross_door (lines 341-357) which explicitly checks landed_room == expected. If the engine ever returns ok=True but lands the party in the wrong room (or no-ops the cross), run_journey records crossed:true with a misleading dest_id, then proceeds to run reciprocal/hero/inverse-coherence checks against whatever surface get_surface() returns next. The downstream checks are partly self-correcting (hero cell and reciprocal are read from the fresh surface), but the transition's 'from/to/crossed' label and the step's 'crossed X -> Y' caption would be wrong, and a no-op cross that leaves the party in the origin room would be reported as a successful crossing. Reuse journey_click_sweep._cross_door's landed_room==expected guard (or assert the post-cross surface location.id equals door.to before recording crossed:true).
Category: Runtime correctness
Why this matters: This instrument's whole value is paint-truth evidence; a transition mislabeled as crossed when it silently failed would corrupt the reciprocal-door finding count and the per-step captions, silently weakening the red-test signal #1541 depends on.
| "walkable_floor_cells": 94, | ||
| "inverse_coherence_flags": 5, | ||
| "clean_pct": 94.7, | ||
| "meets_95": false |
There was a problem hiding this comment.
P3: Committed report.json embeds machine-local absolute plate paths (/Users/lume/...)
The 'plate' field of each room record is the absolute path the operator's machine resolved at capture time (e.g. "/Users/lume/WorldOS-worktrees/wt-visual-journey/qa/evidence/plate-audit/camp_clearing_night_truegrey_v1.png"). These paths will not exist on any other machine/CI and make the evidence artifact non-reproducible-looking. It is evidence-only (no runtime/CI impact), but for an artifact committed as proof, storing a repo-relative path (or just the basename, which resolve_plate already keys on) would keep the report portable and self-describing.
Category: Docs only
Why this matters: Evidence artifacts that hardcode a developer's home path read as environment-coupled and reduce trust in reproducibility, even though the underlying check is deterministic.
| } | ||
| } | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
P3: report.json has no trailing newline
The committed report.json ends with '\ No newline at end of file' in the diff. JSON parsers handle this fine, but POSIX text-file convention and many linters/formatters expect a terminal newline; if a future CI step normalizes/reformats this file it will show as a spurious whole-file diff. Add a single trailing newline when writing (json.dumps already; the write_text could append '\n').
Category: Docs only
Why this matters: Minor hygiene; prevents noisy future diffs if the file is ever touched by a formatter.
1 participant
Pull Request
Summary
The VISUAL JOURNEY instrument for milestone M-ALIGN — the one-layer-up sibling of
qa/journey_click_sweep.py(#1537). #1537 drives the engine's own move-validation surface and reported 133 targets / 0 defects clean the same night the owner walked a real player through painted tavern tables and a drifted crypt sarcophagus — because engine-truth is blind to paint truth (the same recall lesson asqa/evidence/journey-eval-first-run/RECALL.md's honest 0/4 table, one layer up).qa/journey_visual_sweep.pydrives the walkslice 3-room world (crypt hub ⇄ camp, crypt ⇄ tavern) through every room + every door round-trip on a scratch viewer (port ≥8767 — never the owner's :8766), captures a composite frame per step, and verifies paint deterministically:greybox_render_headlesscell_to_world/world_to_screen, the rig the 1344×768 plates are registered to) and asserts the feet land in the cell's floor quad and not on an inverse-coherence-flagged cell (the actor-inside-the-coffin incident).Plus a per-step composite frame →
gallery.htmland a machinereport.json.Linked Issue
Closes #1540
Tracker / Milestone
Changes
qa/journey_visual_sweep.py— the instrument (pure geometry/detector core + injected-IO driver, mirrors journey_eval v2: adversarial prop-click sweep (#1523) #1537's split; reuses journey_eval v2: adversarial prop-click sweep (#1523) #1537's seed/boot/teardown +manifest_from_surfaceverbatim).qa/test_journey_visual_sweep.py— 13 pure-function unit tests (no engine/HTTP): projection invariant, edge-density, inverse-coherence flag/exclude, reciprocal-door pass/fail/no-door, hero-position, CLEAN% arithmetic.qa/evidence/1540/— first real-runreport.json+gallery.html+ 5 annotated frames.Visual Evidence — the checks fire on today's known defects (red-first)
Overall CLEAN% = 91.6% (< 95%), 25 invented-furniture flags, 1 reciprocal-door failure. The instrument exits non-zero on today's world by design — that is the proof it is not blind.
Tavern — invented tables + stools on authored-walkable cells (orange = flagged; green = hero; cyan = door):
Crypt — the sarcophagus drift (the painted effigy/tomb rises onto walkable cells the grid authored clear — the exact M-ALIGN paint-vs-footprint drift):
Camp — painted shelter / fallen log / ruin wall overlapping walkable floor:
Reciprocal-door FAIL — tavern → crypt dumps the party at the camp door (8 cells / Chebyshev from the tavern return door at (13,4)): the multi-door crypt hub always lands arrivals at the first door. (#1541's red test — reported, not fixed here.)
Per-room: crypt 94.7% · camp 90.8% · tavern 88.3%.
Deviations from the issue text (flagged, not hidden)
WorldOSPlayer.appis unsafe (macOS window lookup by owner name can't disambiguate twoWorldOSPlayerwindows — it could grab the owner's live :8766 window; documented in journey_eval v2: adversarial prop-click sweep (#1523) #1537). The frame composites the same registered plate the player renders as its backdrop + the hero at the contract-camera projection of its engine cell — exercising exactly the paint-vs-grid contract the three checks name, zero risk to the owner's session. What a headless composite can't independently verify (does the shipped client's projection match this contract camera) is flagged per-step ashero_render_source: contract-projectionand deferred to a client-render check.Method honesty (RECALL discipline): inverse-coherence is a hard-edge robust-outlier suspicion detector — high recall on the clear cases (verified in the frames above), and it can flag walkable cells whose silhouette band overlaps a tall authored prop (the occlusion-vs-footprint nuance). It is paired with the frame gallery for human confirmation and complements
qa/check_grid_paint_coherence.py's authored-prop localisation gate; it is not claimed as a perfect binary oracle.Licensing / CLA
CLA.mdand submit this contribution under the WorldOS Contributor License Agreement.Review Thread State
Validation
python3 -m pytest qa/test_journey_visual_sweep.py -q→ 13 passed (single-process, PIL-only pure checks).python3 qa/journey_visual_sweep.py run --state-dir /tmp/visual_journey_scratch --rundir qa/evidence/1540→ overall 91.6%, artifacts written, scratch viewer torn down by PID; owner :8766 confirmed untouched.Release / WorldOS Proof Tier
Release Notes / Changelog
Safety / Rollback
Additive new files only (no existing symbol edited). Scratch-only state dir; viewer bound to port ≥8767, never :8766 / /tmp/walkslice_owner / the LaunchAgents; killed by PID. Rollback = revert the commit.
Evidence
qa/evidence/1540/(report.json, gallery.html, frames/).Notes For Next Agent
/clickchannel's screen-position report to upgrade hero-position fromcontract-projectionto a true client-render check when a scratch player build can be driven safely.