Issue #72: Hold-based distraction / awareness tracking (web + iOS)#181
Conversation
Replace toggle mind-state with press-and-hold (and Space on web) so mindStateLog records distraction as time ranges. Show optional thought capture after release, keep a persistent aware/distracted indicator, surface awareness vs distraction in summaries, and align buddy flows. Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHold-driven mind-state handling replaces toggle-based flows: Space/Comma or pointer press-and-hold begin Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (press/hold)
participant UI as UI Component (SessionView / BuddySessionRoom)
participant VM as ViewModel / Session Logic
participant Log as MindState Log
participant Timer as Timer
User->>UI: press-and-hold (pointer / Space / Comma)
UI->>VM: beginDistraction() / beginHyperfocus()
VM->>Log: append {time, "thinking" or "hyperfocus"}
VM->>Timer: mark hold start
Note over User,VM: hold continues...
User->>UI: release
UI->>VM: endDistraction() / endHyperfocus()
VM->>Log: append {time, "clear"} (finalize)
VM->>UI: set showPostDistractionCapture? (if thinking)
UI->>VM: onComplete/pause -> request resolved log
VM->>Log: return resolved log (with final clear if needed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ios/StillPointApp/Views/CompletionView.swift (1)
51-57: Minor platform inconsistency: iOS has defensive clamp, web doesn't.iOS uses
max(0, 100 - clearPercent)for distraction percentage whilesrc/components/CompletionScreen.tsxuses100 - clearPercentdirectly. IfclearPercentcould ever exceed 100 (e.g., due to a calculation bug), the web version would show a negative percentage.Consider aligning both platforms to use the defensive clamp, or verify that
clearPercentis always bounded to 0-100 upstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/Views/CompletionView.swift` around lines 51 - 57, iOS defensively clamps the displayed distraction percent with max(0, 100 - clearPercent) in the statCard call, but the web component CompletionScreen.tsx uses 100 - clearPercent directly; update CompletionScreen.tsx to clamp the value (e.g., use Math.max(0, 100 - clearPercent) or an equivalent clamp function) when rendering the distraction percentage, or alternatively ensure clearPercent is constrained to 0–100 upstream — change should target the rendering logic that constructs the displayed percentage in CompletionScreen.tsx to match the iOS statCard behavior.src/components/SessionView.tsx (1)
140-143: Dependency array includes state but function reads from ref.The
calcClearPercentcallback depends onmindStateLog(state) but reads frommindStateLogRef.current(ref). While this works because the state dependency triggers recreation and the ref is synced viauseEffect, it's slightly confusing. Consider using the state directly since it's already a dependency:♻️ Suggested simplification
const calcClearPercent = useCallback(() => { const endTime = elapsedRef.current || todayDuration; - return computeClearPercentFromLog(mindStateLogRef.current, endTime); + return computeClearPercentFromLog(mindStateLog, endTime); }, [mindStateLog, todayDuration]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SessionView.tsx` around lines 140 - 143, The calcClearPercent callback currently reads mindStateLogRef.current but lists mindStateLog in its dependency array, which is confusing; update calcClearPercent to read the state directly instead of the ref (use mindStateLog in the computeClearPercentFromLog call) and keep the dependency array as [mindStateLog, todayDuration] (elapsedRef can remain used for endTime as a ref). Specifically, modify the calcClearPercent function body to use mindStateLog and still pass elapsedRef.current || todayDuration as endTime when calling computeClearPercentFromLog.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/StillPointApp/Views/CompletionView.swift`:
- Around line 51-57: iOS defensively clamps the displayed distraction percent
with max(0, 100 - clearPercent) in the statCard call, but the web component
CompletionScreen.tsx uses 100 - clearPercent directly; update
CompletionScreen.tsx to clamp the value (e.g., use Math.max(0, 100 -
clearPercent) or an equivalent clamp function) when rendering the distraction
percentage, or alternatively ensure clearPercent is constrained to 0–100
upstream — change should target the rendering logic that constructs the
displayed percentage in CompletionScreen.tsx to match the iOS statCard behavior.
In `@src/components/SessionView.tsx`:
- Around line 140-143: The calcClearPercent callback currently reads
mindStateLogRef.current but lists mindStateLog in its dependency array, which is
confusing; update calcClearPercent to read the state directly instead of the ref
(use mindStateLog in the computeClearPercentFromLog call) and keep the
dependency array as [mindStateLog, todayDuration] (elapsedRef can remain used
for endTime as a ref). Specifically, modify the calcClearPercent function body
to use mindStateLog and still pass elapsedRef.current || todayDuration as
endTime when calling computeClearPercentFromLog.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90bd5c04-456f-4648-82fd-e0dd1227b865
📒 Files selected for processing (8)
ios/StillPointApp/Components/MindStateBarView.swiftios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/SessionView.swiftsrc/components/BuddySessionRoom.tsxsrc/components/CompletionScreen.tsxsrc/components/SessionView.tsxsrc/lib/mindStateSession.ts
…t deps Clamp displayed distraction percent on web completion to match iOS. Use mindStateLog state in calcClearPercent so the dependency array matches. Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/SessionView.tsx (1)
42-45: Consider removing redundantuseEffectfor consistency.This effect syncs
mindStateLogRefafter render, but everysetMindStateLogcall in this file (lines 85-89, 98-101, 153-155) already updatesmindStateLogRef.currentsynchronously within the setter callback. The effect is therefore redundant and creates an inconsistent pattern compared tomindStateRef(lines 38-39) which uses direct assignment.🧹 Suggested simplification
const [mindStateLog, setMindStateLog] = useState<Array<{ time: number; state: string }>>([]); const mindStateLogRef = useRef(mindStateLog); - useEffect(() => { - mindStateLogRef.current = mindStateLog; - }, [mindStateLog]); + mindStateLogRef.current = mindStateLog;This mirrors the
mindStateRefpattern and removes the async sync gap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SessionView.tsx` around lines 42 - 45, The useEffect that syncs mindStateLogRef is redundant—remove the effect and rely on the existing synchronous updates to mindStateLogRef.current that occur inside setMindStateLog calls (see setMindStateLog usage at lines around where it's invoked). Mirror the mindStateRef pattern by deleting the useEffect block and keeping the initial const mindStateLogRef = useRef(mindStateLog) so all updates happen synchronously via the setter callbacks (ensure no other code depends on the post-render effect before removing it).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/SessionView.tsx`:
- Around line 42-45: The useEffect that syncs mindStateLogRef is
redundant—remove the effect and rely on the existing synchronous updates to
mindStateLogRef.current that occur inside setMindStateLog calls (see
setMindStateLog usage at lines around where it's invoked). Mirror the
mindStateRef pattern by deleting the useEffect block and keeping the initial
const mindStateLogRef = useRef(mindStateLog) so all updates happen synchronously
via the setter callbacks (ensure no other code depends on the post-render effect
before removing it).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4af3c1aa-20d4-4b6a-8450-834160d94c2e
📒 Files selected for processing (2)
src/components/CompletionScreen.tsxsrc/components/SessionView.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/CompletionScreen.tsx
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/SessionView.tsx (1)
260-260: Use the narrow-component margin rule here.This container tops out at 420px, so it fits the repo’s
maxWidth ≤ 500pxbucket.calc(100vw - 24px)makes the side gutters tighter than the other narrow components;calc(100vw - 40px)would keep this aligned with the established mobile spacing pattern.Suggested tweak
- <div style={{ width: "100%", maxWidth: "min(420px, calc(100vw - 24px))", marginTop: "12px" }}> + <div style={{ width: "100%", maxWidth: "min(420px, calc(100vw - 40px))", marginTop: "12px" }}>Based on learnings: In the Still Point app (auerbachb/still-point), apply horizontal margins using
calc(100vw - N px)based on the component's maxWidth: ifmaxWidth ≤ 500px, usecalc(100vw - 40px).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SessionView.tsx` at line 260, The container div in SessionView.tsx uses calc(100vw - 24px) which tightens mobile side gutters; update the inline style on that div (the element with style {{ width: "100%", maxWidth: "min(420px, calc(100vw - 24px))", marginTop: "12px" }}) to use calc(100vw - 40px) so the maxWidth ≤ 500px narrow-component follows the repo's established mobile horizontal margin rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/SessionView.tsx`:
- Around line 90-101: The beginDistraction handler clears
showPostDistractionCapture immediately, which causes an open ThoughtCapture
draft to be lost when the user re-holds; update beginDistraction (and the
similar handler used around the 354-360 code) to guard against starting a new
hold while showPostDistractionCapture is true or to persist the draft outside
ThoughtCapture: check showPostDistractionCapture (or a persistedDraftRef/state)
at the top of beginDistraction and return early if a post-capture draft exists,
or move the draft state into a higher-level ref/state so
setShowPostDistractionCapture(false) no longer drops the draft when calling
setMindState/setMindStateLog/setSessionThoughtCount. Ensure references to
mindStateRef.current and setShowPostDistractionCapture are adjusted accordingly
to prevent accidental loss of the draft.
- Line 100: The code currently increments thoughtCount via
setSessionThoughtCount(prev => prev + 1) when distraction segments begin, but
that counts holds rather than actual captured notes; introduce a separate
segment counter state (e.g., sessionSegmentCount or distractionSegmentCount) and
increment that wherever setSessionThoughtCount is currently incremented (the
distraction/segment-start and skip handlers referenced around those
occurrences), update the live badge to read from the new segment counter, and
stop incrementing thoughtCount in those places; derive the final/completion
payload and any “thoughts captured” logic from sessionThoughts.length (and
ensure handleSaveThought only pushes to sessionThoughts and updates thoughtCount
based on sessionThoughts.length if needed).
---
Nitpick comments:
In `@src/components/SessionView.tsx`:
- Line 260: The container div in SessionView.tsx uses calc(100vw - 24px) which
tightens mobile side gutters; update the inline style on that div (the element
with style {{ width: "100%", maxWidth: "min(420px, calc(100vw - 24px))",
marginTop: "12px" }}) to use calc(100vw - 40px) so the maxWidth ≤ 500px
narrow-component follows the repo's established mobile horizontal margin rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5fa8defb-8443-485f-840b-ff5fdf18d704
📒 Files selected for processing (1)
src/components/SessionView.tsx
…tems - Web/Buddy: two hold controls (light distraction + hyperfocus) with Space and Comma hints; log hyperfocus segments; keyboard uses holdKindRef so Space and comma do not fight. - Block starting a new hold while post-distraction ThoughtCapture is open; thoughtCount in saved payloads matches captured notes only; segment badge uses distractionSegmentCount. - mindStateSession: treat hyperfocus as awareness time; MindStateBar shows blue hyperfocus segments. - iOS: beginHyperfocus/endHyperfocus, dual hold pills, MindStateBarView colors, MindStateEntry.isClear includes hyperfocus; thoughtCount derived from captures. Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/SessionView.tsx (1)
234-244: MisleadingholdKindRefvalue in hyperfocus pointer handler.Line 236 sets
holdKindRef.current = "pointerDistraction"for the hyperfocus handler, which is inconsistent with the naming. While the code functions correctly becausehandlePointerHyperfocusUpalso checksmindStateRef.current === "hyperfocus", this naming inconsistency could cause confusion during maintenance.Consider using a distinct value or a more generic name:
Option 1: Use a generic "pointerHold" value for both
const handlePointerDistractionDown = () => { if (!isActive || mindStateRef.current !== "clear" || showPostDistractionCapture) return; - holdKindRef.current = "pointerDistraction"; + holdKindRef.current = "pointerHold"; beginDistraction(); }; const handlePointerDistractionUp = () => { - if (holdKindRef.current !== "pointerDistraction") return; + if (holdKindRef.current !== "pointerHold" || mindStateRef.current !== "thinking") return; holdKindRef.current = "none"; finalizeActiveHold(elapsedRef.current, true); }; const handlePointerHyperfocusDown = () => { if (!isActive || mindStateRef.current !== "clear" || showPostDistractionCapture) return; - holdKindRef.current = "pointerDistraction"; + holdKindRef.current = "pointerHold"; beginHyperfocus(); }; const handlePointerHyperfocusUp = () => { - if (holdKindRef.current !== "pointerDistraction" || mindStateRef.current !== "hyperfocus") return; + if (holdKindRef.current !== "pointerHold" || mindStateRef.current !== "hyperfocus") return; holdKindRef.current = "none"; finalizeActiveHold(elapsedRef.current, false); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SessionView.tsx` around lines 234 - 244, The holdKindRef value set in handlePointerHyperfocusDown is misleadingly "pointerDistraction" for a hyperfocus flow; change the value to a clearer identifier (e.g., "pointerHold" or "pointerHyperfocus") and update handlePointerHyperfocusUp to check against that same identifier via holdKindRef.current, ensuring both functions (handlePointerHyperfocusDown and handlePointerHyperfocusUp) use the matching, descriptive holdKindRef value to avoid confusion during maintenance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/SessionView.tsx`:
- Around line 234-244: The holdKindRef value set in handlePointerHyperfocusDown
is misleadingly "pointerDistraction" for a hyperfocus flow; change the value to
a clearer identifier (e.g., "pointerHold" or "pointerHyperfocus") and update
handlePointerHyperfocusUp to check against that same identifier via
holdKindRef.current, ensuring both functions (handlePointerHyperfocusDown and
handlePointerHyperfocusUp) use the matching, descriptive holdKindRef value to
avoid confusion during maintenance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7663f91a-a000-4525-a9f3-cb2277ab3a1b
📒 Files selected for processing (8)
ios/StillPointApp/Components/MindStateBarView.swiftios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointApp/Views/SessionView.swiftios/StillPointShared/Sources/StillPointShared/Models/MindStateEntry.swiftsrc/components/BuddySessionRoom.tsxsrc/components/MindStateBar.tsxsrc/components/SessionView.tsxsrc/lib/mindStateSession.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/MindStateBar.tsx
- src/components/BuddySessionRoom.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ios/StillPointApp/Components/MindStateBarView.swift
- src/lib/mindStateSession.ts
Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/SessionView.tsx (2)
55-55: Consider adding a comment to clarify the purpose.The unused first element of
[, setLiveElapsed]is a valid pattern for triggering re-renders, but a brief comment would improve clarity for future maintainers.- const [, setLiveElapsed] = useState(0); + // State setter used only to trigger re-renders when elapsed time changes + const [, setLiveElapsed] = useState(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SessionView.tsx` at line 55, The state tuple const [, setLiveElapsed] = useState(0) in the SessionView component is intentionally using the setter to trigger re-renders but lacks clarification; add a short inline comment above or beside this declaration explaining that the state variable is unused and setLiveElapsed is used solely to force re-renders (e.g., "unused state value; using setter to trigger re-render on timer/elapsed updates"), so future maintainers understand why the first element is omitted.
36-38: Consider extracting shared hold-tracking logic.Both
SessionView.tsxandBuddySessionRoom.tsxduplicate the hold-tracking pattern (refs, keyboard handlers, begin/finalize functions). Consider extracting auseMindStateHoldhook or shared utilities in a future iteration to reduce maintenance burden and ensure consistent behavior.This is not blocking for this PR given the current scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SessionView.tsx` around lines 36 - 38, Both files duplicate the same hold-tracking pattern (refs, keyboard handlers, begin/finalize functions) — extract that behavior into a reusable hook named useMindStateHold and replace the duplicated logic in SessionView.tsx and BuddySessionRoom.tsx with calls to this hook; the hook should expose the ref(s), event handler functions (e.g., onKeyDown/onKeyUp equivalents), and beginHold/finalizeHold methods so SessionView's and BuddySessionRoom's existing refs and handlers can be replaced with the hook's returned API for consistent behavior and easier maintenance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/SessionView.tsx`:
- Line 55: The state tuple const [, setLiveElapsed] = useState(0) in the
SessionView component is intentionally using the setter to trigger re-renders
but lacks clarification; add a short inline comment above or beside this
declaration explaining that the state variable is unused and setLiveElapsed is
used solely to force re-renders (e.g., "unused state value; using setter to
trigger re-render on timer/elapsed updates"), so future maintainers understand
why the first element is omitted.
- Around line 36-38: Both files duplicate the same hold-tracking pattern (refs,
keyboard handlers, begin/finalize functions) — extract that behavior into a
reusable hook named useMindStateHold and replace the duplicated logic in
SessionView.tsx and BuddySessionRoom.tsx with calls to this hook; the hook
should expose the ref(s), event handler functions (e.g., onKeyDown/onKeyUp
equivalents), and beginHold/finalizeHold methods so SessionView's and
BuddySessionRoom's existing refs and handlers can be replaced with the hook's
returned API for consistent behavior and easier maintenance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ed75193-ef41-4731-8272-76314457915d
📒 Files selected for processing (2)
src/components/BuddySessionRoom.tsxsrc/components/SessionView.tsx
… trick Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
Summary
Implements distraction tracking as press-and-hold (not tap-toggle) while keeping persisted
mindStateLogentries as backward-compatible"clear"/"thinking"transitions keyed by session elapsed seconds.Web (
SessionView,BuddySessionRoom)data-no-space-distraction)."clear"and optionally show ThoughtCapture (dismiss with skip / Escape as before).mindStateLogis consistent.iOS (
SessionView,SessionViewModel)beginDistraction()/endDistraction()replace toggle;showPostDistractionCapturegates post-release thought capture.MindStateBarViewtakescurrentMindStateso the tail segment reflects live holds before the log gets the closing"clear"entry.Summary screens
CompletionScreenand iOSCompletionView: labels for awareness, distraction (derived as100 - clearPercent), and captured notes (thought count).Verification
npm run build(includes lint + typecheck) passes locally.coderabbitCLI was not available in this environment.Notes
No API or schema changes: existing
mindStateLogJSONB andclearPercentcontinue to round-trip; distraction intervals are reconstructible from successivethinking→clearpairs at loggedtimevalues relative to session start.Summary by CodeRabbit
New Features
Style
Bug Fixes