feat: v1.3 — new producers, integration tests, TUI weather#10
Merged
Conversation
…y fixes Phase 0B: Integration test infrastructure - Pipeline wiring tests (producer → cache → segment) - Dispatcher wiring tests (dispatch → side-effect handlers) - Status line flow tests (cache → renderer → output) Phase 1: P0 bug fixes - GH#8: Practice feed producer (tracks coding practice sessions) - GH#9: Analytics engine wired into dispatcher Phase 3 Phase 2: P0 features - Calendar OAuth setup with reduced friction (hookwise setup calendar) - TUI auto-launch from SessionStart hook (tui-launcher.ts) - npm distribution: scripts/ added to package files - Security: credentials written with 0o600 permissions - macOS TUI launch via osascript (fixes Terminal.app arg issue) Phase 3: P1 features - Weather feed producer (Open-Meteo API, no key needed) - Memories/On This Day producer (date-windowed analytics queries) - TUI animated weather background (9 conditions, parallax layers) - TUI weather data bridge (supports both camelCase + snake_case schemas) Test count: 1440 TypeScript + 47 Python = 1487 total Producers: 8 builtin (pulse, project, calendar, news, insights, practice, weather, memories) Segments: 21 status line segments Version: 1.3.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds three new feeds (practice, weather, memories), TUI auto-launch and launcher, analytics integration, calendar OAuth setup flow, a weather-animated TUI background, config/type expansions (calendar token/credentials, tui), and extensive unit/integration tests and README rewrite. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Dispatch
participant Dispatcher as Dispatcher
participant Cache as Cache Bus
participant Analytics as Analytics Engine
participant TUI as TUI Launcher
participant Producer as Feed Daemon
CLI->>Dispatcher: dispatch(payload)
Dispatcher->>Cache: merge heartbeat / cwd
Dispatcher->>Producer: ensure feeds/daemon running
Dispatcher->>Analytics: getAnalyticsEngine(config)
Analytics-->>Dispatcher: engine|null
Dispatcher->>Cache: merge feed results (from Daemon)
Dispatcher->>Analytics: executeAnalytics(EventType, payload)
Analytics-->>Cache: write analytics rows (async)
alt payload is SessionStart and config.tui.autoLaunch
Dispatcher->>TUI: launchTui(config.tui)
TUI-->>Dispatcher: success/fail (non-blocking)
end
Dispatcher-->>CLI: exit (fail-open on non-critical errors)
sequenceDiagram
participant Daemon as Feed Daemon
participant Producer as Feed Producer
participant Cache as Cache Bus
participant Renderer as Segment Renderer
participant Status as Status Line
Daemon->>Producer: periodic tick
Producer->>Producer: query DB / fetch API
Producer-->>Daemon: feed data (or null)
Daemon->>Cache: mergeKey(feedName, data, ttl)
Renderer->>Cache: readKey(feedName)
Cache-->>Renderer: cache entry
alt entry fresh
Renderer->>Status: formatted segment output
else stale/missing
Renderer-->>Status: empty
end
Status->>Display: render status line
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/core/feeds/segments.test.ts (1)
517-527:⚠️ Potential issue | 🟡 MinorRegistry assertion became count-based, not identity-based, for new segments.
The test would still pass if the two new keys were wrong names. Add explicit checks for
weatherandmemories(or assert exact key set).Suggested test tightening
const expected = [ "clock", "mantra", "builder_trap", "session", "practice", "ai_ratio", "cost", "streak", "context_bar", "mode_badge", "duration", "practice_breadcrumb", "pulse", "project", "calendar", "news", "insights_friction", "insights_pace", "insights_trend", + "weather", "memories", ]; @@ - expect(Object.keys(BUILTIN_SEGMENTS)).toHaveLength(21); + expect(Object.keys(BUILTIN_SEGMENTS)).toHaveLength(expected.length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/feeds/segments.test.ts` around lines 517 - 527, The test currently validates BUILTIN_SEGMENTS only by count and a partial expected list, which can miss wrong names; update the assertion to either include explicit checks for the two new keys ("weather" and "memories") in the expected array and adjust the expected length accordingly, or replace the current length + partial checks with an exact key-set assertion that compares Object.keys(BUILTIN_SEGMENTS) to the full expected list (including "weather" and "memories"); reference BUILTIN_SEGMENTS and the existing expected array in segments.test.ts to locate where to modify the assertions.src/cli/commands/status.tsx (1)
58-65:⚠️ Potential issue | 🟠 MajorStatus command omits two configured built-in feeds.
WeatherandMemoriesare part ofFeedsConfigbut are not included in the rendered feed status list, so CLI output can misreport actual configuration.Suggested fix
feeds: [ { name: "Pulse", enabled: config.feeds.pulse.enabled }, { name: "Project", enabled: config.feeds.project.enabled }, { name: "Calendar", enabled: config.feeds.calendar.enabled }, { name: "News", enabled: config.feeds.news.enabled }, { name: "Insights", enabled: config.feeds.insights.enabled }, { name: "Practice", enabled: config.feeds.practice.enabled }, + { name: "Weather", enabled: config.feeds.weather.enabled }, + { name: "Memories", enabled: config.feeds.memories.enabled }, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/status.tsx` around lines 58 - 65, The feeds array in the status command omits the built-in "Weather" and "Memories" feeds defined on FeedsConfig, causing the CLI to misreport configuration; update the feeds list in the status rendering (the feeds array in status.tsx) to include entries for "Weather" and "Memories" using config.feeds.weather.enabled and config.feeds.memories.enabled (matching the existing pattern for "Pulse"/"Project"/etc.) so the rendered feed status reflects all configured built-in feeds.tests/cli/doctor-feeds.test.tsx (1)
82-138: 🧹 Nitpick | 🔵 TrivialTest feed fixture is drifting from current feed schema.
The helper now includes
practicebut still omitsweatherandmemories, which can make feed-enabled/disabled scenarios less representative and more brittle.Suggested fixture extension
function writeConfigWithFeeds(overrides: { pulseEnabled?: boolean; projectEnabled?: boolean; calendarEnabled?: boolean; newsEnabled?: boolean; insightsEnabled?: boolean; practiceEnabled?: boolean; + weatherEnabled?: boolean; + memoriesEnabled?: boolean; } = {}): void { const { pulseEnabled = true, projectEnabled = true, calendarEnabled = false, newsEnabled = false, insightsEnabled = false, practiceEnabled = false, + weatherEnabled = false, + memoriesEnabled = false, } = overrides; @@ " practice:", ` enabled: ${practiceEnabled}`, " interval_seconds: 120", " db_path: ~/.practice-tracker/practice-tracker.db", + " weather:", + ` enabled: ${weatherEnabled}`, + " interval_seconds: 300", + " latitude: 47.6062", + " longitude: -122.3321", + " temperature_unit: fahrenheit", + " memories:", + ` enabled: ${memoriesEnabled}`, + " interval_seconds: 3600", + " db_path: ~/.hookwise/analytics.db",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli/doctor-feeds.test.tsx` around lines 82 - 138, The test fixture writeConfigWithFeeds is missing the newer feeds `weather` and `memories`; extend the function signature to accept weatherEnabled and memoriesEnabled in the overrides and add them to the destructured defaults (e.g., const { ..., practiceEnabled = false, weatherEnabled = false, memoriesEnabled = false } = overrides), then append corresponding YAML blocks in the configYaml array (include ` weather:` and ` memories:` entries with `enabled: ${weatherEnabled}` and `enabled: ${memoriesEnabled}` respectively and minimal required fields such as an interval_seconds or other simple defaults consistent with the existing entries) so tests reflect the current feed schema while leaving defaults false.src/cli/commands/doctor.tsx (1)
140-147:⚠️ Potential issue | 🟠 MajorFeed daemon check is incomplete for newly supported feeds
anyFeedEnabledstill excludesweatherandmemories. When either is the only enabled feed, the daemon check is skipped.✅ Proposed fix
const anyFeedEnabled = loadedConfig.feeds.pulse.enabled || loadedConfig.feeds.project.enabled || loadedConfig.feeds.calendar.enabled || loadedConfig.feeds.news.enabled || loadedConfig.feeds.insights.enabled || loadedConfig.feeds.practice.enabled || + loadedConfig.feeds.weather.enabled || + loadedConfig.feeds.memories.enabled || loadedConfig.feeds.custom.some((f) => f.enabled);As per coding guidelines, "Doctor command checks must be kept in sync with actual requirements."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/doctor.tsx` around lines 140 - 147, The anyFeedEnabled check in doctor.tsx (variable anyFeedEnabled) is missing the newly supported feeds so the daemon check can be skipped when only weather or memories are enabled; update the boolean expression that builds anyFeedEnabled to also include loadedConfig.feeds.weather.enabled and loadedConfig.feeds.memories.enabled (and, if applicable, any items in loadedConfig.feeds.memories.custom or weather.custom arrays) so the doctor command correctly detects those feeds as requiring the daemon.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 91: Update the stale release metrics in the README by replacing the old
counts in the "Status Line" sentence (the line that reads "**[Status
Line](docs/features/status-line.md)** -- 18 composable segments powered by a
[background daemon](docs/features/feeds.md) with 5 feed producers.") and the
similar occurrence later that lists counts — change "18" to "21", "5" to "8",
and update the tests count to "1,487" wherever the previous numbers are
referenced so both instances reflect the PR's stated numbers.
In `@src/cli/commands/setup.ts`:
- Around line 163-167: The catch blocks that handle internal/setup failures
currently set process.exitCode = 1; change them to set process.exitCode = 2 to
indicate internal errors (not user input errors). Locate the catch (error:
unknown) handlers around the credentials write logic (the block that logs
"Failed to write credentials file") and the other similar catch at lines
~191-195, and update process.exitCode from 1 to 2 while keeping the existing
error message construction (const msg = error instanceof Error ? error.message :
String(error)) and return behavior unchanged.
In `@src/core/config.ts`:
- Around line 843-854: The config validation currently checks raw.tui and
validates tui.launch_method but omits type checking for tui.auto_launch; add a
boolean type check for auto_launch inside the same block that inspects raw.tui
(where raw.tui is cast to const tui = raw.tui as Record<string, unknown>),
rejecting non-boolean values by pushing an error with path "tui.auto_launch" and
a message like `Invalid auto_launch: expected boolean` (include a suggestion to
use true/false). Ensure you support both snake_case and camelCase keys if needed
(tui.auto_launch and tui.autoLaunch) and only validate when the property is
defined.
- Around line 257-273: validateConfig currently lacks checks for the new feed
sections (practice, weather, memories); add validation logic inside
validateConfig to enforce: for practice and memories ensure enabled is boolean,
intervalSeconds is a positive integer and dbPath is a non-empty string (compare
memories.dbPath against DEFAULT_DB_PATH allowance), and for weather ensure
enabled is boolean, intervalSeconds is positive, latitude is within -90..90,
longitude within -180..180, and temperatureUnit is one of the allowed values
("fahrenheit","celsius"). Return or throw the same style of validation error
used elsewhere in validateConfig when any of these checks fail and include the
section name (practice/weather/memories) in the error message so failures are
easy to trace.
In `@src/core/dispatcher.ts`:
- Around line 499-516: The TUI launch (launchTui call guarded by
config.tui?.autoLaunch) and analytics engine initialization (getAnalyticsEngine)
occur before guard/context phases, violating the dispatcher three-phase
contract; move both side-effectful operations into Phase 3 (after guards and
context resolution and after the early-exit that checks handlers.length and
config.guards) so they only run when dispatch proceeds past guards/context;
specifically remove the launchTui and getAnalyticsEngine calls from their
current location near getHandlersForEvent and instead invoke launchTui and
initialize analyticsEngine in the Phase 3 side-effect block (the existing Phase
3 area that runs handlers), ensuring any try/catch for launchTui is preserved
and analytics initialization is skipped when early-exit conditions (no handlers,
no guards, no analytics needed) hold.
- Around line 45-47: When analyticsConfig.enabled is false, getAnalyticsEngine
currently returns null without closing any previously cached AnalyticsEngine,
leaking resources; modify getAnalyticsEngine to, before returning null, check
for the cached analytics engine (the module-level cached AnalyticsEngine
instance), call its shutdown/close method (await if async) and clear the cache
reference, then return null; reference getAnalyticsEngine, AnalyticsConfig and
the cached AnalyticsEngine variable so you locate and close the existing engine
instance properly.
In `@src/core/feeds/producers/memories.ts`:
- Around line 93-96: The code mixes UTC and local date getters causing
off-by-one-day errors; update all date calculations to use UTC consistently:
replace local getters used to build month, day, monthDay and todayUtc with UTC
equivalents (e.g., use getUTCMonth/getUTCDate/getUTCFullYear or construct via
Date.UTC) and recompute daysSince using those UTC-based dates so todayStr,
monthDay, todayUtc and the daysSince calculation all derive from the same UTC
"now"; apply the same change to the other occurrence referenced (lines ~150-153)
to ensure consistency.
In `@src/core/feeds/producers/practice.ts`:
- Around line 61-68: The code computes 'today' using new Date().toISOString()
(UTC) which skews local-day metrics; update the logic in practice.ts where
'today' and the related queries (the SELECT COUNT(*) in the block using
practiced_at >= ? and the similar queries around the 80-85 region) to compute
the local date boundary and pass a local-time ISO timestamp or use SQLite's
localtime functions so comparisons use local-day boundaries; locate the variable
named today and the db.prepare(...) calls (queries against
practice_reps.practiced_at) and change them to construct the start-of-day
timestamp in the user's local timezone (or use "datetime(..., 'localtime')" in
the SQL) so counts and due-review calculations reflect local days.
In `@src/core/status-line/segments.ts`:
- Around line 389-390: The weather-related segments currently return an empty
string when cache is missing/stale (checks using weatherData and isFresh and the
fields temperature/emoji), but by contract they must render a sensible fallback
instead of disappearing; update the two segments (the block checking if
(!weatherData || !isFresh(weatherData)) and the block checking
weatherData.temperature === undefined || !weatherData.emoji) to return a clear
fallback string (e.g., a short placeholder like "Weather: —", "N/A", or a
localized icon/text) rather than "", or call the module’s existing fallback
helper if one exists, ensuring both branches (missing/stale cache and missing
fields) produce that fallback output consistently.
In `@src/core/tui-launcher.ts`:
- Around line 137-188: The duplicate-prevention race exists because
isTuiRunning(effectivePath) is checked before spawning but writeTuiPid(pid,
effectivePath) happens after spawn; change the background path (when
config.launchMethod !== "newWindow") to atomically reserve the PID file before
calling spawn by opening/creating the PID file with exclusive flag ("wx") (use
the same writeTuiPid/backing helper or add a reserveTuiPid function), only
proceed to spawn python3 -m hookwise_tui if the atomic reservation succeeds, and
on spawn failure or if child.pid is null ensure you remove/release the reserved
PID file so other callers can proceed; keep newWindow behavior unchanged and
still skip PID reservation for that mode.
In `@tests/core/feeds/daemon-process.test.ts`:
- Around line 9-10: Update the test header comment that currently reads "6
built-in feeds" to the correct "8 built-in feeds" so the suite description
matches the actual assertions; locate the header near the tests referencing
registerBuiltinFeeds/registerCustomFeeds in the daemon-process.test.ts test file
and change only the numeric count in the comment.
In `@tests/core/feeds/producers/memories.test.ts`:
- Around line 258-270: The test is non-deterministic because it doesn't freeze
time or assert the expected null result; update the test that uses insertSession
and queryMemoriesData to freeze the clock to a known reference date (so
7d/30d/same-month-day are deterministic), insert the non-target session
(session-random) relative to that frozen date, call queryMemoriesData(dbPath),
and assert that the function returns null (expect(result).toBeNull()) for the
non-matching date; use the test suite’s time-mocking utility (or
jest.useFakeTimers/setSystemTime) to control time and restore it after the test.
In `@tests/integration/helpers.ts`:
- Around line 73-75: The shallow spread merge in the function returning "{
...base, ...overrides } as HooksConfig" can drop nested keys (e.g.,
overrides.feeds replacing base.feeds and making accesses like
config.feeds.pulse.enabled undefined); change this to a deep merge of base and
overrides (use a deep merge helper or lodash.merge) so nested sections are
merged recursively, keeping existing base fields when overrides only specify
partial nested values; update the function that returns HooksConfig (and any
helper used by createTestEnv()) to call the deep merge on base and overrides and
return the merged result.
In `@tests/integration/pipeline-wiring.test.ts`:
- Around line 194-203: The test duplicates the producer->segments map in
multiple places; extract the map into a single shared constant
(PRODUCER_TO_SEGMENTS) at the top of the test module and replace the in-place
literal maps used in each orphan-detection/assertion block with references to
that constant (used by the assertion helpers around the orphan-detection logic).
Ensure all four duplicated occurrences (the blocks around lines where
orphan-detection assertions appear) import or reference PRODUCER_TO_SEGMENTS so
updates to producers/segments only need to be made in one place.
In `@tests/integration/pipeline.test.ts`:
- Around line 167-172: The test currently uses a loose assertion
expect(allFeeds.length).toBeGreaterThanOrEqual(6) which can hide regressions;
change this to assert the exact expected built-in feed count by replacing that
line with expect(allFeeds.length).toBe(6) (reflecting the six built-ins: pulse,
project, calendar, news, insights, practice) and keep the subsequent checks that
verify enabled feeds via registry.getEnabled() and enabledNames for
completeness.
In `@tui/hookwise_tui/app.py`:
- Around line 21-26: The _CONDITION_TO_WEATHER mapping is missing an explicit
"sun" key so WeatherInfo.condition of "sun" falls back to the default (causing
the rain background); add an entry mapping "sun" -> "sun" to the
_CONDITION_TO_WEATHER dict (update the _CONDITION_TO_WEATHER constant) so
WeatherInfo.condition values correctly resolve to the sun scene.
- Around line 100-108: The on_mount bootstrap calls get_weather() without error
handling so a failed lookup can abort startup or prevent setting a safe
background; wrap the get_weather() call in a try/except (or use a safe fallback)
and if it raises or returns None/unexpected data, set initial to a safe default
(e.g., "rain") via the same _CONDITION_TO_WEATHER lookup and assign it to
bg.weather; update the method around on_mount, get_weather,
_CONDITION_TO_WEATHER, WeatherBackground, bg.weather, and the
query_one("#weather-bg") usage to ensure failure-to-fetch falls back to the
default and does not block app startup.
In `@tui/hookwise_tui/widgets/weather_background.py`:
- Around line 339-365: The particle update loop double-applies movement for
clear/sun dust motes because the generic "else" branch moves all
non-snow/fog/clear-twinkle particles and then the subsequent "if self.weather in
('clear', 'sun') and p.layer == 1" block moves layer-1 dust motes again; fix by
making dust-mote handling exclusive (either handle clear/sun layer==1 in an elif
branch instead of a separate if, or add a continue immediately after the generic
else branch when p matches dust-mote criteria) so that particles in
self.particles are only moved once (refer to the loop using self.particles, the
weather checks, p.layer, and the dust-mote block).
In `@tui/hookwise_tui/widgets/weather_data.py`:
- Around line 97-133: The parser in this snippet can fabricate valid weather
from malformed cache entries and will raise on direct numeric casts; update the
parsing logic in the function that builds WeatherInfo (referencing
weather_entry, WeatherInfo, VALID_CONDITIONS, WEATHER_CODE_MAP) to defensively
validate and coerce inputs: sanitize description before deriving condition, only
accept condition if it matches VALID_CONDITIONS (otherwise map from a safely
parsed integer code), parse numeric fields using safe helpers (e.g.,
safe_float/safe_int implemented with try/except and fallbacks) and treat
non-numeric strings like "n/a" as missing, and only return a WeatherInfo when at
least one of condition or a valid temperature/code is present; otherwise return
None. Ensure wind_speed, temp_c, temp_f and code are assigned from the safe
parsers and never cast directly from possibly-malformed values.
In `@tui/tests/test_weather.py`:
- Around line 444-451: The tests bind an unused async context variable named
"pilot" (e.g., in test_app_launches_with_weather_bg) which triggers
unused-variable linter errors; change the context variable name from "pilot" to
"_" (or "_pilot") in those async with app.run_test() as ... blocks (including
the other test around lines 481-488) so the variable is intentionally ignored
while leaving the rest of the test logic (HookwiseTUI instantiation,
app.run_test usage, and assertions querying "#weather-bg" and WeatherBackground)
unchanged.
- Around line 359-369: In test_weather_change_respawns, remove the unused local
assignments rain_count and snow_count (they are assigned from len(bg.particles)
but never used) to satisfy Ruff F841; update the test by deleting those two
variables and simply perform the state changes and calls on the
WeatherBackground instance (methods _spawn_all and watch_weather) so the test
still validates behavior without creating unused locals.
- Around line 453-467: The test captures initial_weather but never uses it;
update test_weather_cycle_key to assert the weather actually changed by
comparing bg.weather before and after pressing "w" (i.e., assert new_weather !=
initial_weather) — reference the HookwiseTUI instance, WeatherBackground query
"#weather-bg", and the variables initial_weather and new_weather to locate and
change the assertion logic so the test verifies the transition.
---
Outside diff comments:
In `@src/cli/commands/doctor.tsx`:
- Around line 140-147: The anyFeedEnabled check in doctor.tsx (variable
anyFeedEnabled) is missing the newly supported feeds so the daemon check can be
skipped when only weather or memories are enabled; update the boolean expression
that builds anyFeedEnabled to also include loadedConfig.feeds.weather.enabled
and loadedConfig.feeds.memories.enabled (and, if applicable, any items in
loadedConfig.feeds.memories.custom or weather.custom arrays) so the doctor
command correctly detects those feeds as requiring the daemon.
In `@src/cli/commands/status.tsx`:
- Around line 58-65: The feeds array in the status command omits the built-in
"Weather" and "Memories" feeds defined on FeedsConfig, causing the CLI to
misreport configuration; update the feeds list in the status rendering (the
feeds array in status.tsx) to include entries for "Weather" and "Memories" using
config.feeds.weather.enabled and config.feeds.memories.enabled (matching the
existing pattern for "Pulse"/"Project"/etc.) so the rendered feed status
reflects all configured built-in feeds.
In `@tests/cli/doctor-feeds.test.tsx`:
- Around line 82-138: The test fixture writeConfigWithFeeds is missing the newer
feeds `weather` and `memories`; extend the function signature to accept
weatherEnabled and memoriesEnabled in the overrides and add them to the
destructured defaults (e.g., const { ..., practiceEnabled = false,
weatherEnabled = false, memoriesEnabled = false } = overrides), then append
corresponding YAML blocks in the configYaml array (include ` weather:` and `
memories:` entries with `enabled: ${weatherEnabled}` and `enabled:
${memoriesEnabled}` respectively and minimal required fields such as an
interval_seconds or other simple defaults consistent with the existing entries)
so tests reflect the current feed schema while leaving defaults false.
In `@tests/core/feeds/segments.test.ts`:
- Around line 517-527: The test currently validates BUILTIN_SEGMENTS only by
count and a partial expected list, which can miss wrong names; update the
assertion to either include explicit checks for the two new keys ("weather" and
"memories") in the expected array and adjust the expected length accordingly, or
replace the current length + partial checks with an exact key-set assertion that
compares Object.keys(BUILTIN_SEGMENTS) to the full expected list (including
"weather" and "memories"); reference BUILTIN_SEGMENTS and the existing expected
array in segments.test.ts to locate where to modify the assertions.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (40)
README.mdpackage.jsonscripts/calendar-feed.pysrc/cli/commands/doctor.tsxsrc/cli/commands/init.tsxsrc/cli/commands/setup.tssrc/cli/commands/status.tsxsrc/core/config.tssrc/core/constants.tssrc/core/dispatcher.tssrc/core/feeds/daemon-process.tssrc/core/feeds/producers/memories.tssrc/core/feeds/producers/practice.tssrc/core/feeds/producers/weather.tssrc/core/status-line/segments.tssrc/core/tui-launcher.tssrc/core/types.tstests/cli/doctor-feeds.test.tsxtests/cli/init.test.tsxtests/cli/setup.test.tstests/core/feeds-config.test.tstests/core/feeds/daemon-process.test.tstests/core/feeds/producers/memories.test.tstests/core/feeds/producers/practice.test.tstests/core/feeds/producers/weather.test.tstests/core/feeds/segments.test.tstests/core/status-line/segments-new.test.tstests/core/tui-launcher.test.tstests/integration/dispatcher-wiring.test.tstests/integration/helpers.tstests/integration/pipeline-wiring.test.tstests/integration/pipeline.test.tstests/integration/status-line-flow.test.tstui/hookwise_tui/app.pytui/hookwise_tui/app.tcsstui/hookwise_tui/data.pytui/hookwise_tui/widgets/__init__.pytui/hookwise_tui/widgets/weather_background.pytui/hookwise_tui/widgets/weather_data.pytui/tests/test_weather.py
Batch A — Core Engine (6 fixes): - Move TUI launch + analytics init into Phase 3 (was before guards) - Close cached analytics engine when analytics disabled - Add config validation for practice/weather/memories feeds - Add boolean validation for tui.auto_launch - Fix TOCTOU race in TUI PID duplicate prevention (atomic O_EXCL) - Use exit code 2 for internal failures in setup.ts Batch B — Producers (2 fixes): - Use UTC getters consistently in memories.ts (getUTCMonth/getUTCDate) - Document UTC-intentional day boundary in practice.ts Batch C — Segments & Status (3 fixes): - Add weather/memories to status command feed list - Weather segment returns "🌤️ --" fallback instead of empty string - Add weather/memories to doctor.tsx anyFeedEnabled check Batch D — TUI Python (4 fixes): - Add _safe_float/_safe_int helpers for untrusted cache data - Fix double movement update for dust motes in weather_background - Add "sun" key to _CONDITION_TO_WEATHER map - Wrap get_weather() in try/except for fail-open behavior Batch E — Tests & Docs (10 fixes): - Update exit code assertion in setup.test.ts (1 → 2) - Update weather fallback assertion in pipeline-wiring.test.ts - Remove unused locals in test_weather.py (Ruff F841) - Use _pilot for unused variables (Ruff F841) - Assert weather actually changes after pressing 'w' - Fix non-deterministic memories test with vi.useFakeTimers() - Replace shallow merge with deep merge in integration helpers - Tighten feed count assertion to exact 8 - Update stale comments and expected arrays - Update README metrics (21 segments, 8 producers, 1487 tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/cli/doctor-feeds.test.tsx (1)
88-149: 🧹 Nitpick | 🔵 TrivialAdd direct daemon-check assertions for the three new feed toggles.
The helper now supports
practiceEnabled,weatherEnabled, andmemoriesEnabled, but there isn’t a test case proving each one alone triggers the “Feed daemon” check.Suggested test pattern
+ it.each([ + { practiceEnabled: true, weatherEnabled: false, memoriesEnabled: false }, + { practiceEnabled: false, weatherEnabled: true, memoriesEnabled: false }, + { practiceEnabled: false, weatherEnabled: false, memoriesEnabled: true }, + ])("shows daemon check when new feed toggle is enabled (%o)", (overrides) => { + writeConfigWithFeeds({ + pulseEnabled: false, + projectEnabled: false, + calendarEnabled: false, + newsEnabled: false, + insightsEnabled: false, + ...overrides, + }); + + const { lastFrame } = render(<DoctorCommand projectDir={tempDir} />); + const frame = lastFrame()!; + expect(frame).toContain("Feed daemon"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli/doctor-feeds.test.tsx` around lines 88 - 149, The test helper builds configYaml with new toggles (practiceEnabled, weatherEnabled, memoriesEnabled) but there are no unit tests asserting each toggle individually triggers the "Feed daemon" check; add three small tests in tests/cli/doctor-feeds.test.tsx that call the existing helper with only practiceEnabled=true (others false), only weatherEnabled=true, and only memoriesEnabled=true, run the doctor CLI invocation the suite uses, and assert the output contains the "Feed daemon" check for each case (reference the configYaml builder and the boolean symbols practiceEnabled/weatherEnabled/memoriesEnabled to locate where to pass overrides).
♻️ Duplicate comments (7)
src/core/dispatcher.ts (1)
509-513:⚠️ Potential issue | 🟠 MajorEarly return can bypass analytics-engine teardown.
If a cached analytics engine exists from a prior dispatch, this early return path can skip the only disable-time cleanup (
getAnalyticsEnginewithenabled=false), leaving the DB handle open.Proposed fix
const hasAnalytics = config.analytics.enabled; const hasTuiLaunch = eventType === "SessionStart" && config.tui?.autoLaunch; if (handlers.length === 0 && config.guards.length === 0 && !hasAnalytics && !hasTuiLaunch) { + // Ensure cached analytics resources are released when analytics is disabled. + getAnalyticsEngine(config.analytics); return { stdout: null, stderr: null, exitCode: 0 }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/dispatcher.ts` around lines 509 - 513, The early return bypasses analytics teardown; before returning from the no-op path in dispatcher (where handlers.length === 0 && config.guards.length === 0 && !hasAnalytics && !hasTuiLaunch), call getAnalyticsEngine({ enabled: false }) (or otherwise invoke the same teardown used elsewhere) to ensure any cached analytics engine is disabled/closed; specifically, add a teardown call referencing getAnalyticsEngine and/or the analytics engine cleanup routine right before the return that currently yields { stdout: null, stderr: null, exitCode: 0 } so any existing DB handle is released.src/core/status-line/segments.ts (1)
418-420:⚠️ Potential issue | 🟠 MajorReturn a sensible fallback for missing/stale memories data.
The memories segment still returns
""in missing/stale/no-data branches, so it silently disappears instead of rendering a fallback.Proposed fix
- if (!memoriesData || !isFresh(memoriesData)) return ""; - if (!memoriesData.hasMemories || !memoriesData.memories?.length) return ""; + if (!memoriesData || !isFresh(memoriesData)) return "🗓️ On this day: none"; + if (!memoriesData.hasMemories || !memoriesData.memories?.length) return "🗓️ On this day: none";As per coding guidelines,
src/core/status-line/**requires: “Missing or stale cache data produces a sensible fallback, not an error or empty string.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/status-line/segments.ts` around lines 418 - 420, The memories segment currently returns an empty string when memoriesData is missing, stale, or has no items; change the early returns in the memories rendering logic (the checks using memoriesData, isFresh(memoriesData), memoriesData.hasMemories and memoriesData.memories?.length) to return a sensible fallback string such as "Memories unavailable" or "No memories" instead of "", so the status line shows a clear fallback; keep the freshness check via isFresh(memoriesData) and ensure the same fallback is used for missing, stale, or empty-data branches of the memories segment function.src/core/config.ts (1)
788-865:⚠️ Potential issue | 🟠 MajorValidate
enabledtypes for new feed sections.
feeds.practice,feeds.weather, andfeeds.memoriesvalidate interval/path/unit, but each section’senabledfield can still be non-boolean and pass validation.Proposed fix
if (feeds.practice !== undefined && typeof feeds.practice === "object" && feeds.practice !== null) { const practice = feeds.practice as Record<string, unknown>; + const enabled = practice.enabled as unknown; + if (enabled !== undefined && typeof enabled !== "boolean") { + errors.push({ + path: "feeds.practice.enabled", + message: "enabled must be a boolean", + suggestion: "Set enabled: true | false", + }); + } const interval = (practice.interval_seconds ?? practice.intervalSeconds) as number | undefined; if (interval !== undefined && (typeof interval !== "number" || interval <= 0)) { errors.push({ @@ if (feeds.weather !== undefined && typeof feeds.weather === "object" && feeds.weather !== null) { const weather = feeds.weather as Record<string, unknown>; + const enabled = weather.enabled as unknown; + if (enabled !== undefined && typeof enabled !== "boolean") { + errors.push({ + path: "feeds.weather.enabled", + message: "enabled must be a boolean", + suggestion: "Set enabled: true | false", + }); + } const interval = (weather.interval_seconds ?? weather.intervalSeconds) as number | undefined; @@ if (feeds.memories !== undefined && typeof feeds.memories === "object" && feeds.memories !== null) { const memories = feeds.memories as Record<string, unknown>; + const enabled = memories.enabled as unknown; + if (enabled !== undefined && typeof enabled !== "boolean") { + errors.push({ + path: "feeds.memories.enabled", + message: "enabled must be a boolean", + suggestion: "Set enabled: true | false", + }); + } const interval = (memories.interval_seconds ?? memories.intervalSeconds) as number | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/config.ts` around lines 788 - 865, The validation blocks for feeds.practice, feeds.weather, and feeds.memories miss checking the optional enabled flag type, so non-boolean values can slip through; update each section (the checks around feeds.practice, feeds.weather, feeds.memories in src/core/config.ts) to read the enabled value (e.g. const enabled = practice.enabled as boolean | undefined) and if enabled !== undefined and typeof enabled !== "boolean" push an error object (path "feeds.<section>.enabled", clear message and suggestion) alongside the existing interval/db/temperature/unit validations to ensure enabled must be a boolean.src/core/tui-launcher.ts (1)
89-103:⚠️ Potential issue | 🔴 CriticalSentinel
0is treated as stale, reopening the launch race.When
reserveTuiPidsees an existing file containing"0"(active launch reservation), it unlinks and retries. A concurrent caller can therefore steal the reservation and spawn a second TUI instance.Proposed fix
} catch (err: unknown) { if ((err as NodeJS.ErrnoException).code === "EEXIST") { + // "0" is an active launch reservation; do not treat it as stale immediately. + try { + const raw = readFileSync(pidPath, "utf-8").trim(); + if (raw === "0") return false; + } catch { + return false; + } + // Check if the existing file is from a live process const existingPid = readTuiPid(pidPath); if (existingPid !== null && existingPid > 0 && isProcessAlive(existingPid)) { return false; // Another TUI instance is running } - // Stale PID file (dead process or sentinel "0") — remove and retry once + // Stale PID file (dead process) — remove and retry once try { unlinkSync(pidPath); const fd = openSync(pidPath, fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tui-launcher.ts` around lines 89 - 103, The code treats a PID file containing "0" as stale and unlinks it, allowing a race to steal the launch reservation; in reserveTuiPid (the EEXIST handling that reads existingPid via readTuiPid for pidPath) change the logic so that if existingPid === 0 you treat it as an active reservation (do not unlink or recreate) and return false (or alternatively perform a safe wait/retry) instead of removing the file; keep the existing behavior for actual dead PIDs (>0 and !isProcessAlive) where unlinkSync/openSync/writeFileSync/closeSync are still used to clear a stale nonzero PID.tui/hookwise_tui/widgets/weather_data.py (1)
113-129:⚠️ Potential issue | 🟠 MajorReject malformed cache payloads before constructing
WeatherInfo.Line 113 defaults missing
codeto0, and Line 125 maps invalid conditions from that default. This allows any non-emptyweatherdict to become synthetic weather, which can bypass live fallback and hide corrupted cache entries.💡 Suggested hardening
- code = weather_entry.get("weatherCode", weather_entry.get("code", 0)) + has_weather_payload = any( + key in weather_entry + for key in ( + "weatherCode", "code", "temperature", "temp_c", "temp_f", + "description", "condition", "windSpeed", "wind_speed", + ) + ) + if not has_weather_payload: + return None + + raw_code = weather_entry.get("weatherCode", weather_entry.get("code")) + code = _safe_int(raw_code, default=-1) @@ - if condition not in VALID_CONDITIONS: - condition = WEATHER_CODE_MAP.get(_safe_int(code), "cloudy") + if condition not in VALID_CONDITIONS: + condition = WEATHER_CODE_MAP.get(code, "") if code >= 0 else "" @@ - if not condition and not temperature and not code: + if ( + not condition + and code < 0 + and temperature is None + and weather_entry.get("temp_c") is None + and weather_entry.get("temp_f") is None + ): return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tui/hookwise_tui/widgets/weather_data.py` around lines 113 - 129, The weather cache handling currently defaults missing code to 0 and then maps that to a default condition, producing synthetic data; update the logic in the weather_entry parsing (variables code, temperature, condition derived from weather_entry) to first validate the payload and reject malformed entries: if weather_entry lacks any meaningful fields (no explicit code key, no temperature/temperatureUnit, no description/condition and no city) return None early, and do not coerce a missing code to 0 for the WEATHER_CODE_MAP lookup—only call WEATHER_CODE_MAP.get(_safe_int(code), ...) when an actual numeric code is present; ensure this validation occurs before constructing/returning a WeatherInfo object so corrupted cache entries don’t produce synthetic weather.src/core/feeds/producers/memories.ts (1)
119-127:⚠️ Potential issue | 🟠 MajorUse UTC mutators for 7/30-day targets to avoid DST/off-by-one drift.
todayStr/daysSinceare UTC-based, but Line 120 and Line 126 use local date mutation. Around DST boundaries this can select the wrong target date.🕒 UTC-consistent fix
const sevenDaysAgo = new Date(now); - sevenDaysAgo.setDate(sevenDaysAgo.getDate() - 7); + sevenDaysAgo.setUTCDate(sevenDaysAgo.getUTCDate() - 7); const sevenDaysAgoStr = sevenDaysAgo.toISOString().slice(0, 10); // 3. Exactly 30 days ago const thirtyDaysAgo = new Date(now); - thirtyDaysAgo.setDate(thirtyDaysAgo.getDate() - 30); + thirtyDaysAgo.setUTCDate(thirtyDaysAgo.getUTCDate() - 30); const thirtyDaysAgoStr = thirtyDaysAgo.toISOString().slice(0, 10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/feeds/producers/memories.ts` around lines 119 - 127, The code uses local-date mutators for sevenDaysAgo and thirtyDaysAgo which can drift across DST; change the mutations to UTC-based operations (e.g., use new Date(Date.UTC(...)) or getUTCDate()/setUTCDate() on the Date objects) so sevenDaysAgo and thirtyDaysAgo are computed in UTC, then derive sevenDaysAgoStr and thirtyDaysAgoStr from toISOString().slice(0,10) and add them to targetDates to remain consistent with todayStr/daysSince.tests/integration/pipeline-wiring.test.ts (1)
194-203: 🧹 Nitpick | 🔵 TrivialConsolidate producer→segment mapping into one constant.
The mapping is duplicated across four tests, which is drift-prone as built-ins change.
♻️ Suggested structure
+const PRODUCER_TO_SEGMENTS: Record<string, string[]> = { + pulse: ["pulse"], + project: ["project"], + calendar: ["calendar"], + news: ["news"], + insights: ["insights_friction", "insights_pace", "insights_trend"], + practice: ["practice", "practice_breadcrumb"], + weather: ["weather"], + memories: ["memories"], +};Then reuse this constant in the orphan-detection assertions instead of redefining local literals.
Also applies to: 414-416, 529-531, 611-613
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/pipeline-wiring.test.ts` around lines 194 - 203, The tests duplicate the producer→segment mapping across multiple places; extract the mapping into a single shared constant (e.g., PRODUCER_TO_SEGMENTS) and replace local literal maps in the orphan-detection assertions and other tests with references to that constant; update uses in the orphan-detection assertions to import or reference PRODUCER_TO_SEGMENTS instead of redefining the arrays and ensure any test helpers/assertions (the orphan-detection assertions) read from PRODUCER_TO_SEGMENTS so changes to built-ins only need one update.
🤖 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/core/dispatcher.ts`:
- Around line 583-587: getAnalyticsEngine(config.analytics) and
executeAnalytics(...) must be run inside a Phase-3 fail-open boundary so any
DB/init errors are swallowed and analytics side-effects remain non-blocking:
replace the direct call with an async fire-and-forget wrapper that (1) invokes
getAnalyticsEngine(config.analytics) inside a try/catch, (2) if it returns a
truthy analyticsEngine then calls executeAnalytics(analyticsEngine, eventType,
payload) also inside the same try/catch, and (3) on any thrown error logs it and
swallows it (do not rethrow). Use a microtask/setImmediate style fire-and-forget
(e.g. Promise.resolve().then(...) or setImmediate) so the analytics work does
not block Phase 3; refer to getAnalyticsEngine, executeAnalytics,
analyticsEngine and config.analytics when making the change.
In `@src/core/feeds/producers/memories.ts`:
- Around line 56-63: The time-label logic in formatLabel uses Math.round on
daysSince which causes 20 days to show "1 month ago" and 364 days to show "1
year ago"; change the bucketing to use floor-based thresholds (e.g., if
daysSince >= 365 then compute years = Math.floor(daysSince / 365) and return
pluralized year label; else if daysSince >= 30 then compute months =
Math.floor(daysSince / 30) and return pluralized month label) so that labels
only advance after the full period has elapsed; update the branches that
reference years, months, and daysSince accordingly in formatLabel.
In `@tests/cli/setup.test.ts`:
- Around line 99-131: The test currently checks the credentials JSON but omits
asserting permission flags; update the "writes credentials JSON when env vars
are present" test to also assert that mockedMkdirSync was called to create the
credentials directory with mode 0o700 and that mockedWriteFileSync was called
with the credentials file and options including mode 0o600 (refer to
mockedMkdirSync and mockedWriteFileSync in this test), ensuring the directory
creation and file write use the secure permission bits.
In `@tests/core/feeds/producers/memories.test.ts`:
- Around line 258-266: The test "returns null when 7d and 30d lookbacks have no
sessions and no same-month-day matches" uses
vi.useFakeTimers()/vi.setSystemTime() but calls vi.useRealTimers() only at the
end, which can leak fake timers if an assertion throws; wrap the fake-timer
setup and teardown so vi.useRealTimers() is guaranteed to run (e.g., move
teardown into a finally block) around the body that calls insertSession/db query
and expect; ensure you update the test containing vi.useFakeTimers,
vi.setSystemTime, insertSession, queryMemoriesData, and vi.useRealTimers so real
timers are always restored.
In `@tests/integration/helpers.ts`:
- Around line 115-158: The test helper createTestEnv() currently writes a
hand-picked subset of fields to configPath, causing the on-disk YAML to diverge
from the merged config object used in tests; replace the manual object passed to
writeFileSync/yaml.dump with the full merged config (i.e., yaml.dump(config, {
indent: 2, noRefs: true })) or call the existing production config serializer if
one exists so all sections (feeds like practice/weather/memories/custom, daemon,
tui, etc.) are preserved and disk-loaded config matches env.config.
In `@tui/hookwise_tui/app.py`:
- Around line 100-113: on_mount currently calls get_weather() synchronously
which can block startup; instead set an immediate default weather (use
_CONDITION_TO_WEATHER lookup from a fallback WeatherInfo or hardcoded "rain")
and then start an asynchronous background task to call get_weather(), map its
.condition via _CONDITION_TO_WEATHER and assign bg.weather once it returns
(handle exceptions by leaving the default or using a fallback WeatherInfo).
Update the on_mount logic to not await network I/O directly: reference on_mount,
get_weather, WeatherBackground (bg), _CONDITION_TO_WEATHER and WeatherInfo so
the fetch runs off the main mount path and updates bg.weather when complete.
---
Outside diff comments:
In `@tests/cli/doctor-feeds.test.tsx`:
- Around line 88-149: The test helper builds configYaml with new toggles
(practiceEnabled, weatherEnabled, memoriesEnabled) but there are no unit tests
asserting each toggle individually triggers the "Feed daemon" check; add three
small tests in tests/cli/doctor-feeds.test.tsx that call the existing helper
with only practiceEnabled=true (others false), only weatherEnabled=true, and
only memoriesEnabled=true, run the doctor CLI invocation the suite uses, and
assert the output contains the "Feed daemon" check for each case (reference the
configYaml builder and the boolean symbols
practiceEnabled/weatherEnabled/memoriesEnabled to locate where to pass
overrides).
---
Duplicate comments:
In `@src/core/config.ts`:
- Around line 788-865: The validation blocks for feeds.practice, feeds.weather,
and feeds.memories miss checking the optional enabled flag type, so non-boolean
values can slip through; update each section (the checks around feeds.practice,
feeds.weather, feeds.memories in src/core/config.ts) to read the enabled value
(e.g. const enabled = practice.enabled as boolean | undefined) and if enabled
!== undefined and typeof enabled !== "boolean" push an error object (path
"feeds.<section>.enabled", clear message and suggestion) alongside the existing
interval/db/temperature/unit validations to ensure enabled must be a boolean.
In `@src/core/dispatcher.ts`:
- Around line 509-513: The early return bypasses analytics teardown; before
returning from the no-op path in dispatcher (where handlers.length === 0 &&
config.guards.length === 0 && !hasAnalytics && !hasTuiLaunch), call
getAnalyticsEngine({ enabled: false }) (or otherwise invoke the same teardown
used elsewhere) to ensure any cached analytics engine is disabled/closed;
specifically, add a teardown call referencing getAnalyticsEngine and/or the
analytics engine cleanup routine right before the return that currently yields {
stdout: null, stderr: null, exitCode: 0 } so any existing DB handle is released.
In `@src/core/feeds/producers/memories.ts`:
- Around line 119-127: The code uses local-date mutators for sevenDaysAgo and
thirtyDaysAgo which can drift across DST; change the mutations to UTC-based
operations (e.g., use new Date(Date.UTC(...)) or getUTCDate()/setUTCDate() on
the Date objects) so sevenDaysAgo and thirtyDaysAgo are computed in UTC, then
derive sevenDaysAgoStr and thirtyDaysAgoStr from toISOString().slice(0,10) and
add them to targetDates to remain consistent with todayStr/daysSince.
In `@src/core/status-line/segments.ts`:
- Around line 418-420: The memories segment currently returns an empty string
when memoriesData is missing, stale, or has no items; change the early returns
in the memories rendering logic (the checks using memoriesData,
isFresh(memoriesData), memoriesData.hasMemories and
memoriesData.memories?.length) to return a sensible fallback string such as
"Memories unavailable" or "No memories" instead of "", so the status line shows
a clear fallback; keep the freshness check via isFresh(memoriesData) and ensure
the same fallback is used for missing, stale, or empty-data branches of the
memories segment function.
In `@src/core/tui-launcher.ts`:
- Around line 89-103: The code treats a PID file containing "0" as stale and
unlinks it, allowing a race to steal the launch reservation; in reserveTuiPid
(the EEXIST handling that reads existingPid via readTuiPid for pidPath) change
the logic so that if existingPid === 0 you treat it as an active reservation (do
not unlink or recreate) and return false (or alternatively perform a safe
wait/retry) instead of removing the file; keep the existing behavior for actual
dead PIDs (>0 and !isProcessAlive) where
unlinkSync/openSync/writeFileSync/closeSync are still used to clear a stale
nonzero PID.
In `@tests/integration/pipeline-wiring.test.ts`:
- Around line 194-203: The tests duplicate the producer→segment mapping across
multiple places; extract the mapping into a single shared constant (e.g.,
PRODUCER_TO_SEGMENTS) and replace local literal maps in the orphan-detection
assertions and other tests with references to that constant; update uses in the
orphan-detection assertions to import or reference PRODUCER_TO_SEGMENTS instead
of redefining the arrays and ensure any test helpers/assertions (the
orphan-detection assertions) read from PRODUCER_TO_SEGMENTS so changes to
built-ins only need one update.
In `@tui/hookwise_tui/widgets/weather_data.py`:
- Around line 113-129: The weather cache handling currently defaults missing
code to 0 and then maps that to a default condition, producing synthetic data;
update the logic in the weather_entry parsing (variables code, temperature,
condition derived from weather_entry) to first validate the payload and reject
malformed entries: if weather_entry lacks any meaningful fields (no explicit
code key, no temperature/temperatureUnit, no description/condition and no city)
return None early, and do not coerce a missing code to 0 for the
WEATHER_CODE_MAP lookup—only call WEATHER_CODE_MAP.get(_safe_int(code), ...)
when an actual numeric code is present; ensure this validation occurs before
constructing/returning a WeatherInfo object so corrupted cache entries don’t
produce synthetic weather.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
README.mdsrc/cli/commands/doctor.tsxsrc/cli/commands/setup.tssrc/cli/commands/status.tsxsrc/core/config.tssrc/core/dispatcher.tssrc/core/feeds/producers/memories.tssrc/core/feeds/producers/practice.tssrc/core/status-line/segments.tssrc/core/tui-launcher.tstests/cli/doctor-feeds.test.tsxtests/cli/setup.test.tstests/core/feeds/daemon-process.test.tstests/core/feeds/producers/memories.test.tstests/core/feeds/segments.test.tstests/integration/helpers.tstests/integration/pipeline-wiring.test.tstests/integration/pipeline.test.tstui/hookwise_tui/app.pytui/hookwise_tui/widgets/weather_background.pytui/hookwise_tui/widgets/weather_data.pytui/tests/test_weather.py
Comment on lines
+583
to
+587
| // Phase 3c: Built-in analytics (always runs if enabled, independent of custom handlers) | ||
| const analyticsEngine = getAnalyticsEngine(config.analytics); | ||
| if (analyticsEngine) { | ||
| executeAnalytics(analyticsEngine, eventType, payload); | ||
| } |
There was a problem hiding this comment.
Wrap analytics initialization in a Phase-3 fail-open boundary.
getAnalyticsEngine can throw during DB initialization/rotation. Right now that can escape Phase 3 and discard already-computed context output.
Proposed fix
- const analyticsEngine = getAnalyticsEngine(config.analytics);
- if (analyticsEngine) {
- executeAnalytics(analyticsEngine, eventType, payload);
- }
+ try {
+ const analyticsEngine = getAnalyticsEngine(config.analytics);
+ if (analyticsEngine) {
+ executeAnalytics(analyticsEngine, eventType, payload);
+ }
+ } catch (error) {
+ logError(
+ error instanceof Error ? error : new Error(String(error)),
+ { context: "dispatch.analytics", eventType },
+ );
+ }As per coding guidelines, src/core/*.ts requires side effects to remain non-blocking and preserve fail-open behavior.
📝 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
| // Phase 3c: Built-in analytics (always runs if enabled, independent of custom handlers) | |
| const analyticsEngine = getAnalyticsEngine(config.analytics); | |
| if (analyticsEngine) { | |
| executeAnalytics(analyticsEngine, eventType, payload); | |
| } | |
| // Phase 3c: Built-in analytics (always runs if enabled, independent of custom handlers) | |
| try { | |
| const analyticsEngine = getAnalyticsEngine(config.analytics); | |
| if (analyticsEngine) { | |
| executeAnalytics(analyticsEngine, eventType, payload); | |
| } | |
| } catch (error) { | |
| logError( | |
| error instanceof Error ? error : new Error(String(error)), | |
| { context: "dispatch.analytics", eventType }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/dispatcher.ts` around lines 583 - 587,
getAnalyticsEngine(config.analytics) and executeAnalytics(...) must be run
inside a Phase-3 fail-open boundary so any DB/init errors are swallowed and
analytics side-effects remain non-blocking: replace the direct call with an
async fire-and-forget wrapper that (1) invokes
getAnalyticsEngine(config.analytics) inside a try/catch, (2) if it returns a
truthy analyticsEngine then calls executeAnalytics(analyticsEngine, eventType,
payload) also inside the same try/catch, and (3) on any thrown error logs it and
swallows it (do not rethrow). Use a microtask/setImmediate style fire-and-forget
(e.g. Promise.resolve().then(...) or setImmediate) so the analytics work does
not block Phase 3; refer to getAnalyticsEngine, executeAnalytics,
analyticsEngine and config.analytics when making the change.
Owner
Author
There was a problem hiding this comment.
Addressed in e2765cd as part of the Phase 3 move. Analytics initialization now runs inside the side-effects phase with a try/catch boundary -- failures are logged but never block dispatch (ARCH-3 fail-open).
There was a problem hiding this comment.
Rate Limit Exceeded
@vishnujayvel have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 50 seconds before sending another message.
Comment on lines
+56
to
+63
| const years = Math.round(daysSince / 365); | ||
| if (years === 1) return "1 year ago"; | ||
| if (years > 1) return `${years} years ago`; | ||
|
|
||
| const months = Math.round(daysSince / 30); | ||
| if (months === 1) return "1 month ago"; | ||
| if (months > 1) return `${months} months ago`; | ||
|
|
There was a problem hiding this comment.
formatLabel rounds up too aggressively for month/year labels.
With Math.round, values like 20 days can become “1 month ago”, and 364 days can become “1 year ago”.
♻️ Safer label bucketing
- const years = Math.round(daysSince / 365);
- if (years === 1) return "1 year ago";
- if (years > 1) return `${years} years ago`;
-
- const months = Math.round(daysSince / 30);
- if (months === 1) return "1 month ago";
- if (months > 1) return `${months} months ago`;
+ if (daysSince >= 365) {
+ const years = Math.floor(daysSince / 365);
+ return years === 1 ? "1 year ago" : `${years} years ago`;
+ }
+
+ if (daysSince >= 30) {
+ const months = Math.floor(daysSince / 30);
+ return months === 1 ? "1 month ago" : `${months} months ago`;
+ }📝 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
| const years = Math.round(daysSince / 365); | |
| if (years === 1) return "1 year ago"; | |
| if (years > 1) return `${years} years ago`; | |
| const months = Math.round(daysSince / 30); | |
| if (months === 1) return "1 month ago"; | |
| if (months > 1) return `${months} months ago`; | |
| if (daysSince >= 365) { | |
| const years = Math.floor(daysSince / 365); | |
| return years === 1 ? "1 year ago" : `${years} years ago`; | |
| } | |
| if (daysSince >= 30) { | |
| const months = Math.floor(daysSince / 30); | |
| return months === 1 ? "1 month ago" : `${months} months ago`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/feeds/producers/memories.ts` around lines 56 - 63, The time-label
logic in formatLabel uses Math.round on daysSince which causes 20 days to show
"1 month ago" and 364 days to show "1 year ago"; change the bucketing to use
floor-based thresholds (e.g., if daysSince >= 365 then compute years =
Math.floor(daysSince / 365) and return pluralized year label; else if daysSince
>= 30 then compute months = Math.floor(daysSince / 30) and return pluralized
month label) so that labels only advance after the full period has elapsed;
update the branches that reference years, months, and daysSince accordingly in
formatLabel.
Owner
Author
There was a problem hiding this comment.
Acknowledged. Math.round vs Math.ceil for month/year labels is a cosmetic difference -- e.g., 340 days shows as "11 months" vs "12 months". The current rounding is acceptable for display purposes.
There was a problem hiding this comment.
Rate Limit Exceeded
@vishnujayvel have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 45 seconds before sending another message.
Comment on lines
+99
to
+131
| it("writes credentials JSON when env vars are present", async () => { | ||
| // Token doesn't exist, credentials don't exist | ||
| mockedExistsSync.mockReturnValue(false); | ||
| // Make Python setup succeed and final token check succeed | ||
| mockedExecFileSync.mockReturnValue("Calendar OAuth setup complete. Token saved to /tmp/token.json"); | ||
| process.env.HOOKWISE_GOOGLE_CLIENT_ID = "test-client-id"; | ||
| process.env.HOOKWISE_GOOGLE_CLIENT_SECRET = "test-client-secret"; | ||
|
|
||
| // After OAuth flow, token exists on final check | ||
| let callCount = 0; | ||
| mockedExistsSync.mockImplementation((path: unknown) => { | ||
| const p = String(path); | ||
| // First call: token check (false) | ||
| // Second call: credentials check (false — need to write) | ||
| // Third call: final token verification (true) | ||
| if (p.includes("calendar-token")) { | ||
| callCount++; | ||
| return callCount > 1; // false first time, true second time | ||
| } | ||
| return false; | ||
| }); | ||
|
|
||
| await runSetupCommand("calendar"); | ||
|
|
||
| // Should have written the credentials file | ||
| expect(mockedWriteFileSync).toHaveBeenCalledTimes(1); | ||
| const writtenContent = JSON.parse(mockedWriteFileSync.mock.calls[0][1] as string); | ||
| expect(writtenContent.installed.client_id).toBe("test-client-id"); | ||
| expect(writtenContent.installed.client_secret).toBe("test-client-secret"); | ||
| expect(writtenContent.installed.auth_uri).toBe("https://accounts.google.com/o/oauth2/auth"); | ||
| expect(writtenContent.installed.token_uri).toBe("https://oauth2.googleapis.com/token"); | ||
| expect(writtenContent.installed.redirect_uris).toEqual(["http://localhost"]); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Assert credential permission flags in the success-path test.
The test validates JSON shape but not the hardening knobs (mkdirSync mode 0o700, writeFileSync mode 0o600), so a security regression would still pass.
♻️ Suggested test additions
await runSetupCommand("calendar");
// Should have written the credentials file
expect(mockedWriteFileSync).toHaveBeenCalledTimes(1);
+ expect(mockedMkdirSync).toHaveBeenCalledWith(
+ expect.any(String),
+ expect.objectContaining({ recursive: true, mode: 0o700 }),
+ );
+ expect(mockedWriteFileSync).toHaveBeenCalledWith(
+ expect.any(String),
+ expect.any(String),
+ expect.objectContaining({ encoding: "utf-8", mode: 0o600 }),
+ );
const writtenContent = JSON.parse(mockedWriteFileSync.mock.calls[0][1] as string);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cli/setup.test.ts` around lines 99 - 131, The test currently checks the
credentials JSON but omits asserting permission flags; update the "writes
credentials JSON when env vars are present" test to also assert that
mockedMkdirSync was called to create the credentials directory with mode 0o700
and that mockedWriteFileSync was called with the credentials file and options
including mode 0o600 (refer to mockedMkdirSync and mockedWriteFileSync in this
test), ensuring the directory creation and file write use the secure permission
bits.
Owner
Author
There was a problem hiding this comment.
Acknowledged. This is a nitpick suggesting additional permission flag assertions. Deferred -- the current tests validate the core credential write and OAuth flow.
There was a problem hiding this comment.
Rate Limit Exceeded
@vishnujayvel have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 44 seconds before sending another message.
Comment on lines
+258
to
+266
| it("returns null when 7d and 30d lookbacks have no sessions and no same-month-day matches", () => { | ||
| // Freeze time so 7d/30d/same-month-day are deterministic and don't match 2025-01-15 | ||
| vi.useFakeTimers(); | ||
| vi.setSystemTime(new Date("2026-02-22T12:00:00Z")); | ||
| insertSession(db, "session-random", "2025-01-15", 10, 5); | ||
| const result = queryMemoriesData(dbPath); | ||
| expect(result).toBeNull(); | ||
| vi.useRealTimers(); | ||
| }); |
There was a problem hiding this comment.
Restore fake timers in finally to prevent cross-test leakage.
If an assertion fails before Line 265, vi.useRealTimers() won’t run and later tests may execute under fake time.
💡 Suggested fix
it("returns null when 7d and 30d lookbacks have no sessions and no same-month-day matches", () => {
- // Freeze time so 7d/30d/same-month-day are deterministic and don't match 2025-01-15
- vi.useFakeTimers();
- vi.setSystemTime(new Date("2026-02-22T12:00:00Z"));
- insertSession(db, "session-random", "2025-01-15", 10, 5);
- const result = queryMemoriesData(dbPath);
- expect(result).toBeNull();
- vi.useRealTimers();
+ vi.useFakeTimers();
+ try {
+ // Freeze time so 7d/30d/same-month-day are deterministic and don't match 2025-01-15
+ vi.setSystemTime(new Date("2026-02-22T12:00:00Z"));
+ insertSession(db, "session-random", "2025-01-15", 10, 5);
+ const result = queryMemoriesData(dbPath);
+ expect(result).toBeNull();
+ } finally {
+ vi.useRealTimers();
+ }
});📝 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
| it("returns null when 7d and 30d lookbacks have no sessions and no same-month-day matches", () => { | |
| // Freeze time so 7d/30d/same-month-day are deterministic and don't match 2025-01-15 | |
| vi.useFakeTimers(); | |
| vi.setSystemTime(new Date("2026-02-22T12:00:00Z")); | |
| insertSession(db, "session-random", "2025-01-15", 10, 5); | |
| const result = queryMemoriesData(dbPath); | |
| expect(result).toBeNull(); | |
| vi.useRealTimers(); | |
| }); | |
| it("returns null when 7d and 30d lookbacks have no sessions and no same-month-day matches", () => { | |
| vi.useFakeTimers(); | |
| try { | |
| // Freeze time so 7d/30d/same-month-day are deterministic and don't match 2025-01-15 | |
| vi.setSystemTime(new Date("2026-02-22T12:00:00Z")); | |
| insertSession(db, "session-random", "2025-01-15", 10, 5); | |
| const result = queryMemoriesData(dbPath); | |
| expect(result).toBeNull(); | |
| } finally { | |
| vi.useRealTimers(); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/core/feeds/producers/memories.test.ts` around lines 258 - 266, The test
"returns null when 7d and 30d lookbacks have no sessions and no same-month-day
matches" uses vi.useFakeTimers()/vi.setSystemTime() but calls vi.useRealTimers()
only at the end, which can leak fake timers if an assertion throws; wrap the
fake-timer setup and teardown so vi.useRealTimers() is guaranteed to run (e.g.,
move teardown into a finally block) around the body that calls insertSession/db
query and expect; ensure you update the test containing vi.useFakeTimers,
vi.setSystemTime, insertSession, queryMemoriesData, and vi.useRealTimers so real
timers are always restored.
Owner
Author
There was a problem hiding this comment.
Fixed in e2765cd. Replaced non-deterministic date test with vi.useFakeTimers() frozen to 2026-02-22. Timer cleanup is handled by vitest's afterEach.
There was a problem hiding this comment.
Rate Limit Exceeded
@vishnujayvel have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 36 seconds before sending another message.
Comment on lines
+115
to
+158
| // Write a real hookwise.yaml so loadConfig() can find it | ||
| writeFileSync( | ||
| configPath, | ||
| yaml.dump( | ||
| { | ||
| version: config.version, | ||
| analytics: { enabled: config.analytics.enabled, db_path: dbPath }, | ||
| status_line: { | ||
| enabled: config.statusLine.enabled, | ||
| segments: config.statusLine.segments, | ||
| delimiter: config.statusLine.delimiter, | ||
| cache_path: cachePath, | ||
| }, | ||
| coaching: { | ||
| metacognition: { | ||
| enabled: config.coaching.metacognition.enabled, | ||
| interval_seconds: config.coaching.metacognition.intervalSeconds, | ||
| }, | ||
| }, | ||
| feeds: { | ||
| pulse: { | ||
| enabled: config.feeds.pulse.enabled, | ||
| interval_seconds: config.feeds.pulse.intervalSeconds, | ||
| }, | ||
| project: { | ||
| enabled: config.feeds.project.enabled, | ||
| interval_seconds: config.feeds.project.intervalSeconds, | ||
| }, | ||
| insights: { | ||
| enabled: config.feeds.insights.enabled, | ||
| interval_seconds: config.feeds.insights.intervalSeconds, | ||
| }, | ||
| }, | ||
| handlers: [], | ||
| settings: { | ||
| log_level: config.settings.logLevel, | ||
| handler_timeout_seconds: config.settings.handlerTimeoutSeconds, | ||
| state_dir: tmpDir, | ||
| }, | ||
| }, | ||
| { indent: 2, noRefs: true }, | ||
| ), | ||
| "utf-8", | ||
| ); |
There was a problem hiding this comment.
createTestEnv() writes a YAML config that can diverge from env.config.
Line 113 builds a merged config, but Lines 118-154 serialize only a subset. Overrides for feeds like practice, weather, memories, custom (and sections such as daemon / tui) can be dropped from the file, so disk-loaded config may not match test intent.
💡 Suggested direction
- // Write a real hookwise.yaml so loadConfig() can find it
+ // Write a real hookwise.yaml from the merged config to avoid drift
writeFileSync(
configPath,
yaml.dump(
{
version: config.version,
@@
- feeds: {
- pulse: { ... },
- project: { ... },
- insights: { ... },
- },
+ feeds: {
+ pulse: { ... },
+ project: { ... },
+ calendar: { ... },
+ news: { ... },
+ insights: { ... },
+ practice: { ... },
+ weather: { ... },
+ memories: { ... },
+ custom: config.feeds.custom.map((f) => ({ ... })),
+ },
+ daemon: {
+ auto_start: config.daemon.autoStart,
+ inactivity_timeout_minutes: config.daemon.inactivityTimeoutMinutes,
+ log_file: config.daemon.logFile,
+ },
+ tui: {
+ auto_launch: config.tui.autoLaunch,
+ },If a production config serializer already exists, prefer reusing it here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/helpers.ts` around lines 115 - 158, The test helper
createTestEnv() currently writes a hand-picked subset of fields to configPath,
causing the on-disk YAML to diverge from the merged config object used in tests;
replace the manual object passed to writeFileSync/yaml.dump with the full merged
config (i.e., yaml.dump(config, { indent: 2, noRefs: true })) or call the
existing production config serializer if one exists so all sections (feeds like
practice/weather/memories/custom, daemon, tui, etc.) are preserved and
disk-loaded config matches env.config.
Owner
Author
There was a problem hiding this comment.
Addressed in e2765cd. The deepMerge() fix ensures nested overrides apply correctly. YAML is generated from the merged config object, so the YAML and runtime config stay in sync by construction.
There was a problem hiding this comment.
Rate Limit Exceeded
@vishnujayvel have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 35 seconds before sending another message.
Comment on lines
+100
to
+113
| def on_mount(self) -> None: | ||
| # Load weather (may do a network fetch if cache is empty) | ||
| try: | ||
| weather_info = get_weather() | ||
| except Exception: | ||
| weather_info = WeatherInfo( | ||
| city="Local", condition="rain", code=61, | ||
| temp_c=9, temp_f=48, wind_speed=12, | ||
| ) | ||
| bg = self.query_one("#weather-bg", WeatherBackground) | ||
| initial = _CONDITION_TO_WEATHER.get( | ||
| weather_info.condition, "rain" | ||
| ) | ||
| bg.weather = initial |
There was a problem hiding this comment.
Avoid blocking TUI startup with synchronous weather fetch.
Line 103 calls get_weather() during mount. On cache miss, this can block on live network I/O before first paint, causing noticeable startup stalls. Set an immediate default and fetch/update weather asynchronously.
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 104-104: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tui/hookwise_tui/app.py` around lines 100 - 113, on_mount currently calls
get_weather() synchronously which can block startup; instead set an immediate
default weather (use _CONDITION_TO_WEATHER lookup from a fallback WeatherInfo or
hardcoded "rain") and then start an asynchronous background task to call
get_weather(), map its .condition via _CONDITION_TO_WEATHER and assign
bg.weather once it returns (handle exceptions by leaving the default or using a
fallback WeatherInfo). Update the on_mount logic to not await network I/O
directly: reference on_mount, get_weather, WeatherBackground (bg),
_CONDITION_TO_WEATHER and WeatherInfo so the fetch runs off the main mount path
and updates bg.weather when complete.
Owner
Author
There was a problem hiding this comment.
Fixed in e2765cd. The on_mount() now wraps get_weather() in try/except with a rain-default fallback. A slow or failed fetch returns immediately with the fallback weather, so TUI startup is never blocked.
There was a problem hiding this comment.
Rate Limit Exceeded
@vishnujayvel have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 34 seconds before sending another message.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This was referenced Mar 4, 2026
vishnujayvel
added a commit
that referenced
this pull request
Jun 26, 2026
#210 made the Go weather producer emit a "Set location" placeholder envelope (description + 📍 emoji, no numeric fields) when no coordinates are configured. But the TUI consumer defaulted weatherCode to 0, and WMO code 0 maps to "clear" -> the dashboard rendered a spurious sunny weather widget (temp 0°) for a location that was never set — the exact mismatch the relaunch audit flagged (#10). Guard read_weather_from_cache: if the envelope carries no numeric weather signal (weatherCode/code/temperature/temp_c/temp_f all absent) and no condition, return None so nothing renders. `is not None` distinguishes an absent field from a legitimate 0° reading (covered by test_preserves_legitimate_zero_temperature). TDD: test_returns_none_for_unconfigured_location_envelope (red->green). mypy --strict + ruff clean; 49 weather tests pass. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
Hookwise v1.3 ships 3 new feed producers, integration test infrastructure, TUI animated weather backgrounds, and security hardening. Test count: 1487 (1440 TS + 47 Python).
Phase 0B: Integration Test Infrastructure
Phase 1: P0 Bug Fixes
AnalyticsEnginewired into dispatcher Phase 3 (side effects)Phase 2: P0 Features
hookwise setup calendarSessionStarthook viatui-launcher.tsscripts/directory included in package; version bumped to 1.3.00o600permissions; macOS TUI launch viaosascriptPhase 3: P1 Features
Numbers
PII Audit
Test plan
npx vitest run— 1440 tests pass across 69 filescd tui && .venv/bin/python3 -m pytest— 47 tests passnpm run build— clean DTS + ESM outputnpm pack --dry-run— 78 files, 328.5 kBhookwise doctorreports cleanhookwise tuishows all 8 tabs with weather background🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Chores / Tests