qa: flavor keys must not name generator-owned features (altar leak from #1626 review)#1629
Conversation
…tend the flavor guard (post-merge review of #1626) The #1626 guard checked flavor keys for structural KEYWORDS but let room-specific feature NOUNS through gemini_flavor: room_0's still asserted 'a carved stone ALTAR'. That's the exact contradiction class #1619 kills — a future altar-less room_0 variant would carry a flavor asserting the altar while the generated NEGATIVE denies it (conflicting prompt = the measured invention class). Altar presence/position is generator-owned (FOCAL/NEGATIVE clauses); the guard now asserts no flavor key names it. 23/23.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 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 (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #1629 - qa: flavor keys must not name generator-owned features (altar leak from #1626 review) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1629 Review URL: #1629 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #1629 - qa: flavor keys must not name generator-owned features (altar leak from #1626 review)
Head: 1ffb3e95564047bb0fe1f0c58363eeeca3cbf2d1 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~14 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
qa/test_render_recipe.py |
modified | +9/-0 | Test coverage | Moderate: validated P3 finding |
qa/unified_paint_recipes.json |
modified | +2/-2 | Changed file | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 0, P3: 3).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- CI/build: 1
- Proof gap: 1
- 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: #1626, #1624, #1625, #1619.
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.
…+ trailing newline + guard-scope docs (evaOS #1629 P3 trio) The hedge — not the altar mention — was the measured bug (a shareable clause that invited altars into altar-less rooms); room_0 HAS an altar so the legacy static now asserts it plainly. Guard scope documented: flavor keys of geometry-driven classes; statics are the no-geometry fallback.
evaOS review status: completedPR: #1629 - qa: flavor keys must not name generator-owned features (altar leak from #1626 review) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1629 Review URL: #1629 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #1629 - qa: flavor keys must not name generator-owned features (altar leak from #1626 review)
Head: ba898d8bf11383d6f1a895f17480720fdfab6447 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~14 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
qa/test_render_recipe.py |
modified | +13/-0 | Test coverage | Moderate: validated P3 finding |
qa/unified_paint_recipes.json |
modified | +2/-2 | Changed file | Low |
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
- Proof gap: 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: #1626, #1624, #1625, #1619.
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.
…hand-maintained list is the freehand pattern this suite kills; the guard now auto-extends to any future flavor-carrying class)
evaOS review status: closed or merged before reviewPR: #1629 - qa: flavor keys must not name generator-owned features (altar leak from #1626 review) evaOS review stopped because the PR closed or merged before this queued head could be reviewed. Automation note: agents should wait for this comment to reach PR URL: #1629 Details: state=closed |
… under the proven lock library (lit-fire, exact-count, single-level, anti-invention, treeline-not-masonry grounding); brazier vocabulary matches geometry kinds (#1629 class)
Post-merge adversarial review of the Kimi wave (#1624/#1625/#1626) — overall verdict: HIGH QUALITY,
all three verified against ground truth (regen byte-identical; generated clauses preserve every
proven lock; 58/58 suites; err_cells gate intact; NOW.md/onboarding accurate). One real gap found:
dwing_room_0.gemini_flavorstill asserted "the wide low mass near them is a carved stone ALTAR" —room-SPECIFIC content in a hand-authored flavor key. If a future room_0 variant has no altar, the
flavor asserts one while the generated NEGATIVE clause denies it: a self-contradicting prompt,
which is the measured Gemini invention class #1619 exists to kill. The #1626 guard checked for
structural KEYWORDS but not feature NOUNS.
Fix: the sentence is stripped (the generated FOCAL clause names the altar + position
authoritatively for rooms that have one), and the guard now asserts no flavor key names an altar
in any class carrying flavor keys. 23/23.