Architecture deepening: reader collapse 037-040 + TimerTicker 044 + ADRs - #51
Merged
Conversation
Add architecture queue alongside priority queue. Two tracks: Frontend reader collapse (037-040, 045) — drafted previously plus new 045 closing the caps fixture gap so 040's generic Tauri factory has a symmetric fixture/Tauri pair. Domain seams (041-044) from this session's deepening audit: - 041 FocusWriter returns WriteOutcome (no more swallowed rejections) - 042 Unify cap-state behind one CapStatus projection (tray + notifier agree by construction) - 043 ProposalLifecycle::accept atomicity — HITL design, produces ADR - 044 Carve domain TimerTicker pure module ahead of 029 wiring Why: shallow modules and split error/transaction responsibility surfaced during architecture review. Each slice is independent and AFK except 043 which needs a design decision recorded as an ADR. 029 scope-expansion edit deliberately left unstaged — separate commit.
`crates/domain/src/timer_ticker.rs` — `tick(now_secs, focuses) -> Vec<TimerTransition>`. Pure: no I/O, no async, no clock read. Returns one transition per Focus with `timer.status == Running` and elapsed >= duration. Reuses `timer_remaining_secs` to avoid parallel arithmetic. `TimerStatus::Running` gate is the dedup lock against persisted Expired focuses — without it every tick after the first expiry would re-fire. Six unit tests cover the acceptance grid (no timer, not-yet, exactly-at, past, already-Expired, multiple-in-order). No `app/` wiring; issue 029 will consume `tick`.
029 — scope expansion already drafted pre-session: unify cap notifier onto the NotificationSource trait, delete `Alerts` struct, single notifications.sources toggle surface. Plus this session: require expiry detection consumes `adhd_ranch_domain::tick(...)` from 044; declare 044 as a blocker. 043 — tighten the 3-step transactional description against the actual shape of `accept()` at lifecycle.rs:41-52 + helper `apply()` at :64-90. Was conflating six calls with three writes; now names each write and the file it touches. 044 — drop misleading "mirrors `OverCapMonitor`'s shape: stateless detection" claim. `OverCapMonitor` is stateful (`Mutex<State>` for in-memory dedup). `TimerTicker` is stateless because dedup lives in persisted `timer.status == Expired`. Replace analogy with the contrast. 045 — factual rewrite. `createFixtureCapsReader` already exists at `src/api/caps.ts:23`. Real gap: `useAppState.test.tsx:14` reimplements the body locally instead of importing, AND no generic helper exists. Acceptance criteria updated to delete the local wrapper, not "add a factory that doesn't exist".
ADR-0001 records why. Verified against caps.rs:17 and both consumers (tray.rs:34,104 + commands/caps.rs:39-40): both derive from the same `cap_state(focuses, caps)` function. No two-path divergence to fix. 042 file deleted; README architecture queue trimmed; future audits should verify the projection function before asserting "two read paths". Also marks 044 as done in the queue (merged this session).
…ocus[]> `FocusReader` was the same shape as `PolledReader<readonly Focus[]>` minus a `list`/`read` rename. `useFocuses` was a 22-line rename hop (`value` → `focuses`). Two interfaces, one rename, no leverage. Delete: - `src/api/focuses.ts` (FocusReader + duplicate Unsubscribe export) - `src/hooks/useFocuses.ts` - `src/hooks/useFocuses.test.tsx` (its 2 behaviors are already covered by `src/hooks/usePolledReader.test.tsx` against a generic `<number>` reader) Rename: - `createTauriFocusReader` returns `PolledReader<readonly Focus[]>`, method `list` → `read` - `createFixtureFocusReader` same Update consumers (`App.tsx`, `useAppState.ts`, `useAppState.test.tsx`): - prop type → `PolledReader<readonly Focus[]>` - `useFocuses(reader)` → `usePolledReader(reader)` - ready state → `state.value` (was `state.focuses`) - `failingFocusReader` builder rewritten to the new shape Collateral: `Unsubscribe` was exported from both `src/api/focuses.ts` (the deleted file) and `src/hooks/usePolledReader.ts`. Redirected the two non-focus consumers (`src/api/proposals.ts`, `src/api/tauriProposalReader.ts`) to import from `usePolledReader`. This is not 038 scope — just keeping the proposal side compiling. `task check` green. 69/69 tests pass.
…ader<Proposal[]> Mirror of 037 for the proposal catalog. `ProposalReader` was the same shape as `PolledReader<readonly Proposal[]>` minus a `list`/`read` rename. `useProposals` was a 22-line rename hop (`value` → `proposals`). Delete: - `src/hooks/useProposals.ts` (no test file existed; `usePolledReader` tests already cover loading/ready/error/subscribe behaviors) - `ProposalReader` interface in `src/api/proposals.ts` (file retained for `ProposalDecisionResult`, `ProposalEdit`, `ProposalWriter`) Rename: - `createTauriProposalReader` returns `PolledReader<readonly Proposal[]>`, method `list` → `read` - `createFixtureProposalReader` same Update consumers (`useAppState.ts`, `useAppState.test.tsx`): - prop type → `PolledReader<readonly Proposal[]>` - `useProposals(reader)` → `usePolledReader(reader)` - ready state → `state.value` (was `state.proposals`) - `failingProposalReader` builder rewritten to the new shape `task check` green. 69/69 tests pass.
…aps> `useCaps` returned `Caps` directly with a silent `DEFAULT_CAPS` fallback on error and no loading state — a different pattern than the focus and proposal readers (now both `PolledReader<T>`-shaped via 037/038). Two patterns for the same job. Unify on `usePolledReader<Caps>`. UX preserved (silent default fallback, no loading flash) by mapping the polled state at the call site: const caps = capsState.status === "ready" ? capsState.value : DEFAULT_CAPS; Delete: - `src/hooks/useCaps.ts` - `CapsReader` interface in `src/api/caps.ts` (factories now return `PolledReader<Caps>` directly; `subscribe?` omitted because caps are not pushed by Tauri) Rename: - `createTauriCapsReader` / `createFixtureCapsReader` return `PolledReader<Caps>`; method `get` → `read` Collateral (folds part of 045): - `src/hooks/useAppState.test.tsx:14` local `fixtureCapsReader` wrapper deleted; tests now use `createFixtureCapsReader(fixtureCaps)` directly - `CapsReader` import removed (type no longer exists) `task check` green. 69/69 tests pass.
After 037/038/039 unified the three Tauri readers on `PolledReader<T>`,
each `createTauri*Reader` repeated the same plumbing: `invoke<Raw>(key)`,
optionally `listen(event)`, optionally a `Raw → Out` map. Three copies.
Consolidate into one factory at `src/api/tauriReader.ts`:
createTauriReader<Raw, Out>({ invokeKey, eventKey?, map })
-> PolledReader<Out>
Per-domain files now contain only the channel name, optional event name,
the `Raw → Out` mapping, and any Rust shape interfaces. No `invoke`,
`listen`, or `Unsubscribe` plumbing duplicated.
- `tauriFocusReader.ts` — `Raw = readonly RustFocus[]`, map calls
`fromRust` per element; keeps `RustFocus` + `fromRust` co-located
- `tauriProposalReader.ts` — `Raw = readonly Proposal[]`, identity map;
`createTauriProposalWriter` unchanged
- `caps.ts` — `Raw = Caps`, identity map; no `eventKey`
`task check` green. 69/69 tests pass.
ADR-0002 records why. Each fixture factory is 3 lines of
`{ read: () => Promise.resolve(value) }`; a generic helper would
deduplicate one line across three call sites — zero leverage gain
per CLAUDE.md ("No abstractions for single-use code").
Issue 040's own "Out of scope" note already documented this. The
second piece of 045 — deleting the test wrapper at
useAppState.test.tsx:14 and importing createFixtureCapsReader
directly — was folded into the 039 commit.
045 file deleted; README queue updated; 037-040 marked done.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR consolidates three separate reader/hook pairs (FocusReader/useFocuses, ProposalReader/useProposals, CapsReader/useCaps) into a unified PolledReader abstraction, introduces a generic Tauri reader factory, adds a pure domain timer-expiry detection module, and documents architecture decisions via ADRs and issue specifications. ChangesFrontend Reader Consolidation (037–040)
Timer Ticker Domain Module (044)
Architecture Documentation & Decisions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
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)
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: 2
🤖 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 `@issues/044-domain-timer-ticker.md`:
- Line 21: The spec and implementation disagree on tick's timestamp type: change
the spec signature pub fn tick(now_secs: u64, focuses: &[Focus]) ->
Vec<TimerTransition>; to use i64 (pub fn tick(now_secs: i64, focuses: &[Focus])
-> Vec<TimerTransition>), keeping the implementation's signed timestamp; update
any references or docs that rely on the spec signature to use now_secs: i64 and
ensure types align with the implementation of tick.
In `@src/api/fixtureProposalReader.ts`:
- Line 1: Move the PolledReader type out of the hooks layer into a new shared
boundary module (e.g., create a shared types module and export PolledReader from
it), then update the import sites in fixtureProposalReader.ts, caps.ts,
tauriFocusReader.ts, and tauriProposalReader.ts to import PolledReader from that
shared module instead of from ../hooks/usePolledReader; also update the hook
implementation to import PolledReader from the same shared module so that
src/hooks handles state/effects only and api files no longer depend on hooks.
🪄 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: 38e3ba56-ea5f-4b3b-a84a-1fcef50a922c
📒 Files selected for processing (29)
crates/domain/src/lib.rscrates/domain/src/timer_ticker.rsdocs/adr/0001-cap-state-is-already-a-single-projection.mddocs/adr/0002-fixture-readers-are-too-thin-for-a-generic-helper.mdissues/029-timer-expiry-and-notification-interface.mdissues/037-collapse-focus-reader.mdissues/038-collapse-proposal-reader.mdissues/039-unify-caps-on-polled-reader.mdissues/040-generic-tauri-reader-factory.mdissues/041-focus-writer-write-outcome.mdissues/043-proposal-lifecycle-atomicity-design.mdissues/044-domain-timer-ticker.mdissues/README.mdsrc/api/caps.tssrc/api/fixtureFocusReader.test.tssrc/api/fixtureFocusReader.tssrc/api/fixtureProposalReader.tssrc/api/focuses.tssrc/api/proposals.tssrc/api/tauriFocusReader.tssrc/api/tauriProposalReader.tssrc/api/tauriReader.tssrc/components/App.tsxsrc/hooks/useAppState.test.tsxsrc/hooks/useAppState.tssrc/hooks/useCaps.tssrc/hooks/useFocuses.test.tsxsrc/hooks/useFocuses.tssrc/hooks/useProposals.ts
💤 Files with no reviewable changes (5)
- src/hooks/useProposals.ts
- src/hooks/useCaps.ts
- src/hooks/useFocuses.test.tsx
- src/api/focuses.ts
- src/hooks/useFocuses.ts
Resolves two CodeRabbit findings on PR #51. 1. (Major) `src/api/*` was importing `PolledReader` and `Unsubscribe` from `src/hooks/usePolledReader`, inverting the CLAUDE.md dependency direction (`hooks/` calls `api/`, not the other way). The contract the api/ readers implement belongs in api/. - New `src/api/polledReader.ts` owns `PolledReader<T>` + `Unsubscribe` - `src/hooks/usePolledReader.ts` keeps `usePolledReader<T>` hook and `PolledState<T>` discriminated union; imports `PolledReader` from api/polledReader - All 6 api/ files now import from `./polledReader`; no api/ file references hooks/ - Components and the hook tests import the type from `api/polledReader` and the hook from `hooks/usePolledReader` 2. (Minor) Issue 044's spec block showed `now_secs: u64`; implementation uses `i64`. Unix timestamps are signed; matched the spec to the impl (no code change). `task check` green. 69/69 frontend tests pass.
This was referenced May 12, 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
Architecture deepening pass on the frontend reader layer and the domain timer module, plus issue-queue hygiene.
Frontend reader collapse (037 → 038 → 039 → 040):
FocusReader,ProposalReader,CapsReaderall merged intoPolledReader<T>; the correspondinguseFocuses/useProposals/useCapsrename hops are deleted (their behaviors are covered byusePolledReader.test.tsx).createTauriReader<Raw, Out>({ invokeKey, eventKey?, map })factory insrc/api/tauriReader.tsbacks all three Tauri-side readers. Per-domain files contain only channel name + optional event name +Raw → Outmapping.Domain
TimerTicker(044):crates/domain/src/timer_ticker.rsexposing puretick(now_secs, &[Focus]) -> Vec<TimerTransition>. Six table-driven tests cover the acceptance grid (no timer, not-yet-expired, exactly-at-expiry, past expiry, already-Expired, multiple in input order).NotificationSourceslice) is updated to consumetick— its imperative shell shrinks toread store → tick → for each transition: store.update_timer + Tauri emit + notification.Issue queue + ADRs:
TimerTicker); 043 step-list tightened against actuallifecycle.rsshape; 044 analogy corrected (OverCapMonitoris stateful viaMutex<State>;TimerTickeris stateless because dedup lives in persistedtimer.status == Expired); 045 factually rewritten.cap_stateis already the single projection bothtray.rsandCapEvaluatorconsume; no refactor warranted.Promise.resolve(value)factories; a generic helper would deduplicate one line per use site with zero leverage gain. Test-wrapper deletion piece already folded into 039.Net code: roughly −274 LOC across the 9 commits (collapse + close).
Test plan
task checkgreen at each commit boundaryuseFocusestests were already covered byusePolledReader.test.tsxagainst a generic<number>reader)cargo test: 90 / 90 passing acrossadhd-ranch-domain,adhd-ranch-storage,adhd-ranch-commands,adhd-ranch-http-apiIssue trace
Summary by CodeRabbit
New Features
Documentation
Refactor