feat: pig overlay window + tray icon [012+013] - #10
Merged
Conversation
Transparent fullscreen overlay renders one pixel-pig per Focus via React. Pigs wander at 35px/s with soft boundary steering and 4-frame walk animation. Click-through via Rust polling thread (16ms) that toggles ignore_cursor_events based on pig bounding boxes synced from the frontend every 4 rAF frames. Tray icon shows live focus list; red badge when over-cap; rebuilds within 1s of any focus file change via the existing focuses-changed watcher fan-out. Quit item in tray menu exits cleanly. Closes #6 (013 tray icon).
📝 WalkthroughWalkthroughIntroduces a pixel-pig fullscreen overlay UX and macOS tray menu: adds frontend pig animation/hitbox sync, Rust-side pig hit-testing and cursor polling for click-through, tray/menu wiring with live focus list and over-cap icon, and docs/spec updates deferring agent/proposal flow to v1.3. ChangesPig Overlay & Tray Management System
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as React Frontend
participant Tauri as Tauri App (PigHitTester, Tray)
participant OS as OS/Pointer
Note over Frontend,Tauri: Runtime pig rect sync + tray setup at startup
Frontend->>Tauri: invoke("update_pig_rects", [rects])
Tauri->>Tauri: PigHitTester.update(rects)
OS->>Tauri: Cursor polled by background thread
Tauri->>Tauri: PigHitTester.is_hit(x,y)
alt Cursor over pig
Tauri->>Tauri: set_ignore_cursor_events(false)
else Cursor not over pig
Tauri->>Tauri: set_ignore_cursor_events(true)
end
User->>Frontend: Click on visible pig
Frontend->>Frontend: set selectedId, show PigDetail
User->>Frontend: Click "Clear task"
Frontend->>Tauri: focusWriter.deleteTask(focusId, idx)
Tauri->>Frontend: emit focuses-changed
Frontend->>Frontend: usePigMovement updates pigs
Frontend->>Tauri: invoke("update_pig_rects", [rects])
sequenceDiagram
participant User
participant Tauri as Tauri App (Tray, CapEvaluator)
participant Store as Focus Store (files)
Tauri->>Tauri: tray::setup() build initial menu
Store-->>Tauri: focuses-changed (file/HTTP)
Tauri->>Tauri: tray::rebuild_handler() reads store, build_menu()
Tauri->>Tauri: CapEvaluator -> decide icon
alt any cap exceeded
Tauri->>Tauri: set tray icon = red_icon
else
Tauri->>Tauri: set tray icon = default/clear
end
User->>Tauri: Click tray icon -> native NSMenu shown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PRD.md (1)
97-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign
widgetsettings schema across docs.Line 105 introduces
widget.always_on_top, but the docs elsewhere still definewidget.window_leveloptions. This creates a conflicting configuration contract and can cause implementation drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PRD.md` around lines 97 - 107, The PRD currently defines widget.always_on_top while other docs use widget.window_level, causing a schema mismatch; update the PRD snippet to match the canonical schema by replacing widget.always_on_top with widget.window_level and specify the same allowed values/semantics used elsewhere (e.g., "normal" | "floating" | "always_on_top") so the configuration contract is consistent with the rest of the documentation and any validation/code expecting widget.window_level.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTEXT.md`:
- Around line 9-20: Add blank lines before and after each subsection heading in
the CONTEXT.md block so the headings "Focus", "Pig", "Task", and "Proposal" are
separated by an empty line from the surrounding paragraphs; update the paragraph
boundaries around those headings (the blocks containing the text for Focus, Pig,
Task, and Proposal) to include a blank line above and below each heading to
satisfy markdownlint MD022.
In `@issues/013-tray-icon-focus-list.md`:
- Around line 16-24: The fenced code block under the "NSMenu contents" example
violates markdownlint MD031/MD040; update the markdown around the MenuBuilder
example by adding a blank line before and after the fenced block and include a
language tag (e.g., use ```text) so the block is fenced like a proper
fenced-code section; apply this change to the "NSMenu contents" example and any
similar examples in this file (referencing the example showing MenuItemBuilder
and Quit) to satisfy the linter.
In `@issues/015-delete-focus-from-tray.md`:
- Around line 14-16: The fenced code block containing the text `Delete "Customer
X bug"…` violates markdownlint MD031/MD040; fix it by adding a blank line before
and after the fenced block and include a language tag (e.g., change ``` to
```text) so the block reads with an empty line above, ```text, the content line,
```, and then an empty line after to satisfy the linter.
- Line 41: Update the acceptance criterion text in the issue so it correctly
reflects the dialog flow: replace the line "- [ ] No confirmation dialog if the
user cancels." with a clear statement such as "No deletion occurs when the user
cancels" (or similar wording) to ensure it describes that choosing Cancel inside
the confirmation dialog prevents deletion; update the criterion in the issue
file (issues/015-delete-focus-from-tray.md) where the current contradictory line
appears.
In `@issues/016-real-sprite-sheet.md`:
- Around line 20-31: The fenced TSX example containing the <div
className="pig-sprite-frame"> block (the snippet using pigSheet, FRAME_W,
FRAME_H, frame, and row) needs blank lines added around the fence to satisfy
markdownlint MD031: insert one empty line immediately after the opening "```tsx"
and one empty line immediately before the closing "```" so the code block is
separated by blank lines from surrounding text.
In `@PRD.md`:
- Around line 60-63: The headings in PRD.md violate markdownlint MD022 by
missing required blank lines around them; update the file to ensure there is an
empty line before and after each affected heading (for example the headings "###
FR1 — Focus storage" and "### FR2 — Transparent overlay window" and the other
headings at lines 69, 77, 88, 91, and 109) so every heading is separated by a
blank line from surrounding content; make these spacing fixes throughout the
section to satisfy MD022.
In `@src-tauri/src/app/mod.rs`:
- Around line 139-145: The loop initializes last_over as false causing missed
initial state; before entering the loop, query the current cursor via
app_handle.cursor_position(), compute over with hittester.is_hit(cursor.x,
cursor.y), set last_over = over and call
window_clone.set_ignore_cursor_events(!over) so the window is initialized
click-through correctly; if cursor_position() can fail, handle the Err by
leaving last_over as false or defaulting appropriately before the loop.
- Around line 126-130: The cursor/rect coordinate spaces are mismatched:
app_handle.cursor_position() returns global screen coordinates while
update_pig_rects supplies DOMRect coordinates in webview-local space, so adjust
one to match the other before calling is_hit(); specifically, read the window
global position (via window.outer_position or similar) and subtract that from
app_handle.cursor_position() to produce a webview-local Point before passing to
is_hit(), or instead add the window global position to each rect received in
update_pig_rects to store them in global coordinates; update every hit-testing
call site (where is_hit() is invoked) to use the normalized coordinates and
ensure any storage of pig rects notes which space they are in (webview-local vs
global).
In `@src-tauri/src/app/tray.rs`:
- Around line 18-19: The code is silently converting FocusStore read errors into
an empty tray by calling store.list().unwrap_or_default(); instead handle the
Result explicitly: call store.list() and match Err/Ok, logging the error with
context (include the error and that reading FocusStore failed), and on error
either return Err to fail tray setup (propagating context) or preserve the
existing tray state instead of passing an empty Vec to build_menu; apply the
same change where unwrap_or_default() is used (the call sites around build_menu
and the lines referenced) so failures are not masked and are properly
logged/propagated.
- Around line 55-60: The tray icon isn't cleared when over_cap becomes false if
handle.default_window_icon() returns None; update the logic in the over-cap
handling so that when over_cap is false you always call tray.set_icon to
clear/restore the icon (use tray.set_icon(Some(icon.clone())) when
default_window_icon() yields Some(icon) and call tray.set_icon(None) when it
yields None) so any previously set red_icon() is removed; adjust the block
around over_cap, cap_state, tray.set_icon, and handle.default_window_icon
accordingly.
In `@src-tauri/src/ui_bridge/mod.rs`:
- Around line 12-16: The ui_bridge currently depends on app internals via
PigHitState(pub PigHitTester); instead define and use a trait-based contract
(e.g., a HitTester trait that lives in a lower domain module) and change
PigHitState to wrap an Arc<dyn HitTester + Send + Sync> (or another trait
object) so ui_bridge only references the trait, not PigHitTester/PigRect; update
any places constructing PigHitState to inject an implementation of HitTester
(the existing PigHitTester can implement the trait in app/pig_hittest) and
adjust consumers to use the trait methods instead of concrete types.
In `@src/components/App.tsx`:
- Around line 37-44: Move the deleteTask I/O out of the PigDetail JSX: create an
async handler (e.g., handleDeleteTask) in the parent container/component that
calls focusWriter.deleteTask(selectedFocus.id, index) with await, wraps it in
try/catch to log errors and update UI state (loading/error/recovery UI or
toast), and only pass a safe callback prop into PigDetail (e.g.,
onClearTask={(index) => handleDeleteTask(index)}). Replace the inline void
focusWriter.deleteTask(...) with the pre-handled onClearTask prop and ensure you
remove the void so the Promise is awaited and failures are handled.
In `@src/components/PigDetail.tsx`:
- Around line 25-30: The Escape key handler on the non-focusable backdrop div
won't fire; move keyboard handling into PigDetail via a useEffect: add a
handleKeyDown function that calls onClose when event.key === "Escape", attach it
with window.addEventListener("keydown", handleKeyDown) in useEffect and remove
it on cleanup, remove the onKeyDown from the backdrop div, and include onClose
in the effect dependency array so Escape reliably closes the detail view.
- Around line 15-21: The PigDetail component is reading window.screen (browser
I/O) and must be made view-only: remove the direct window.screen.width/height
reads and move the positioning logic out of the component; either add props like
viewportWidth and viewportHeight to PigDetail and compute rawX/x/y in the
parent/hook before rendering, or implement a shared presenter/util function
(e.g., computeCardPosition(pigX, pigY, viewportW, viewportH, CARD_OFFSET_X,
CARD_W)) in a non-components folder and call that from the parent/hook to supply
x and y (or viewport values) to PigDetail; update PigDetail signature to accept
the computed x/y or viewport props and delete any use of window.screen from
src/components/* files.
---
Outside diff comments:
In `@PRD.md`:
- Around line 97-107: The PRD currently defines widget.always_on_top while other
docs use widget.window_level, causing a schema mismatch; update the PRD snippet
to match the canonical schema by replacing widget.always_on_top with
widget.window_level and specify the same allowed values/semantics used elsewhere
(e.g., "normal" | "floating" | "always_on_top") so the configuration contract is
consistent with the rest of the documentation and any validation/code expecting
widget.window_level.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fd990b05-c3f2-4eaa-9acf-6afc798d36d3
📒 Files selected for processing (18)
CONTEXT.mdPRD.mdissues/013-tray-icon-focus-list.mdissues/014-new-focus-from-tray.mdissues/015-delete-focus-from-tray.mdissues/016-real-sprite-sheet.mdissues/done/012-pig-overlay.mdsrc-tauri/src/app/mod.rssrc-tauri/src/app/pig_hittest.rssrc-tauri/src/app/tray.rssrc-tauri/src/ui_bridge/mod.rssrc/components/App.test.tsxsrc/components/App.tsxsrc/components/PigDetail.tsxsrc/components/PigSprite.tsxsrc/hooks/usePigMovement.tssrc/main.tsxsrc/styles.css
Comment on lines
+20
to
+31
| ```tsx | ||
| <div | ||
| className="pig-sprite-frame" | ||
| style={{ | ||
| backgroundImage: `url(${pigSheet})`, | ||
| backgroundPosition: `-${frame * FRAME_W}px -${row * FRAME_H}px`, | ||
| width: FRAME_W, | ||
| height: FRAME_H, | ||
| imageRendering: "pixelated", | ||
| }} | ||
| /> | ||
| ``` |
There was a problem hiding this comment.
Add blank lines around the fenced TSX example.
This block is currently failing markdownlint MD031 (blanks-around-fences).
Suggested fix
- - Replace with:
- ```tsx
+
+ ```tsx
<div
className="pig-sprite-frame"
style={{
backgroundImage: `url(${pigSheet})`,
backgroundPosition: `-${frame * FRAME_W}px -${row * FRAME_H}px`,
@@
}}
/>
```
+
- `row` derived from `direction`: `left=1, right=2` (up/down unused in v1.2 — pigs only move in 2D horizontal plane for now).🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 20-20: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 31-31: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@issues/016-real-sprite-sheet.md` around lines 20 - 31, The fenced TSX example
containing the <div className="pig-sprite-frame"> block (the snippet using
pigSheet, FRAME_W, FRAME_H, frame, and row) needs blank lines added around the
fence to satisfy markdownlint MD031: insert one empty line immediately after the
opening "```tsx" and one empty line immediately before the closing "```" so the
code block is separated by blank lines from surrounding text.
Comment on lines
+55
to
+60
| let over_cap = cap_state(&focuses, settings.caps).any_over(); | ||
| if over_cap { | ||
| let _ = tray.set_icon(Some(red_icon())); | ||
| } else if let Some(icon) = handle.default_window_icon() { | ||
| let _ = tray.set_icon(Some(icon.clone())); | ||
| } |
There was a problem hiding this comment.
Ensure the over-cap badge clears even when no default icon is configured.
If default_window_icon() is None, Line 58-Line 60 never resets the icon, so a previously red icon can remain stuck after returning under-cap.
Proposed fix
if over_cap {
let _ = tray.set_icon(Some(red_icon()));
- } else if let Some(icon) = handle.default_window_icon() {
- let _ = tray.set_icon(Some(icon.clone()));
+ } else if let Some(icon) = handle.default_window_icon() {
+ let _ = tray.set_icon(Some(icon.clone()));
+ } else {
+ let _ = tray.set_icon(None);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let over_cap = cap_state(&focuses, settings.caps).any_over(); | |
| if over_cap { | |
| let _ = tray.set_icon(Some(red_icon())); | |
| } else if let Some(icon) = handle.default_window_icon() { | |
| let _ = tray.set_icon(Some(icon.clone())); | |
| } | |
| let over_cap = cap_state(&focuses, settings.caps).any_over(); | |
| if over_cap { | |
| let _ = tray.set_icon(Some(red_icon())); | |
| } else if let Some(icon) = handle.default_window_icon() { | |
| let _ = tray.set_icon(Some(icon.clone())); | |
| } else { | |
| let _ = tray.set_icon(None); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/app/tray.rs` around lines 55 - 60, The tray icon isn't cleared
when over_cap becomes false if handle.default_window_icon() returns None; update
the logic in the over-cap handling so that when over_cap is false you always
call tray.set_icon to clear/restore the icon (use
tray.set_icon(Some(icon.clone())) when default_window_icon() yields Some(icon)
and call tray.set_icon(None) when it yields None) so any previously set
red_icon() is removed; adjust the block around over_cap, cap_state,
tray.set_icon, and handle.default_window_icon accordingly.
Comment on lines
+12
to
+16
| use crate::app::pig_hittest::{PigHitTester, PigRect}; | ||
|
|
||
| pub struct CommandsState(pub Arc<Commands>); | ||
|
|
||
| pub struct PigHitState(pub PigHitTester); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Keep ui_bridge independent of app/ internals.
PigHitState(pub PigHitTester) makes this command layer depend on src-tauri/src/app/pig_hittest.rs, which inverts the boundary the repo rules call for. Move the hit-test contract to a lower module and inject a trait-backed state here so ui_bridge stays a pure command/event surface.
As per coding guidelines, src-tauri/src/ui_bridge/**/*.rs: ui_bridge/ should contain Tauri commands / events; depends on domain + storage via traits.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/ui_bridge/mod.rs` around lines 12 - 16, The ui_bridge currently
depends on app internals via PigHitState(pub PigHitTester); instead define and
use a trait-based contract (e.g., a HitTester trait that lives in a lower domain
module) and change PigHitState to wrap an Arc<dyn HitTester + Send + Sync> (or
another trait object) so ui_bridge only references the trait, not
PigHitTester/PigRect; update any places constructing PigHitState to inject an
implementation of HitTester (the existing PigHitTester can implement the trait
in app/pig_hittest) and adjust consumers to use the trait methods instead of
concrete types.
…y errors, view-only violations, markdown lint
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CONTEXT.md (1)
63-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProposal→Focus mapping should use a stable key (prefer
focus.id), not mutabletitle/description.This doc currently states: “Focus is a mental anchor identified by
title+description.” That’s risky if the user edits a Focus title/description between:
- the agent composing/posting a proposal, and
- the user accepting/rejecting (or the app later routing at
/checkpoint).Because the persistence format already has a stable
focus.mdfrontmatterid: <uuid>, I strongly recommend clarifying that mapping usesfocus.idas the primary key, whiletitle/descriptionremain human-readable routing context.📝 Proposed doc tweak
- - **No static binding from agent context to Focus.** Focus is a mental anchor identified by `title` + `description`. The in-session agent decides routing at `/checkpoint` time using the catalog returned by the app. + - **No static binding from agent context to Focus.** The app/proposal handling uses `focus.md`’s stable `id` as the identity key, while `title` + `description` are used as human-readable routing context at `/checkpoint` time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTEXT.md` around lines 63 - 68, Update the doc to say that Proposal→Focus mapping must use the stable focus.id (from focus.md frontmatter) as the canonical key instead of mutable title/description, and clarify that title and description remain human-readable context only; reference the entities proposals.jsonl, focus.id, and focus.md frontmatter so readers know where the stable id lives and that routing/lookup at /checkpoint should match proposals to focuses by focus.id.
♻️ Duplicate comments (2)
src/components/App.tsx (1)
25-31: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winKeep task-deletion I/O out of
src/components/App.tsx.Wrapping the call in
try/awaitfixed the dropped Promise, but this component still performs direct write I/O throughfocusWriter. Please move the delete flow into a hook/container and passPigDetaila view-only callback plus any loading/error state it needs.As per coding guidelines,
src/components/**/*.{ts,tsx}: Frontend React components insrc/components/should be view-only with nofetchand no direct I/O.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/App.tsx` around lines 25 - 31, The handleClearTask function in App.tsx performs direct write I/O by calling focusWriter.deleteTask; move this delete flow out of the component into a dedicated hook or container (e.g., useFocusTasks or FocusTasksContainer) that exposes an async deleteTaskByIndex(focusId, index) method plus loading and error state; update App.tsx to call a view-only callback prop (e.g., onDeleteTask(index)) passed down to PigDetail and read loading/error flags from the hook/container to render UI only, removing any direct use of focusWriter.deleteTask and keeping selectedFocus handling purely view-related.src-tauri/src/app/tray.rs (1)
18-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t turn an initial store read failure into “No focuses yet.”
Falling back to
Vec::new()here still masks a real storage error as an empty tray, so launch can show the wrong state until a later rebuild happens. Please propagate the failure or preserve an explicit error path instead of building the menu from fake empty data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/app/tray.rs` around lines 18 - 24, The match on store.list() currently swallows real read errors by returning Vec::new() into focuses; instead propagate the error (e.g., return Err(...) from the tray setup function or use the ? operator on store.list()) so failures are handled explicitly rather than treated as an empty focus list; replace the Vec::new() fallback and adjust the enclosing function's signature to return a Result if necessary so store.list() errors surface to the caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@issues/013-tray-icon-focus-list.md`:
- Around line 27-31: The live-rebuild should not call tray.set_menu on every
FOCUSES_CHANGED_EVENT; update the setup subscription to coalesce/debounce
focuses-change events (e.g., 200–500ms window) before re-reading the focus store
and rebuilding the menu so rapid event storms don’t cause flicker/thrash;
implement a timer/debounce mechanism inside the setup event handler that batches
events and triggers a single tray.set_menu(Some(new_menu)) call (still ensuring
the rebuild occurs within ~1s of the user's change) and leave the existing
CapEvaluator over-cap check unchanged so the icon-swap logic still runs after
the debounced rebuild.
- Around line 23-24: The polling thread that spins checking cursor position (the
thread created via spawn in the tray polling loop) has no shutdown signal and
will be terminated abruptly when app.exit(0) is called; add a graceful shutdown
by introducing a shared shutdown flag or channel (e.g., an Arc<AtomicBool> or a
oneshot channel) that the spawned thread checks each loop iteration, store the
sender/flag or the thread JoinHandle in the app state (similar to existing
managed resources), and ensure app.exit(0) sets the flag/sends the shutdown
signal and joins the thread (or waits for its clean exit) before exiting; update
the code paths that call app.exit(0) to trigger this shutdown sequence.
In `@PRD.md`:
- Around line 80-89: The PRD’s FR4 section is too broad for this PR: reduce the
scope to match the shipped tray behavior by removing or marking as future work
the unimplemented items ("+ New Focus" popover and clickable Focus actions) and
leave only the implemented live focus list and "Quit" behavior; update the FR4
text to reflect that the NSMenu currently provides a list of current Focuses and
a Quit item, and add a short note such as "Additional tray actions (New Focus
popover, focus click behavior) are planned for follow-up issues" so the v1.2 PRD
accurately describes what this PR delivers.
- Around line 105-114: Add a blank line before and after the fenced YAML block
that begins with "`~/.adhd-ranch/settings.yaml`" and the triple backticks so the
block is separated from surrounding prose; open PRD.md, locate the fenced code
block that contains the YAML keys (caps, alerts, widget) and insert one empty
line above the opening ``` and one empty line below the closing ``` to satisfy
markdownlint MD031.
---
Outside diff comments:
In `@CONTEXT.md`:
- Around line 63-68: Update the doc to say that Proposal→Focus mapping must use
the stable focus.id (from focus.md frontmatter) as the canonical key instead of
mutable title/description, and clarify that title and description remain
human-readable context only; reference the entities proposals.jsonl, focus.id,
and focus.md frontmatter so readers know where the stable id lives and that
routing/lookup at /checkpoint should match proposals to focuses by focus.id.
---
Duplicate comments:
In `@src-tauri/src/app/tray.rs`:
- Around line 18-24: The match on store.list() currently swallows real read
errors by returning Vec::new() into focuses; instead propagate the error (e.g.,
return Err(...) from the tray setup function or use the ? operator on
store.list()) so failures are handled explicitly rather than treated as an empty
focus list; replace the Vec::new() fallback and adjust the enclosing function's
signature to return a Result if necessary so store.list() errors surface to the
caller.
In `@src/components/App.tsx`:
- Around line 25-31: The handleClearTask function in App.tsx performs direct
write I/O by calling focusWriter.deleteTask; move this delete flow out of the
component into a dedicated hook or container (e.g., useFocusTasks or
FocusTasksContainer) that exposes an async deleteTaskByIndex(focusId, index)
method plus loading and error state; update App.tsx to call a view-only callback
prop (e.g., onDeleteTask(index)) passed down to PigDetail and read loading/error
flags from the hook/container to render UI only, removing any direct use of
focusWriter.deleteTask and keeping selectedFocus handling purely view-related.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f3b464e8-759f-45cc-b697-12e00936c3fa
📒 Files selected for processing (10)
CONTEXT.mdPRD.mdissues/013-tray-icon-focus-list.mdissues/015-delete-focus-from-tray.mdissues/016-real-sprite-sheet.mdsrc-tauri/src/app/mod.rssrc-tauri/src/app/tray.rssrc/components/App.tsxsrc/components/PigDetail.tsxsrc/hooks/useViewport.ts
Comment on lines
+23
to
+24
| Quit ← calls app.exit(0) | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n issues/013-tray-icon-focus-list.mdRepository: killallgit/adhd-ranch
Length of output: 2922
🏁 Script executed:
rg -i "poll|thread|watcher|background" --type rust -A 3 src-tauri/src/ | head -80Repository: killallgit/adhd-ranch
Length of output: 2724
🏁 Script executed:
rg -B 5 -A 10 "std::thread::spawn" src-tauri/src/app/mod.rsRepository: killallgit/adhd-ranch
Length of output: 951
🏁 Script executed:
rg -B 2 -A 30 "loop {" src-tauri/src/app/mod.rs | head -100Repository: killallgit/adhd-ranch
Length of output: 141
🏁 Script executed:
rg "loop \{" src-tauri/src/app/mod.rs -A 30 | head -100Repository: killallgit/adhd-ranch
Length of output: 1411
🏁 Script executed:
cat -n src-tauri/src/app/tray.rsRepository: killallgit/adhd-ranch
Length of output: 4259
🏁 Script executed:
rg "struct WatcherHandles" src-tauri/src/app/mod.rs -A 5Repository: killallgit/adhd-ranch
Length of output: 203
🏁 Script executed:
rg "FocusWatcher|watch_path" src-tauri/src/ --type rust | head -30Repository: killallgit/adhd-ranch
Length of output: 458
🏁 Script executed:
rg "FocusWatcher|watch_path" src-tauri/src/ --type rust -B 3 -A 8 | head -80Repository: killallgit/adhd-ranch
Length of output: 1987
🏁 Script executed:
fd "watch" src-tauri/ --type f | grep -E "\.rs$"Repository: killallgit/adhd-ranch
Length of output: 47
🏁 Script executed:
find . -type f -name "*.rs" | xargs grep -l "pub fn watch_path"Repository: killallgit/adhd-ranch
Length of output: 97
🏁 Script executed:
cat -n crates/storage/src/watcher.rsRepository: killallgit/adhd-ranch
Length of output: 2991
🏁 Script executed:
rg "install_http_server|ServerHandle" src-tauri/src/app/mod.rs -B 3 -A 5Repository: killallgit/adhd-ranch
Length of output: 1117
🏁 Script executed:
rg "app.run|on_exit|exit" src-tauri/src/app/mod.rs -B 2 -A 5 | head -80Repository: killallgit/adhd-ranch
Length of output: 47
🏁 Script executed:
cat -n src-tauri/src/app/mod.rs | head -150Repository: killallgit/adhd-ranch
Length of output: 6867
🏁 Script executed:
cat -n src-tauri/src/app/mod.rs | tail -100Repository: killallgit/adhd-ranch
Length of output: 3912
The polling thread must have a graceful shutdown mechanism before app exit.
The spec's "Quit calls app.exit(0)" doesn't address the unmanaged polling thread spawned at lines 138–160 in src-tauri/src/app/mod.rs. This thread runs an infinite loop checking cursor position with no shutdown signal; calling app.exit(0) will forcefully terminate it rather than stopping it cleanly.
Either:
- Add a shutdown signal (atomic flag or channel) to the polling thread and document this in the spec, or
- Store the thread handle with a proper shutdown mechanism before exit.
The managed watchers and HTTP server (via app.manage()) will clean up automatically, but the spawned thread bypasses this lifecycle.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@issues/013-tray-icon-focus-list.md` around lines 23 - 24, The polling thread
that spins checking cursor position (the thread created via spawn in the tray
polling loop) has no shutdown signal and will be terminated abruptly when
app.exit(0) is called; add a graceful shutdown by introducing a shared shutdown
flag or channel (e.g., an Arc<AtomicBool> or a oneshot channel) that the spawned
thread checks each loop iteration, store the sender/flag or the thread
JoinHandle in the app state (similar to existing managed resources), and ensure
app.exit(0) sets the flag/sends the shutdown signal and joins the thread (or
waits for its clean exit) before exiting; update the code paths that call
app.exit(0) to trigger this shutdown sequence.
Comment on lines
+27
to
+31
| - **Live rebuild**: | ||
| - Subscribe to the existing `FOCUSES_CHANGED_EVENT` from within setup. | ||
| - On each event, re-read the focus store and call `tray.set_menu(Some(new_menu))`. | ||
| - Over-cap check runs the existing `CapEvaluator` to decide whether to swap the icon. | ||
| - **No tray popover window** — the overlay window launched at startup is the pig canvas; the tray is menu-only. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Guard “live rebuild on every focuses change” against event storms (menu flicker/thrash).
Right now the spec says to rebuild the native menu on each FOCUSES_CHANGED_EVENT by re-reading the focus store and calling tray.set_menu(Some(new_menu)). If the focus watcher fires multiple times per file write/delete (common on macOS + rename/atomic-write patterns), you can get rapid menu rebuilds/flicker and unnecessary work.
Consider specifying “coalesce/debounce focuses-changed events within a short window (e.g. 200–500ms), but ensure the rebuild still completes within ~1s of the user-visible change,” while keeping the acceptance criterion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@issues/013-tray-icon-focus-list.md` around lines 27 - 31, The live-rebuild
should not call tray.set_menu on every FOCUSES_CHANGED_EVENT; update the setup
subscription to coalesce/debounce focuses-change events (e.g., 200–500ms window)
before re-reading the focus store and rebuilding the menu so rapid event storms
don’t cause flicker/thrash; implement a timer/debounce mechanism inside the
setup event handler that batches events and triggers a single
tray.set_menu(Some(new_menu)) call (still ensuring the rebuild occurs within ~1s
of the user's change) and leave the existing CapEvaluator over-cap check
unchanged so the icon-swap logic still runs after the debounced rebuild.
Comment on lines
+80
to
+89
| ### FR4 — Menu bar item | ||
|
|
||
| - Tray icon in the macOS menu bar. | ||
| - Native NSMenu with: | ||
| - List of current Focuses (each as a menu item showing title). | ||
| - Clicking a Focus item → brings the pig into view / opens its detail (TBD). | ||
| - Separator. | ||
| - "+ New Focus" → opens a small webview popover for title + description input. | ||
| - Separator. | ||
| - "Quit" → Cmd-Q. |
There was a problem hiding this comment.
Scope FR4 to the tray behavior this PR actually ships.
This section documents + New Focus and clickable focus actions, but the implementation in this PR only builds the live focus list plus Quit. Keeping FR4 broader than the shipped behavior makes the v1.2 PRD misleading; move those items to the follow-up tray issues or mark them as future work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PRD.md` around lines 80 - 89, The PRD’s FR4 section is too broad for this PR:
reduce the scope to match the shipped tray behavior by removing or marking as
future work the unimplemented items ("+ New Focus" popover and clickable Focus
actions) and leave only the implemented live focus list and "Quit" behavior;
update the FR4 text to reflect that the NSMenu currently provides a list of
current Focuses and a Quit item, and add a short note such as "Additional tray
actions (New Focus popover, focus click behavior) are planned for follow-up
issues" so the v1.2 PRD accurately describes what this PR delivers.
Comment on lines
105
to
114
| `~/.adhd-ranch/settings.yaml`: | ||
| ```yaml | ||
| caps: | ||
| max_focuses: 5 | ||
| max_tasks_per_focus: 7 | ||
| alerts: | ||
| system_notifications: true | ||
| widget: | ||
| always_on_top: true | ||
| ``` |
There was a problem hiding this comment.
Add blank lines around the YAML fence.
This block is still tripping markdownlint MD031 because the fenced code block is not separated from surrounding prose by blank lines.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 106-106: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PRD.md` around lines 105 - 114, Add a blank line before and after the fenced
YAML block that begins with "`~/.adhd-ranch/settings.yaml`" and the triple
backticks so the block is separated from surrounding prose; open PRD.md, locate
the fenced code block that contains the YAML keys (caps, alerts, widget) and
insert one empty line above the opening ``` and one empty line below the closing
``` to satisfy markdownlint MD031.
This was referenced May 2, 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
1 participant
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.
Summary
set_ignore_cursor_eventsso only pig-area clicks registerfocuses-changedwatcher fan-out; Quit item exits cleanlyPigDetailcard opens near clicked pig: shows focus title + tasks with delete buttons; click-outside closesIssues
Closes #6 (013 — tray icon + live focus list)
Delivers 012 (pig overlay)
Test plan
task checkgreenSummary by CodeRabbit