Skip to content

feat(pro): Reflect live on Windows - #72

Open
Anurag-Wednesday wants to merge 1 commit into
mainfrom
feat/win-pro-reflect
Open

feat(pro): Reflect live on Windows#72
Anurag-Wednesday wants to merge 1 commit into
mainfrom
feat/win-pro-reflect

Conversation

@Anurag-Wednesday

@Anurag-Wednesday Anurag-Wednesday commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

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|spawn over pro/main/crm/reflect.ts and pro/renderer/screens/ReflectScreen.tsx returns zero hits:

  • reflect.ts (232 lines) imports only core getDB, ./schema, ./utils — pure SQLite aggregation.
  • Its IPC (crm:day-reflection, crm:week-reflection in crm-ipc.ts) is platform-free.
  • The only native dep on the path is better-sqlite3, already proven on Windows by core.
  • Catalog copy and ReflectScreen.tsx contain no Mac / macOS / Cmd / Option strings, so no copy neutralization was needed.

Why one catalog edit is sufficient

Same verification that held for Replay:

  • Nav lock is entitlement-only — App.tsx:656, locked: !isPro.
  • The screen gate routes through proFeatureComingSoonApp.tsx:999.
  • No core file special-cases the reflect route (only the ViewMode union, the route map, and the generic proItem('reflect')).

So flipping platforms lights up nav, gating and copy together from the one source of truth.

Tests

WIN_PORTED in proCatalog.lookup.test.ts is parameterized, so adding reflect derives four assertions rather than hand-written ones:

  • reflect is live on Windows
  • reflect is win32-only, not linux by implication
  • the exactly the ported features are win32-supported invariant
  • does NOT gate reflect — it renders live on win32

Reflect's own logic keeps its existing real-DB integration coverage (pro/main/crm/__tests__/reflect.integration.test.ts, 13 cases), unchanged and green.

Test Files  377 passed | 2 skipped (379)
     Tests  3161 passed | 4 skipped (3165)

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, featureSupportsPlatform already returned true for reflect before 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:

  • With capture running and observations accumulating, confirm mind-share / category / app breakdowns populate and the day view is non-empty.
  • Confirm the 7-day week rollup aggregates.
  • Report whether Windows app identities read sensibly in the app breakdown. They come from 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 of chrome.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 test does not rebuild — test:e2e is electron-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:

72 passed | 2 failed | 6 skipped
  • chat-memory.spec.ts:141 and meeting-transcription.spec.ts:46never actually failed. Stale-build artifacts.
  • settings-sections.spec.ts:124 "resource mode survives a relaunch" — a real core bug, unrelated to Reflect: LLMService resolved its settings path in the constructor, before app.setPath('userData', …), so an OFFGRID_USER_DATA profile 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:

Test Files  377 passed | 2 skipped (379)
     Tests  3161 passed | 4 skipped (3165)

One flake seen along the way (PermissionGate.capture-vision.integration.test.tsx, a waitFor timeout 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 moving pro/ 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 still isPro && isMac() ? 'day' : 'models' — a platform decision living outside the seam. Harmless for Reflect (never the landing default) and correct today, but it must move behind featureSupportsPlatform before Day lands, or a ported Day will be stranded on Windows.

🤖 Generated with Claude Code

https://claude.ai/code/session_015U3TnnNULxfCb4TsjAGjiC

Summary by CodeRabbit

  • New Features
    • Reflect Pro is now supported on Windows, in addition to macOS.

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-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Reflect is now marked as supported on Windows in the Pro catalog. The lookup test route set includes reflect as a Windows-ported feature.

Changes

Reflect Windows support

Layer / File(s) Summary
Platform gating and validation
src/renderer/src/components/pro/proCatalog.ts, src/renderer/src/lib/__tests__/proCatalog.lookup.test.ts
Reflect now supports macOS and Windows. Windows platform-support and coming-soon tests include the reflect route.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: alichherawalla

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes enabling the Reflect Pro feature on Windows.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/win-pro-reflect

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant