perf: make day-scoped capture and status-event reads indexed#3569
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3569 +/- ##
=======================================
Coverage 99.22% 99.22%
=======================================
Files 1780 1780
Lines 130297 130324 +27
=======================================
+ Hits 129283 129311 +28
+ Misses 1014 1013 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Opening a day loaded every capture and every day-status event the owning agent had ever recorded and filtered them in Dart, so the cost of the main day surface grew with the user's entire history. It bit hardest on planner-owned days: a day agent is self-limiting, but the coordinator accumulates forever and owns every pre-cutover day. The cause was the projected subtype column — capture stored its own id (redundant with the primary key) and dayStatusEvent its status name, so neither could serve a day-scoped query, while dayPlan/daySummary/ dayDirective already stored the day and were indexed. Both now store the day workspace and read through idx_agent_entities_agent_type_sub. - localDay/dayAgentIdForDate move to lib/classes/day_plan.dart beside dayPlanId so the agent persistence layer can derive a capture's day without depending on a feature package. - A capture with no explicit dayId derives one from capturedAt in local time, the same rule captureDayId applies on read, so legacy rows synced from older peers land on the right day. - Schema v17 backfills through the same Dart projection rather than a SQL json_extract update: a backfilled value disagreeing with the writer would silently drop rows out of their day.
Codecov flagged 9 uncovered lines: the schema v17 backfill and the repository delegator. The backfill is exposed via @VisibleForTesting so the migration logic can be driven directly with rows written at their pre-v17 subtype. Covers the capture-id and status-name rewrites, the local-time derivation for a legacy capture with no dayId, that an undecodable row is skipped rather than costing every other row its index entry, and that types which already stored the day are left alone.
529f99f to
ad6fbb9
Compare
The test pins its draft to the real calendar day, then pumps a minute so the widget's per-minute timer fires. That callback re-reads the *real* clock, not the test binding's, so a run that straddles midnight moves "now" out of the draft's day and correctly retires the badge — CI hit this at 23:59:29 UTC. Assert the badge only while the wall clock is still on the day the draft was built for. The survives-a-tick property the test is named for is carried by the anchor-block and still-mounted checks regardless.
Third of the "day planner must not degrade as history accumulates" series,
after #3564 (outbox → table) and #3566 (priority claiming).
The problem
Opening a day loaded every capture and every day-status event the
owning agent had ever recorded, then filtered them in Dart:
day_agent_provider.dart—getEntitiesByAgentId(ownerId, type: capture)for both owners, no limit, day-filtered afterwards. Its own comment conceded
"an owner can hold many days' captures (the coordinator always does)".
day_agent_persona_provider.dart— everydayStatusEventfor the owner,filtered to the day in Dart. Status events drive the activity indicator and
are the fastest-growing entity type in the feature.
Both are Riverpod providers on the main day surface, so the cost of opening one
day grew with the user's entire history.
Severity was not uniform, which is worth stating: a
day_agent:<dayId>isself-limiting by construction — it holds one day's rows and then goes cold
forever. The coordinator is the accumulator, and it owns every pre-cutover day
(
day_agent_service.dartleaves those under the monolith permanently).Cause
The projected
subtypecolumn.capturestored the capture's own id —redundant with the primary key — and
dayStatusEventstored its status name,so neither could serve a day-scoped lookup. Meanwhile
dayPlan,daySummaryand
dayDirectivealready stored the day and were served byidx_agent_entities_agent_type_sub.Neither old value was used as a query key anywhere, so repurposing them is
safe.
subtypeis a locally-derived projection, not part of the syncedpayload, so a device on an older build keeps deriving its own — no sync skew.
What changed
entitySubtypewrites the day workspace for both types; the two providersread through
getEntitiesByAgentIdAndSubtype.localDay/dayAgentIdForDatemove tolib/classes/day_plan.dartbesidedayPlanId, so the agent persistence layer can derive a capture's daywithout depending on a feature package.
dayId(synced from a peer predating it) derivesone from
capturedAtin local time — the same rulecaptureDayIdapplies on read — so legacy rows land on the right day instead of needing a
Dart-side fallback scan.
than a SQL
json_extractupdate. That is deliberate: the local-timederivation cannot be expressed faithfully in SQL, and a backfilled value that
disagreed with the writer would silently drop rows out of their day.
Undecodable rows are skipped rather than aborting the upgrade.
Testing
1902 daily_os_next + 5066 agents tests pass; analyzer and formatter clean.
New coverage: day-scoped reads return only the requested day; a legacy capture
with no
dayIdis found by its derived day; an explicitdayIdwins over thederivation (a capture recorded after midnight can belong to the previous
planning day); and the conversion tests now pin the day subtype for both types.
No CHANGELOG entry — no runtime behavior a user would notice beyond the day
view getting cheaper.
Still open in this area
_plannerOwnsDay(day_agent_service.dart) still scans the planner's wholecapture history via
getCaptureEventMetaByAgentIdon every day-agentresolution, which runs ahead of both providers. It is now a one-line query
change on top of this index and I left it out to keep the schema migration
reviewable on its own — happy to fold it in here if you would rather.