docs(storage): document why session_insights doctor repair isn't redundant - #3579
Conversation
|
Warning Review limit reached
Next review available in: 53 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
…ndant Problem: polylogue-cfvvt's automagic-invariants audit found that `doctor --repair --target session_insights` looks covered by two daemon automatic mechanisms (the per-ingest insights ConvergenceStage and the periodic convergence_debt retry loop), the same redundancy shape used to justify removing blob-gc's --yes mutate path (#3575). It deferred a final call to polylogue-ygfwa pending a closer look, since session_insights repair is threaded through more surface than a single-file deletion. Solution: traced both automatic mechanisms (make_insights_stage in daemon/convergence_stages.py and the periodic drain in daemon/cli.py) and confirmed they only ever call rebuild_session_insights_sync (per-session profile/work_events/phases). Neither calls refresh_session_insight_aggregates_sync, the archive-wide refresh of thread materialization, tag rollups, and provider-day aggregates that repair_session_insights runs when aggregate debt is nonzero -- verified via grep that repair.py is the only caller of that function in the tree. repair_session_insights is also reused directly by maintenance/rebuild_index.py's terminal stage to materialize insights for a freshly built inactive generation before promotion, a scenario the daemon (which only touches the live/active generation) cannot reach. This is a real gap, not audit-summary redundancy: KEEP-WITH- REASON, not DELETE. Documented the specific mechanism gap inline on repair_session_insights so a future redundancy pass doesn't have to re-derive it. Verification: devtools verify --quick (exit 0, all 19 checks pass); docstring-only change, no behavior touched. Ref polylogue-ygfwa
Ref polylogue-ygfwa
4ce8196 to
9de444e
Compare
1 participant
Summary
Documents, inline on
repair_session_insights, the concrete mechanism gap that makesdoctor --repair --target session_insightsgenuinely not redundant with the daemon's automatic convergence paths. No behavior change.Problem
polylogue-cfvvt's automagic-invariants audit found
doctor --repair --target session_insightsstructurally similar to the redundancy shape that justified removing blob-gc's--yesmutate path (#3575): two automatic mechanisms already exist (the per-ingest insightsConvergenceStage, and a periodicconvergence_debtretry loop). It deliberately deferred a final delete-or-keep call to polylogue-ygfwa rather than rushing it, since session_insights repair is threaded through more surface (REPAIR_HANDLERS, the maintenance planner/replay executor, archive-debt status reporting) than a single CLI file.Solution
Traced both automatic mechanisms end to end:
make_insights_stage(polylogue/daemon/convergence_stages.py)_periodic_convergence_check→_retry_convergence_debt_once→_drain_convergence_debt_onceinpolylogue/daemon/cli.py), which just re-drivesmake_default_convergence_stagesBoth only ever call
rebuild_session_insights_sync(per-session profile/work_events/phases). Neither callsrefresh_session_insight_aggregates_sync— the archive-wide refresh of thread materialization (threads/thread_sessions), tag rollups (session_tag_rollups), and provider-day aggregates.repair_session_insightsis the only caller ofrefresh_session_insight_aggregates_syncin the tree (confirmed by grep acrossdaemon/and the rest of the codebase); it invokes that refresh whenever_session_insight_aggregate_debt_count(missing_thread_materialization_count,stale_thread_count,orphan_thread_count,stale_tag_rollup_count,stale_day_summary_count) is nonzero.So a
SESSION_INSIGHT_MATERIALIZER_VERSIONbump (or anything else that stales thread/tag-rollup aggregates archive-wide) leaves that debt stuck forever unless the manual repair runs — the daemon has no automatic route to clear it.repair_session_insightsis also reused directly (not via the doctor CLI) bymaintenance/rebuild_index.py's terminal stage, to materialize insights for a freshly built inactive generation before promotion — a scenario the daemon (which only ever touches the live/active generation) cannot reach at all.Disposition: KEEP-WITH-REASON, following the same evidence bar as the
rebuild-indexinvestigation (polylogue-rpuqn), which similarly found the daemon path was not actually equivalent at full scope and correctly kept the manual command.Verification
devtools verify --quick— exit 0, all 19 checks pass (docstring-only change, static gates only; no behavior touched, no test selection needed).Ref polylogue-ygfwa