feat(pro): Reflect live on Windows - #72
Open
Anurag-Wednesday wants to merge 1 commit into
Open
Conversation
Reflect is pure aggregation over observations the capture pipeline already writes - it adds no capture of its own. Replay's port put those observations on Windows, so Reflect needs no Windows implementation, only the gate flip. Audit found zero platform coupling on the whole path: crm/reflect.ts imports only core getDB, ./schema and ./utils; its IPC (crm:day-reflection, crm:week-reflection) is platform-free; ReflectScreen carries no native code. The only native dep on the path is better-sqlite3, already proven on Windows by core. Catalog copy and the screen have no Mac/Cmd/Option strings, so no copy neutralization was needed. The single catalog edit is sufficient for the same reason it was for Replay: nav lock is entitlement-only (locked: !isPro) and the screen gate routes through proFeatureComingSoon, with no core file special-casing the route - so nav, gating and copy all light up from the one source of truth. Tests: WIN_PORTED is parameterized, so adding reflect derives four assertions (live on win32; win32-only and not linux by implication; the "exactly the ported features are win32-supported" invariant; and that proFeatureComingSoon does not gate it). Reflect's own logic keeps its existing 13-case real-DB integration coverage. Verified: npm test 3161 passed; node, web and pro typechecks clean; eslint clean on both touched files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015U3TnnNULxfCb4TsjAGjiC
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
📝 WalkthroughWalkthroughReflect is now marked as supported on Windows in the Pro catalog. The lookup test route set includes ChangesReflect Windows support
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fourth Pro feature ported to Windows, after Vault, Clipboard and Replay. The cheapest one yet: no pro-side change at all, just the gate flip.
Why Reflect needs no Windows implementation
Reflect adds no capture of its own — it is pure aggregation over observations the capture pipeline already writes. Replay's port put those observations on Windows, so Reflect had nothing left to port.
A sweep for
osascript|process.platform|darwin|win32|pgrep|pkill|execFile|spawnoverpro/main/crm/reflect.tsandpro/renderer/screens/ReflectScreen.tsxreturns zero hits:reflect.ts(232 lines) imports only coregetDB,./schema,./utils— pure SQLite aggregation.crm:day-reflection,crm:week-reflectionincrm-ipc.ts) is platform-free.better-sqlite3, already proven on Windows by core.ReflectScreen.tsxcontain noMac/macOS/Cmd/Optionstrings, so no copy neutralization was needed.Why one catalog edit is sufficient
Same verification that held for Replay:
App.tsx:656,locked: !isPro.proFeatureComingSoon—App.tsx:999.reflectroute (only theViewModeunion, the route map, and the genericproItem('reflect')).So flipping
platformslights up nav, gating and copy together from the one source of truth.Tests
WIN_PORTEDinproCatalog.lookup.test.tsis parameterized, so addingreflectderives four assertions rather than hand-written ones:reflect is live on Windowsreflect is win32-only, not linux by implicationexactly the ported features are win32-supportedinvariantdoes NOT gate reflect — it renders live on win32Reflect's own logic keeps its existing real-DB integration coverage (
pro/main/crm/__tests__/reflect.integration.test.ts, 13 cases), unchanged and green.Typechecks clean on all three configs (node, web, pro). ESLint clean on both touched files.
Evidence note
No screenshot is possible from this seat, and there is no macOS-visible change to capture. On darwin,
featureSupportsPlatformalready returned true forreflectbefore this change, so behaviour on a Mac is bit-identical. The only visible change is on Windows, where the Reflect tab stops showing "coming soon" and renders the real screen — which is exactly what needs verifying on the Windows test seat.Per the working agreement, this PR stays open until Anurag confirms on real Windows hardware.
Windows verification asked for:
get-windows, which reports the exe FileDescription and can fall back to the filename — the same identity quirk Replay's self-capture fix dealt with. A wall ofchrome.exe-style names would be the tell.Test-failure accounting (corrected)
Correction to an earlier version of this PR body. It claimed three e2e specs failed and that all three "fail identically on
main". That claim was wrong and is withdrawn.npx playwright testdoes not rebuild —test:e2eiselectron-vite build && playwright test— so both the failing run and the baseline run were executed against a stale bundle, and the comparison proved nothing.The accurate picture, after a proper rebuild, on a real display with
pro/present:chat-memory.spec.ts:141andmeeting-transcription.spec.ts:46— never actually failed. Stale-build artifacts.settings-sections.spec.ts:124"resource mode survives a relaunch" — a real core bug, unrelated to Reflect:LLMServiceresolved its settings path in the constructor, beforeapp.setPath('userData', …), so anOFFGRID_USER_DATAprofile was ignored. Root-caused with a probe and fixed in fix(llm): load persisted settings lazily, not in the constructor #73. With that fix the spec file is 3/3.smoke.spec.ts:68— flake under full-suite parallel load; passes twice in isolation.Neither remaining failure is caused by this change, which adds no logic — a comment and one entry in a platform list.
Unit suite on this branch:
One flake seen along the way (
PermissionGate.capture-vision.integration.test.tsx, awaitFortimeout under parallel load) — passes in isolation and in full runs.Coverage gate: the pre-push floor is calibrated core-only; with
pro/present its Electron-shell files enter the denominator and drag branches below the floor. Pre-existing calibration gap, not a regression from this change. Pushed using the documented workaround of movingpro/aside so the gate measured its calibration basis, then restoring it. No--no-verify.On the e2e gate itself: CI's e2e job is advisory (
continue-on-error: true,ci.yml:158), so the green check on this PR coexists with 13 failing specs in the CI run. Most are the headless-runner instability the job comment documents; one was the real bug now fixed in #73.Not in scope
App.tsx:231's landing default is stillisPro && isMac() ? 'day' : 'models'— a platform decision living outside the seam. Harmless for Reflect (never the landing default) and correct today, but it must move behindfeatureSupportsPlatformbefore Day lands, or a ported Day will be stranded on Windows.🤖 Generated with Claude Code
https://claude.ai/code/session_015U3TnnNULxfCb4TsjAGjiC
Summary by CodeRabbit