#1436 W5c U2: native-window T3 playtester palette + runner#1440
Conversation
Extend the blind-playtester harness to drive the native macOS WorldOSPlayer window for the T3 gate (#1322), exposing the SAME 9-tool contract as the browser palette but backed by macOS primitives instead of Playwright. - qa/native_palette/native_input.swift: CGWindowList window-find + CGEvent synthetic click/key/type + an empirical Screen-Recording/Accessibility perm probe (CGPreflight lies on macOS 14+/15+; we gate on normal-window visibility). - qa/native_palette/native_palette_server.js: MCP server; screenshot=screencapture -l <windowid>, click(x,y) window-pixels->global-points (Retina scale-aware), a11y_tree=pixels-only stub, report_bug/give_up/finish identical shape so qa/ui_playtest_score.py scores a native run UNCHANGED. Shares the qa/playwright MCP SDK (no second install). --selfcheck asserts the 9-tool contract GUI-free. - qa/ui_playtest_player.sh: seed_gfx_combat -> viewer -> WorldOSPlayer.app (env launch-contract) -> unchanged run_duo DM loop -> native palette player -> score. FAILS LOUD naming the exact System Settings pane on a missing permission; --preflight validates app+deps+swift+perms without a run. - qa/native_palette/play_player_native_t3.txt: the pixels-only quest-loop persona. - qa/test_native_palette.py: contract + runner-permission + swift-compile + server-boot + scorer-compat self-checks (GUI-free; CI/fast-gate green). Does NOT run the actual T3 gate (orchestrator's session; may need a perm grant). Refs #1436 #1322.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 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 (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
evaOS review status: completedPR: #1440 - #1436 W5c U2: native-window T3 playtester palette + runner evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1440 Review URL: #1440 (review) |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| // there). Resolve normally first (in case this dir grows its own node_modules), then fall back to the | ||
| // playwright workspace — no second npm install required. | ||
| function requireShared(mod) { | ||
| const parts = mod.split("/"); |
| // Used by qa/test_native_palette.py to prove the server boots and registers the 9-tool contract | ||
| // without a live player window or a TCC grant. | ||
| if (SELFCHECK) { | ||
| const registered = Object.keys((server && server._registeredTools) || {}); |
![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: #1440 - #1436 W5c U2: native-window T3 playtester palette + runner
Head: 1ff104d845284c9546ffd458687e4595942947f7 into main. Review event: REQUEST_CHANGES.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~62 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
qa/UI_PLAYTEST.md |
modified | +28/-0 | Documentation | Low |
qa/native_palette/native_input.swift |
added | +188/-0 | Changed file | Low |
qa/native_palette/native_palette_server.js |
added | +494/-0 | Changed file | Elevated: validated P1 finding |
qa/native_palette/play_player_native_t3.txt |
added | +66/-0 | Changed file | Low |
qa/test_native_palette.py |
added | +178/-0 | Test coverage | Moderate: validated P2 finding |
qa/ui_playtest_player.sh |
added | +332/-0 | Changed file | Elevated: large change |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 1, P2: 1, P3: 0).
Dropped findings before posting: 0. High-severity findings: 1.
Risk Taxonomy
- API compatibility: 1
- Release regression: 1
Validation and Proof
2 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.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: missing - 2 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: #1436, #1322, #1430, #1432, #1435, #1438, #1389.
Suggested labels: bug, docs, 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.
| // require.resolve({paths}) honors the package's "exports" map (the SDK maps | ||
| // "server/mcp.js" -> dist/cjs/...), which a raw path.join would bypass. | ||
| for (const root of roots) { | ||
| try { return require(require.resolve(mod, { paths: [root] })); } catch (_e2) {} |
There was a problem hiding this comment.
P1: Native runs always score passed=False (scorer requires 'table' screen, palette stamps 'player')
SCREEN_LABEL defaults to "player" (line 74) and is stamped on every action via logAction(). The shared scorer qa/ui_playtest_score.py computes reached_table = "table" in screens and passed = completed_intro_flow and critical==0 and console_errors==0 and satisfaction>=6, where completed_intro_flow requires reached_table. Because no native action ever carries screen=="table", reached_table is always False, so completed_intro_flow and passed are ALWAYS False for any native run — even one where the player fully completed the quest loop and finished with satisfaction 10. The T3 gate's entire purpose is proving a quest loop completes in the native window, but the gate verdict is structurally unreachable here. The code comments honestly justify the "player" label vs the SPA's "table", but never reconcile that this disables the pass gate. Fix one of: (a) have the native palette stamp screen="table" once the player is visibly in the rendered game view (e.g. after a successful in-game action), (b) add a native-aware pass branch to the scorer keyed on surface=="native-player" + in_story_turns>=1 + finish, or (c) translate screen labels in the scorer when meta.surface=="native-player". The added test test_score_reads_native_palette_artifacts deliberately does NOT assert passed/completed_intro_flow, masking this — add an assertion that a completing native run yields passed=True.
Category: Release regression
Why this matters: A release-readiness gate that can never return True for the exact surface it was built for makes every T3 run report failure regardless of evidence, undermining the proof the gate exists to produce and risking false release-blocking or silent acceptance of a broken metric.
| self.assertEqual(score["satisfaction_source"], "self-reported") | ||
| self.assertEqual(score["bug_reports_minor"], 1) | ||
| self.assertEqual(score["console_errors"], 0) | ||
| self.assertEqual(score["network_failures"], 0) |
There was a problem hiding this comment.
P2: Scorer compat test omits the pass/completed_intro_flow assertions that are the load-bearing contract
test_score_reads_native_palette_artifacts asserts persona, in_story_turns, persona_satisfaction, bug_reports_minor, console_errors, network_failures, and summary.md — but never asserts score['passed'] or score['completed_intro_flow']. Those are the fields the release gate keys on, and (per the screen-label mismatch) they will be False for this exact fixture. Without asserting them, the test proves the scorer PARSES native artifacts but not that it can ever PASS a native run. Add self.assertTrue(score['passed']) and self.assertTrue(score['completed_intro_flow']) (after fixing the screen label), or explicitly document why native runs are expected to be passed=False and how the T3 gate then exits.
Category: API compatibility
Why this matters: A compatibility test that skips the gating fields gives false confidence that the shared scorer works end-to-end for the native surface, hiding the release-regression above until a live run.
…lision-validated clicks (#1446) * #1441 W5d: grounded reposition + engine-path glide + walk clips + click pre-validation - PlaceActor -> UpdateActor: reposition now re-grounds feet->FloorY via the SAME BakeMesh Measure as SpawnActor (float fix; old path preserved raw Y). First-sight snap, changed-cell glide, same-cell no-op (poll never interrupts an in-flight glide). - GlideTo: cell->cell tween at GlideSpeed following the engine-confirmed lastPath polyline (straight-line fallback when absent); rings/AO follow every frame; walk clip during glide (own walk/run -> humanoid donor-walk retarget -> glide-only fallback), idle at rest. - Click pre-validation: reject clicks on impassable/occupied cells with a red ring flash (UX pre-filter only; engine stays authoritative — it already rejects illegal cells). - Reconcile actor heights to named consts (ActorHeightFoe 4.2 / ActorHeightChar 3.2, the paint_combat_v1 #1418-calibrated source); flag replay_v1's stale 5.0. - Parse impassable+lastPath via the runtime Json parser (poll + move-response paths). Refs #1441 #1436 #1322 (PRs #1438/#1440). * #1441 W5d: QA evidence — compile-clean, editor sanity capture, build+scp notes Box validation: 0-error compile (component compiled + attached), editor sanity screenshot (grounded actors + rings, painterly camp scene), Universal build Succeeded (0 errors), zip scp'd to ~/worldos-session-notes/w5a-build/WorldOSPlayer.app.zip (overwrite). --------- Co-authored-by: Eva <arncalso@gmail.com>
1 participant
What
Unit 2 of #1436 (W5c) / the T3 gate of #1322: extend the blind-playtester harness to drive the native macOS WorldOSPlayer window (not the browser), ready for the T3 quest-loop gate. Same 9-tool player contract as the browser palette, backed by macOS primitives instead of Playwright, scored by the same
qa/ui_playtest_score.py.Files
qa/native_palette/native_input.swift— Swift helper:CGWindowListwindow-find +CGEventsynthetic click/key/type + a permission probe.CGPreflightScreenCaptureAccessreturns a false positive on macOS 14+/15+ (observed on the dev box), so the Screen-Recording gate is empirical (normal-window visibility). Runs interpreted (swift …) and compiles (swiftc …).qa/native_palette/native_palette_server.js— the native palette MCP server.screenshot=screencapture -l <windowid>;click(x,y)maps window-relative screenshot pixels → global points (Retina scale-aware);a11y_tree= a pixels-only stub (the T3 persona plays from screenshots);type/key/wait/report_bug/give_up/finishmirror the browser palette byte-for-byte in artifact shape. Shares theqa/playwrightMCP SDK (no second npm install).--selfcheckasserts the 9-tool contract GUI-free.qa/ui_playtest_player.sh— the runner:seed_gfx_combat.py→viewer/server.py(/combat-surface+ move sink + chat) →WorldOSPlayer.applaunched with theWORLDOS_ENGINE_BASE_URL/WORLDOS_CAMPAIGN_IDlaunch-contract → the unchangedrun_duoDM loop → native-palette player agent → teardown → score. FAILS LOUD with the exact System Settings pane on a missing permission (never a silent skip).--preflightvalidates app + deps + swift helper + both permissions without a run.qa/native_palette/play_player_native_t3.txt— the pixels-only quest-loop persona brief.qa/test_native_palette.py— contract + runner-permission-discipline + swift-compile + server-boot + scorer-compatibility self-checks. GUI-free; swift/node subtests skip where the toolchain is absent, so CI/fast-gate stay green.qa/UI_PLAYTEST.md— a section documenting the native variant + the permission requirements.macOS permissions (owner action)
The palette needs Screen Recording (System Settings ▸ Privacy & Security ▸ Screen Recording — for the capture AND for
CGWindowListto enumerate the window) and Accessibility (▸ Accessibility — for synthetic input). Missing → the run aborts naming the pane.qa/ui_playtest_player.sh --preflightchecks both.Tests
python3 -m pytest -q qa/test_native_palette.py→ 11 passed (incl. swift compile + node MCP selfcheck + scorer-compat on this Mac).qa/fast_gate.sh→ 257 passed (green; additive, no engine touch).qa/ui_playtest_player.sh --preflight→ ALL GREEN on the dev box.qa/ui_playtest_score.pyscored the native output unchanged (in_story_turns=1, satisfaction=7 self-reported).Constraints honored
One live GUI harness at a time; never touches Eva; engine stays the sole writer; DM side unchanged (same
run_duoloop). Does NOT run the actual T3 gate — that is the orchestrator's session (a real playtest that may need a permission grant).Refs #1436 #1322. PRs #1430/#1432/#1435/#1438.
Admin-merge note (#1389): auto-merge armed, no manual merge; admin-merge is acceptable per the relaxed policy once CI is green.