docs(runbook): fold in the three cold-agent gaps from the CRYPT-RICH run#1539
Conversation
…run (#1538) 1. Enclosed rooms: cutaway wall_height convention made explicit + the two measured artifacts (per-cell wall boxes -> crenellated depth -> 'tiled/ gamey' paint; over-dark frames) with the continuous-wall-band rule. 2. Reference-images LAW: clarify the img2img base-in-referenceImages[0] is the sanctioned required case; the LAW governs EXTERNAL anchors. 3. Coherence gate: scope note — diagnostic instrument, not the promotion floor; incumbents FLAG identically; blocking = candidate regressing cells the incumbent passes.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 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 selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #1539 - docs(runbook): fold in the three cold-agent gaps from the CRYPT-RICH run evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1539 Review URL: #1539 (review) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f739fe0ca
ℹ️ 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".
| **ENCLOSED ROOMS need a CUTAWAY greybox (tavern finding, PR #1531; codified from the CRYPT-RICH | ||
| cold-agent run, PR #1538):** full-height near walls poison the interior's depth/lighting read (NCC | ||
| collapses, the frame goes mostly black). Author enclosed interiors with `wall_height ≈ 5` on the | ||
| near/south walls (the camera-facing side stays open, the back walls keep height). Two artifacts to |
There was a problem hiding this comment.
Pass the cutaway height in the render examples
With this new requirement, the runnable commands in step 3 still render qa/greybox_render_headless.py ... and qa/greybox_sidecars_headless.py ... without --wall-height; those CLIs default wall_height to 9.0, so an enclosed-room agent following the runbook for crypt/tavern will produce the full-height near-wall/black-frame base this paragraph says to avoid. Please make the examples include --wall-height 5 or add an explicit enclosed-room branch so the documented cutaway is actually exercised.
Useful? React with 👍 / 👎.
| - **Crenellation:** authoring wall bands as PER-CELL boxes gives the depth map toothed/castellated | ||
| wall-tops → the depth-CN paints a "tiled/gamey" motif panels penalize. Author each wall run as | ||
| ONE CONTINUOUS box spanning its cells, not a stack of cell-sized boxes. |
There was a problem hiding this comment.
Define continuous walls in a supported form
This tells authors to avoid per-cell wall boxes, but the geometry contract immediately below still represents walls as cells, and the current headless renderer turns every geo["walls"] cell into its own 1-cell box. A cold agent using tools/author_room_geometry.py for an enclosed perimeter will therefore still generate the crenellated control image unless the runbook says to model these runs as multi-cell wall props (or the renderer is changed to coalesce wall cells).
Useful? React with 👍 / 👎.
| as a black-margin defect at panel time — check `visual_pregate.py`'s frame-lit gate on the BASE, | ||
| not just the styled result. |
There was a problem hiding this comment.
Use a gate that detects black margins
For the >40% black-margin case described here, G1_frame_lit is the wrong check: qa/visual_pregate.py::gate_frame_lit only fails near-black mean luminance, blown-out mean luminance, or flat variance, so a base with large black borders plus a lit interior can still PASS. That lets the exact over-dark artifact reach the panel; point agents at a margin/near-black coverage check (or add one) instead of treating G1 as sufficient.
Useful? React with 👍 / 👎.
| **Clarification (from the #1538 cold-agent run):** the LAW governs *external anchors*. Gemini | ||
| img2img mechanically requires the image being edited in `referenceImages[0]` — passing the | ||
| registered BASE you are styling (which was minted from this room's own greybox) is the sanctioned, | ||
| required case, not a violation. "No referenceImages" means: no SECOND image, no external style | ||
| anchor, no other room's plate. |
There was a problem hiding this comment.
Keep the edit input out of referenceImages
This redefines “no referenceImages” to still put the registered base in referenceImages[0], but the wired Gemini edit path in generate_room.py::_run_gemini_pass sends the image being edited as image, and the camp-armB evidence literally used a style pass WITHOUT referenceImages to avoid content hijack. Following this clarification sends cold agents back through the content-anchor field for no-anchor rooms; keep the base as the edit image and reserve referenceImages for actual anchors.
Useful? React with 👍 / 👎.
| **Scope note (from the #1538 cold-agent run):** this gate is a *diagnostic instrument*, not the | ||
| promotion floor — `promote.py`'s panel-delta gate is the floor. Organic/legacy-styled incumbents |
There was a problem hiding this comment.
Don't reduce promotion to the panel delta
promote.py::evaluate_visual_gate also fails a visual nomination when the deterministic pre-gate record is missing/failing or the registered control is out of band before it checks candidate-vs-control delta. Calling the panel delta “the floor” here can make agents treat a good delta as promotable even when the pre-gate/control requirements are absent; say the coherence gate is diagnostic, but keep the documented pre-gate + control + delta promotion floors intact.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Walkthrough
PR: #1539 - docs(runbook): fold in the three cold-agent gaps from the CRYPT-RICH run
Head: 9f739fe0cace2cbfbfe8273d0ded4c33b407c7d9 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~10 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
Documentation-only changed surface; runtime execution proof is not required by default.
- recommended: Docs/readme review - All reviewed files are documentation paths. Proof: PR description notes the docs intent.
- not_applicable: 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: not_applicable - No required behavior proof selected for this changed surface.
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: #1538.
Suggested labels: none.
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.
The tavern plate out-painted a room LARGER than its authored 12x10 grid (unreachable painted floor, invisible walls at grid edges; owner playtest #8). Root cause: the fixed ortho=13 greybox leaves canvas margins around the room and the style pass out-paints them into "more room". RECIPE half (fix by construction, strictly OPT-IN so existing rooms' math is byte-identical — the coherence/drift/journey instruments share the rig): - greybox_render_headless: CAMERA-FIT mode (--camera-fit / geometry field camera_fit:true) fits the ortho SCALE from the room's grid extent so the diamond + wall band fills the frame edge-to-edge. Only the ortho scale changes; cell_to_world/world_to_screen default to ORTHO_SIZE untouched. - author_room_geometry: explicit PERIMETER WALL BAND as continuous wall RUNS (kind wall_run, one box per edge run split at doors — the #1539 no-crenellation rule) so painted walls sit on impassable cells by construction and the door stays walkable. New tavern_fit reference room emits both; tavern stays as-is (pinned by engine-grid tests). HOTFIX half: tools/mask_plate_extent.py feathers everything outside the grid diamond + wall band to a dark vignette (deterministic, PIL-only). Masked the tavern plate into qa/evidence/1543/ (before/after) — no plates_manifest/deploy change; adoption is a separate orchestrator call. Compat: all room geometries + default greybox renders byte-identical vs main; engine walkslice/scene_grid (39) + qa coherence/drift/journey/ derive/dungen (39) + new extent-contract (9) tests green. Co-authored-by: Eva <arncalso@gmail.com>
Docs-only. PR #1538's lane was the first cold agent to run docs/ROOM-PIPELINE-RUNBOOK.md end-to-end and hit three ambiguities (its report §runbook-gaps); this folds the answers back in so the next cold agent doesn't: (1) enclosed-room cutaway convention + the crenellation/over-dark artifacts with the continuous-wall-band rule, (2) the img2img base-image clarification on the reference-images LAW, (3) the coherence-gate scope note (instrument, not promotion floor). No pixels changed — no visual evidence section; ground truth is #1538's panels + RECIPE.md.