[codex] add task timer dropdowns and AnimalDetail - #60
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
✨ Finishing Touches🧪 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 |
archae0pteryx
force-pushed
the
codex/fix-animal-timer-scale
branch
from
May 15, 2026 20:01
88dad10 to
7fdafc6
Compare
archae0pteryx
marked this pull request as ready for review
May 16, 2026 22:41
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/components/AnimalDetail.test.tsx (1)
186-194:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrap the
Date.nowspy intry/finallyto guarantee cleanup.Without
try/finally, if the assertion on line 193 throws,mockRestore()is never called. Vitest's configuration lacksrestoreAllMocks, so the mocked time persists and can leak into subsequent tests.Suggested patch
it("shows remaining time for a running timer", () => { const nowSpy = vi.spyOn(Date, "now").mockReturnValue(1_030_000); - renderDetail({ - focus: { - ...baseFocus, - timer: { duration_secs: 120, started_at: 1_000, status: "Running" }, - }, - }); - expect(screen.getByRole("button", { name: /edit focus timer/i })).toHaveTextContent("01:30"); - nowSpy.mockRestore(); + try { + renderDetail({ + focus: { + ...baseFocus, + timer: { duration_secs: 120, started_at: 1_000, status: "Running" }, + }, + }); + expect(screen.getByRole("button", { name: /edit focus timer/i })).toHaveTextContent("01:30"); + } finally { + nowSpy.mockRestore(); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AnimalDetail.test.tsx` around lines 186 - 194, The Date.now spy (nowSpy) in the test block around renderDetail should be cleaned up even if assertions fail; wrap the spy and the test actions/assertions in a try/finally so nowSpy.mockRestore() is always called. Locate the test that calls vi.spyOn(Date, "now") and renderDetail(...) (and the expect on the edit focus timer text) and move the expect and renderDetail into the try, with nowSpy.mockRestore() in the finally block to guarantee restoration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/storage/src/focus_store.rs`:
- Around line 219-220: The sidecar cleanup calls clear_expired_timer(focus_id)
and clear_fired_timer(focus_id) run after the primary mutation (writing
focus.md) has been committed but currently propagate errors with ?, which causes
the API to return Err even though the mutation succeeded; change these calls to
catch and log or convert their errors to non-fatal (e.g., match or map_err/log)
so the function returns Ok(()) after the primary write regardless of sidecar I/O
failures, while still recording the cleanup error for diagnostics (refer to
clear_expired_timer and clear_fired_timer and the surrounding commit logic).
In `@issues/053-task-timer-expiry-workflow.md`:
- Line 34: The "Blocked by" ordered-list entry currently reads "52." which
triggers a markdown list warning; update the "Blocked by" section in
issues/053-task-timer-expiry-workflow.md to use a plain issue reference instead
of a numbered list item (e.g., replace the "52." entry with a plain reference
like "`#52`" or "Blocked by: `#52`") so it matches the issues/README.md template and
other issue files.
In `@src/components/App.tsx`:
- Around line 70-94: The handler handleAddTask currently performs I/O (calls
focusWriter.appendTask and onWriteFailure) inside the App component; extract
that logic into a controller/hook (e.g., useFocusController) so components
remain view-only. Create a hook or controller function (e.g.,
useFocusController().appendTask or appendTaskController) that accepts focusId
and text, performs focusWriter.appendTask, calls onWriteFailure, and returns the
outcome; then update App's handleAddTask to only call the controller, build the
optimistic newTask, and call setOptimisticFocuses (removing direct focusWriter
usage and onWriteFailure calls from App). Also apply the same extraction for the
similar I/O block referenced around lines 116-126 so all focusWriter and fetch
side-effects live in the hook/controller (keep symbols: handleAddTask,
focusWriter.appendTask, onWriteFailure, setOptimisticFocuses, readerFocuses,
focuses, selectedFocus).
In `@src/components/TimerDropdown.tsx`:
- Around line 38-39: Toggle currently only flips open via setOpen, leaving any
custom-value validation state set; update the onClick handler used in
TimerDropdown (where setOpen is called) to also clear the custom-value
validation state when toggling — e.g., call the validation-state setter
(customError / setCustomError or whatever validation state exists) to set
no-error when toggling the dropdown so stale errors are removed on close/reopen,
and ensure this runs both when closing and opening the menu.
---
Outside diff comments:
In `@src/components/AnimalDetail.test.tsx`:
- Around line 186-194: The Date.now spy (nowSpy) in the test block around
renderDetail should be cleaned up even if assertions fail; wrap the spy and the
test actions/assertions in a try/finally so nowSpy.mockRestore() is always
called. Locate the test that calls vi.spyOn(Date, "now") and renderDetail(...)
(and the expect on the edit focus timer text) and move the expect and
renderDetail into the try, with nowSpy.mockRestore() in the finally block to
guarantee restoration.
🪄 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: 513e4ba8-052b-4f51-8f93-a405e4fb0ca7
⛔ Files ignored due to path filters (1)
src/types/generated/Task.tsis excluded by!**/generated/**
📒 Files selected for processing (32)
CHANGELOG.mdCONTEXT.mdPRD.mdREADME.mdcrates/commands/src/caps.rscrates/commands/src/focus.rscrates/domain/src/caps.rscrates/domain/src/focus.rscrates/domain/src/parse.rscrates/storage/src/focus_store.rsissues/051-ranch-animal-vocabulary-seam.mdissues/053-task-timer-expiry-workflow.mdissues/README.mdissues/done/030-pig-growth-expired-visual-tray-list.mdissues/done/052-task-timers-and-clock-dropdown-editing.mdsrc-tauri/src/app/mod.rssrc-tauri/src/app/tray.rssrc-tauri/src/ui_bridge/mod.rssrc/api/fixtureFocusWriter.tssrc/api/focusWriter.test.tssrc/api/focusWriter.tssrc/api/tauriFocusReader.test.tssrc/api/tauriFocusReader.tssrc/components/AnimalDetail.test.tsxsrc/components/AnimalDetail.tsxsrc/components/App.test.tsxsrc/components/App.tsxsrc/components/PigSprite.tsxsrc/components/TimerDropdown.tsxsrc/hooks/usePigMovement.test.tssrc/hooks/usePigMovement.tssrc/styles.css
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
Adds independent task timers alongside the existing focus-level timer, and consolidates timer editing into compact clock/time dropdowns. The clicked-animal detail surface has also been renamed from
PigDetailtoAnimalDetail, with matching CSS/test IDs and current docs updated.What changed
Task.timerto the domain and regenerated TypeScript type.task-timers.jsonsidecar, indexed alongside the markdown task list.TimerDropdownthat shows a clock icon when no timer is set and the current remaining time when one is set.PigDetailtoAnimalDetailand removes the heavy offset shadow that created the bubbly rounded artifact behind the detail card.Local issues
issues/done/052-task-timers-and-clock-dropdown-editing.mdfor this PR's slice.issues/053-task-timer-expiry-workflow.mdfor persisted Task timer expiry/notification semantics.Impact
Users can set separate timers per task while keeping the global focus timer. Timer editing now lives on the clock/time control instead of a permanently visible picker. Existing focus timers and task markdown remain compatible.
Validation
task testnpm run lintnpm run typechecknpm run test -- AnimalDetail AppNote
Task timers are persisted and displayed in the UI, but the existing background expiry notification workflow still only transitions focus-level timers to persisted
Expiredstate. That follow-up is now tracked in issue 053.Summary by CodeRabbit
New Features
Style
Documentation