CRYPT-RICH: denser crypt greybox (richness principle) — HONEST NEGATIVE (6.5 vs incumbent 8.0), evidence-only#1538
Conversation
…ple greybox Adds author_crypt_rich to tools/author_room_geometry.py: keeps the canonical layout contract EXACTLY (pillars (3,3)/(3,4)+(8,9)/(9,9), full 12-cell coffin cols3-7 x rows6-8, door zones (6,0)+(13,4) kept clear) and ADDS 11 ornamentation volumes the incumbent's 8.0 paint is rich in and the sparse 3-prop true-greybox (PR #1528, capped 7.1) lacked: carved wall reliefs/niches, engaged full-height column, lit torch brackets, rubble/broken-slab piles, skull-and-bone cluster, spilled funerary urn. All added props hug wall bands/corners; walkable topology flood-fill-verified connected (both doors + both spawns reachable). Derived manifest + cutaway greybox (wall_height=5, enclosed-room convention) + depth/normal sidecars under qa/evidence/crypt-rich/. Deterministic/offline; engine untouched.
…0 CU) candidate_final.png (gem_3): two knotwork columns, three tombs (two reclining effigies + figural relief panels), carved alcove, 4 lit torch brackets, skull/bone pile w/ rusted armor, spilled urn + coin basket, cobwebs, PoE2 warm-core/cool-periphery chiaroscuro. Honestly richer ornamentation than the incumbent while composition-faithful to the authored denser greybox. Gates: visual_pregate PASS (frame-lit+occupancy+floor-contact = the promotion hard floor). check_grid_paint_coherence INCOHERENT (diagnostic only; the 8.0 incumbent fails it identically). Recipe + exact prompts + all candidates under qa/evidence/crypt-rich/. Next: blind 5-scorer panel.
…2 candidate) Panel-1 (5 blind scorers): candidate(gem_3) median 6.5 vs incumbent 8.0 vs control(PoE2) 9.0 in-band. Convergent critique = cutaway greybox artifacts (toothed/crenellated walls read tiled/gamey + ~52% black empty margins read sparse); ornamentation itself landed. Refine pass (Gemini structure-lock, frame-fill + wall-solidify + deepen paint, best-of-3): candidate_final_v2.png (refine/ref_3) — frame filled edge-to-edge (black margin 52%->6%), solid continuous masonry (crenellation gone), ornamentation preserved. Pre-gate PASS. +60 CU (running total ~200/400).
Panel-2 (5 blind scorers, v2 refine candidate): candidate median 6.5 vs control(PoE2) 9.5 (advisory, >9.2) vs incumbent 8.0. Two panels now flat at 6.5 -> candidate does NOT reach the incumbent's 8.0 band. NOT adopted (adopting a 6.5 over the deployed 8.0 would be a regression). Logged both panels to qa/scores.db (vc-crypt_rich-panel1/panel2) + rendered ledger. gallery.html embeds greybox->base->enrich->refine->incumbent with the recipe lesson (cutaway crenellation artifact + carving-fidelity ceiling). Total Scenario spend ~200 CU (cap 400). No training.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 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 (35)
📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #1538 - CRYPT-RICH: denser crypt greybox (richness principle) — HONEST NEGATIVE (6.5 vs incumbent 8.0), evidence-only evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1538 Review URL: #1538 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #1538 - CRYPT-RICH: denser crypt greybox (richness principle) — HONEST NEGATIVE (6.5 vs incumbent 8.0), evidence-only
Head: 22b3f01f273ebca2e8e90e8fddda1a5f20fcd74c 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/crypt-rich/RECIPE.md |
added | +37/-0 | Documentation | Moderate: validated P3 finding |
qa/evidence/crypt-rich/base/base_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base/base_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base/base_3.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/a_cut_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/a_cut_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/a_cut_3.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/b_tall_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/b_tall_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/b_tall_3.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base_prompt.txt |
added | +3/-0 | Changed file | Low |
qa/evidence/crypt-rich/candidate_final.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/candidate_final_v2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/candidate_flux.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_depth.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_geometry.json |
added | +1/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_greybox.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_greybox_tall.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_normal.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gallery.html |
added | +46/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_1_raw.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_2_raw.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_3.png |
added | +0/-0 | Changed file | Low |
21 additional changed files omitted from this walkthrough.
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 0, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Docs only: 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: #1528, #1508.
Suggested labels: docs.
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.
|
|
||
| ## 1. Geometry (denser) | ||
| tools/author_room_geometry.py crypt_rich -> qa/evidence/crypt-rich/crypt_rich_geometry.json | ||
| - Canonical layout kept EXACTLY: pillars (3,3)/(3,4)+(8,9)/(9,9); full 12-cell coffin cols3-7 x rows6-8; |
There was a problem hiding this comment.
P3: "Canonical layout kept EXACTLY" contradicts the deliberate 12-cell sarcophagus expansion
RECIPE.md line 9 states 'Canonical layout kept EXACTLY', but the canonical crypt_geometry.json sarcophagus is a 2x2 (4-cell) block at [[5,6],[6,6],[5,7],[6,7]], while crypt_rich_geometry.json expands it to a 12-cell block [[4,6],[5,6],[6,6],[7,6],[3,7],[4,7],[5,7],[6,7],[7,7],[4,8],[5,8],[6,8]]. The expansion is honestly disclosed two lines later ('full 12-cell coffin cols3-7 x rows6-8'), so the geometry is intentional — only the 'kept EXACTLY' wording is self-contradictory. Reword to 'canonical pillar/door layout kept; sarcophagus intentionally expanded to a 12-cell coffin (cols3-7 x rows6-8)' so the evidence record is internally consistent.
Category: Docs only
Why this matters: This evidence lane exists to be a defensible, reproducible record of a HONEST-NEGATIVE art experiment. An internal contradiction between a 'kept EXACTLY' claim and a deliberate 12->4 cell expansion (even if benign here) weakens the record's auditability for future readers who may treat the geometry file as a canonical-room variant rather than an experimental expansion.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22b3f01f27
ℹ️ 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".
| ("torch_door_l", "brazier", [[5, 1]]), # lit torch bracket, left of the top door | ||
| ("torch_door_r", "brazier", [[7, 1]]), # lit torch bracket, right of the top door |
There was a problem hiding this comment.
Move torch props out of the top door zone
When crypt_rich is used as authored geometry, these two prop footprints fall inside the protected Chebyshev-1 zone for door_cells=[[6,0], ...]: [5,1] and [7,1] are both in the [6,0] door ring. servers/engine/scene_grid.py's door_zone_cells/validate_scene_grid contract rejects any prop footprint in that ring, so the generated geometry violates the door-zone/pathing gate despite the recipe saying the door zones are kept clear.
Useful? React with 👍 / 👎.
| ("urn_spill", "barrel", [[11, 9]]), # tipped funerary urn spilling coins | ||
| ] | ||
| geo = _geometry(sc.GRID_W, sc.GRID_H, "ancient stone", props, perimeter=True, | ||
| door_cells=[[6, 0], [13, 4]]) |
There was a problem hiding this comment.
Use the canonical tavern doorway cell
The current two-door crypt contract in qa/seed_gfx_walkslice.py defines the tavern door as [0,5], and server.cross_door resolves multi-door hubs by door_cells[i] -> connections[i]. Emitting [13,4] as the second crypt_rich door would advertise the tavern transition on the right wall instead of the canonical left-wall doorway, so any adoption or replay using this geometry would render/cross the wrong door.
Useful? React with 👍 / 👎.
| 6.8, | ||
| 9.2 | ||
| ], | ||
| "control_in_band": false, |
There was a problem hiding this comment.
Rerun or exclude the out-of-band P2 panel
For the v2 refine evidence, the control median is outside the stated 6.8–9.2 validity band and this field records false. I checked the room-pipeline runbook's blind-panel section, which treats out-of-band controls as advisory and says to rerun once; keeping this as a normal scores.db/ledger panel and using it to support the “flat at 6.5 across both panels” conclusion makes the v2 result non-citable until the control-valid rerun exists or the row is clearly excluded.
Useful? React with 👍 / 👎.
…lied crypt-rich panel rows
evaOS review status: completedPR: #1538 - CRYPT-RICH: denser crypt greybox (richness principle) — HONEST NEGATIVE (6.5 vs incumbent 8.0), evidence-only evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1538 Review URL: #1538 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #1538 - CRYPT-RICH: denser crypt greybox (richness principle) — HONEST NEGATIVE (6.5 vs incumbent 8.0), evidence-only
Head: ce18b4a8012b674f783adc6f98940701f2064f95 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/crypt-rich/RECIPE.md |
added | +37/-0 | Documentation | Low |
qa/evidence/crypt-rich/base/base_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base/base_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base/base_3.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/a_cut_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/a_cut_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/a_cut_3.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/b_tall_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/b_tall_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base2/b_tall_3.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/base_prompt.txt |
added | +3/-0 | Changed file | Low |
qa/evidence/crypt-rich/candidate_final.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/candidate_final_v2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/candidate_flux.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_depth.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_geometry.json |
added | +1/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_greybox.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_greybox_tall.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/crypt_rich_normal.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gallery.html |
added | +46/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_1.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_1_raw.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_2.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_2_raw.png |
added | +0/-0 | Changed file | Low |
qa/evidence/crypt-rich/gemini/gem_3.png |
added | +0/-0 | Changed file | Low |
21 additional changed files omitted from this walkthrough.
Review Signal
Validated inline findings: 4 (P0: 0, P1: 0, P2: 2, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- API compatibility: 1
- Migration: 1
- Proof gap: 2
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: #1528, #1508.
Suggested labels: docs.
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.
| "members": { | ||
| "candidate": "qa/evidence/crypt-rich/candidate_final.png", | ||
| "incumbent": "qa/evidence/crypt-replicate/refs/incumbent_crypt_armb_iter3.jpg", | ||
| "control": "/Volumes/LEXAR/WorldOS-Unity-spike/refs/poe2_ruins_brazier_integration_01.jpg" |
There was a problem hiding this comment.
P2: Control image referenced by absolute /Volumes/LEXAR path (non-reproducible evidence)
Both panel_key.json (line 18) and panel2_key.json (members.control) point the disguised PoE2 control at /Volumes/LEXAR/WorldOS-Unity-spike/refs/poe2_ruins_brazier_integration_01.jpg — an absolute path on the operator's Mac that does not exist in CI, on the support VM, or for any other reviewer. The incumbent and candidate are repo-relative (qa/evidence/...), so the control should be too. If the control file is not committed (it isn't — there's no diff for it), the panel is not independently reproducible: no one can re-run panel_verdict_p1.json/p2.json and confirm the control anchor (8.0) or the in-band check. Either commit the control image under qa/evidence/... and rewrite members.control to a repo-relative path, or document in RECIPE.md that the control is operator-local-by-design and out of repo scope. This matters because the PR's entire claim rests on the control-anchored blind-panel protocol.
Category: Proof gap
Why this matters: The HONEST NEGATIVE verdict depends on a disguised control anchoring the panel at 8.0. If the control cannot be located or hashed by a reviewer, the 8.0 anchor and the in-band validation are unverifiable — weakening the evidence trail the ledger row cites.
| ], | ||
| "noise_law": 1.2, | ||
| "members": { | ||
| "candidate": "qa/evidence/crypt-rich/candidate_final_v2.png", |
There was a problem hiding this comment.
P2: panel2_key.json drops control_file field present in panel_key.json (inconsistent schema)
panel_key.json records both control_file: "poe2_ruins_brazier_integration_01.jpg" (bare filename) and members.control (absolute path). panel2_key.json omits control_file entirely and only carries the absolute path in members.control. Same panel protocol, two different schemas. Any consumer that keys off control_file (e.g. a gallery builder or an audit script iterating panel keys) will silently miss the control identity for panel 2. Pick one shape and use it for both, or document the field as optional. Low runtime impact (evidence-only), but it's exactly the kind of drift that breaks later reproducibility audits.
Category: Migration
Why this matters: Inconsistent evidence schema across two panels from the same run makes automated panel-key validation brittle and invites a future audit to flag panel 2 as malformed.
| }, | ||
| "control_band": [ | ||
| 6.8, | ||
| 9.2 |
There was a problem hiding this comment.
P3: Control at 9.5 is out of the declared [6.8, 9.2] band — panel-validity caveat only in a note field
panel_verdict_p2.json reports control_in_band: false with control median 9.5 against a declared control_band: [6.8, 9.2]. The PR text and gallery correctly flag this as advisory, and it does not rescue the HONEST NEGATIVE verdict (candidate 6.5 < incumbent 8.0 unambiguously on both panels). However, the standard control-anchored panel protocol treats an out-of-band control as a panel-validity failure; here it is downgraded to an advisory solely via a free-text note. If the repo has any panel-validation tooling that reads control_in_band, panel 2 would be flagged invalid. Confirm whether the panel protocol permits control_in_band: false panels to be logged as evidence at all, or whether panel 2 should be marked calibration-invalid and excluded from the ledger row count.
Category: Proof gap
Why this matters: Logging an out-of-band control panel as a scored evidence row (rather than a calibration-invalid discard) may violate the panel protocol the rest of the lane relies on, even though it doesn't change this PR's negative outcome.
|
|
||
| > Rows: **107** · rendered 2026-07-10T22:39:31+00:00 | ||
| > Rows: **109** · rendered 2026-07-10T22:53:02+00:00 | ||
|
|
There was a problem hiding this comment.
P3: Two new ledger rows carry no SHA while adjacent rows are SHA-stamped
The two new rows (vc-crypt_rich-panel1-gem3-20260711, vc-crypt_rich-panel2-refine-20260711) leave the SHA column blank, whereas surrounding visual-critic rows (e.g. crypt-replicate-iter3-recipe-a/b) carry sha=c650b70a in Notes and the tavern row carries 3f8698d8 in the SHA column. For a visual-critic panel the SHA is arguably the code-genesis SHA of the candidate image pipeline, not a gameplay build SHA — but the recipe (RECIPE.md) records asset IDs (asset_rYC4EXay2XdSjACLmPFNuLgj, asset_ssap7oAn9ZvywsBFTuL9t94i) that could serve as the reproducibility key. Either stamp the SHA column with the head SHA (ce18b4a) or record the asset IDs in Notes so the row is traceable. Evidence-only, no runtime impact.
Category: API compatibility
Why this matters: Blank SHA on scored ledger rows breaks the traceability contract the ledger header implies and makes future RRI/lens-fenced comparisons unable to pin these rows to a code state.
…run (#1538) (#1539) 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. Co-authored-by: Eva <arncalso@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce18b4a801
ℹ️ 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".
| geo = _geometry(sc.GRID_W, sc.GRID_H, "ancient stone", props, perimeter=True, | ||
| door_cells=[[6, 0], [13, 4]]) |
There was a problem hiding this comment.
Punch crypt-rich door cells out of the wall set
When crypt_rich is emitted with perimeter=True, _geometry has already added the entire perimeter to walls/impassable; passing door_cells here only records the coordinates and does not remove [6,0] or [13,4] from the blocked set. The committed geometry therefore advertises two doorways that are still solid walls, so the scene-grid validator would report both door cells as blocked and the derived manifest omits them from walkable, making the camp/tavern crossings unusable if this geometry is reused or adopted.
Useful? React with 👍 / 👎.
| ("torch_door_l", "brazier", [[5, 1]]), # lit torch bracket, left of the top door | ||
| ("torch_door_r", "brazier", [[7, 1]]), # lit torch bracket, right of the top door | ||
| ("niche_back_r", "altar", [[10, 1], [11, 1]]), # recessed wall niche / 2nd tomb slab | ||
| ("pilaster_arch", "stone_pillar", [[12, 3]]), # engaged full-height column beside the door |
There was a problem hiding this comment.
Move the pilaster out of the right-door landing
With the current second doorway at [13,4], the protected Chebyshev-1 door zone includes [12,3], so this pilaster_arch footprint sits directly in the tavern-door landing. Even after the doorway cell itself is made walkable, validate_scene_grid rejects props in that door ring, so this authored geometry still violates the “door zones kept clear” contract; this is separate from the already-commented top-door torch placements.
Useful? React with 👍 / 👎.
| "slot_to_role": { | ||
| "A": "candidate", | ||
| "B": "incumbent", | ||
| "C": "control" |
There was a problem hiding this comment.
Drop the committed PoE2 control slot
This panel maps slot C to the shipped PoE2 control, and the same commit adds qa/evidence/crypt-rich/panel/slot_C.jpg (also duplicated as panel2/slot_B.jpg) into the repo. Other panel verdicts for this exact poe2_ruins_brazier_integration_01 control explicitly keep it as a local, not-committed copyrighted ref, so committing the disguised control image here leaks third-party reference art into the repository; keep only the local path/provenance and remove the copied control slots.
Useful? React with 👍 / 👎.
CRYPT-RICH — honest attempt to beat the crypt incumbent (panel 8.0) via the geometry-richness principle
Verdict: HONEST NEGATIVE — evidence-only PR, NOT adopting. Two blind 5-scorer panels put the candidate at 6.5 vs the deployed incumbent
crypt_armb_iter3at 8.0. Adopting a 6.5 plate over the deployed 8.0 would be a regression, sopromote.py --batchwas NOT run and the incumbent is preserved. This PR lands the geometry + full evidence + the recipe lesson.What this validated (and where it hit a wall)
The #1528 crypt-replicate attempt capped at 7.1 because its greybox was the sparse 3-prop true-greybox. Per the richness principle (epic #1508), I authored a denser 14-prop crypt greybox — canonical layout kept EXACTLY (pillars (3,3)/(3,4)+(8,9)/(9,9), full 12-cell coffin cols3-7×rows6-8, door zones (6,0)+(13,4) kept clear, walkable topology flood-fill-verified connected) plus 11 added ornament volumes (carved wall reliefs/niches, engaged full-height column, lit torch brackets, rubble, broken slabs, skull/bone cluster, spilled funerary urn).
The geometry-richness principle held for CONTENT — the denser greybox did produce a denser painted crypt (two effigy tombs, two knotwork columns, skull piles, torch brackets, spilled urn all rendered). But content density alone did not beat the incumbent's superior painterly carving craft — the candidate stayed flat at 6.5 across both panels.
Panels (blind 5-scorer, disguised in-band PoE2 control + incumbent anchor)
Deterministic pre-gate (frame-lit + occupancy + floor-contact) PASSES on both candidates.
Visual Evidence
Denser authored greybox (14 props) — the depth-CN control image:

Panel-1 candidate (flux depth-CN base + interior LoRA → Gemini structure-lock enrich) — median 6.5:

Panel-2 candidate (v2 refine: frame-fill + wall-solidify + deepen paint) — black-margin 52%→6%, crenellation removed, but median still 6.5:

The bar it did not clear — incumbent

crypt_armb_iter3(median 8.0, crisper hand-painted Celtic carvings):Recipe lesson (for the runbook / epic #1508)
wall_height=5) renders perimeter walls as per-cell boxes → a toothed/crenellated wall-top that the depth-ControlNet paints as a repetitive tiled/gamey motif (panel-1's convergent defect, alongside ~52% flat-black margins reading "sparse"). A second Gemini "frame-fill + wall-solidify" pass removes both (black-margin fraction 52%→6%, measurable) — but it is a post-hoc patch, not registration-by-construction.check_grid_paint_coherence.pyreports INCOHERENT on painterly plates — but the 8.0 incumbent fails it identically (pillar_l 1.03 off, pillar_r + sarcophagus not-locatable), so it is a diagnostic, not the promotion floor (which isvisual_pregate.py+ the blind-panel delta).Artifacts (all under
qa/evidence/crypt-rich/)crypt_rich_geometry.json(via newauthor_room_geometry.py crypt_rich) + derivedqa/room_manifests/crypt_rich.cells.jsonbase/,gemini/,refine/; panelspanel/+panel2/(+ keys); verdictspanel_verdict_p1.json/panel_verdict_p2.jsonRECIPE.md,base_prompt.txt,style_pass_prompt.txt,gallery.htmlvc-crypt_rich-panel1/panel2(+ rendered ledger)CU spent: ~200 CU (base 26 + tighter A/B 54 + Gemini enrich 60 + refine 60) of the 400 cap. No model training. GEX44 box untouched; only my own worktree used.
Runbook gaps hit (cold-agent test of
docs/ROOM-PIPELINE-RUNBOOK.md)referenceImages[0]— the runbook should clarify that passing the base-being-edited (minted from the greybox) is the sanctioned case, distinct from an external style anchor.wall_heightfor enclosed rooms; the cutaway convention (wall_height=5) is only implied by the tavern example, and its crenellation artifact (lesson 1 above) is undocumented.visual_pregate.py, but step 5 emphasizescheck_grid_paint_coherence.pywithout noting it is NOT the promotion floor (and that even the incumbent fails it) — a cold agent can misread it as blocking.