fix(memory-sources): drain coding-session backlog across bounded passes (#5352) - #5368
Conversation
…es (tinyhumansai#5352) The coding-session ingest capped at 15 sessions per run and never auto-continued, so users with large histories (~7,800+ files) could never fully drain their backlog: new sessions arrived faster than one manual 15-session run could clear. The 15-session cap is a per-call bound, not an arbitrary limit — a single ingest RPC must stay under the core client's ~10-minute ceiling (120s + 15*30s + 15s). Raising it to the backend's 1,000-session max would blow the RPC timeout on the first call. So the cap stays and a new multi-pass drain loop does the scaling: - drainCodingSessions() repeats bounded incremental passes until the backlog reports no more budget, the user stops, or a pass makes no forward progress. Incremental mode only, by design: each pass skips already-distilled sessions by cursor so the backlog strictly shrinks and the loop converges. Backfill re-reads every file regardless of cursor, so looping it under the per-call budget would re-process the same oldest slice forever; incremental reproduces the same persona anyway since evidence ids are content-addressed. - The card runs the full drain from one action, renders live progress (sessions imported, observations, remaining estimate) and offers a Stop control that pauses cleanly between passes. Tests cover the drain loop (drain-to-completion, stop-before-pass, no-forward-progress guard) and the card's success / stop / partial- failure / error paths.
📝 WalkthroughWalkthroughCoding-session import now uses a multi-pass drain with cumulative progress, stop handling, stall and pass limits, partial-failure reporting, updated UI controls, expanded tests, and localized status messages. ChangesCoding Session Drain Import
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08776b3c2d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| app/src/services/memorySourcesService.ts | Adds drainCodingSessions with accumulating multi-pass loop, proper termination guards (maxPasses, no-forward-progress, shouldStop), and a sound remaining estimate using files_seen - sessions_skipped - sessions_processed. |
| app/src/components/intelligence/CodingSessionsCard.tsx | Replaces single-pass ingest with full drain; adds Stop button (ref+state mirror for immediate re-render), live progress display, and toast logic keyed on result.moreRemaining to distinguish success, paused, and partial-failure states. |
| app/src/services/memorySourcesService.test.ts | Adds drain-to-completion (3-pass), stop-before-pass, and no-forward-progress-guard service-level tests; all exit conditions of the drain loop are exercised. |
| app/src/components/intelligence/tests/CodingSessionsCard.test.tsx | Replaces single-ingest mocks with drainCodingSessions mocks; adds stop-mid-drain (live progress + paused toast) and cap/stall (moreRemaining without Stop click) card tests. |
| app/src/lib/i18n/en.ts | Adds 7 new i18n keys (importAll, draining, stop, progress, remaining, stopped, stoppedMessage) and removes 3 obsolete ones (ingest, ingesting, moreRemaining). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([User clicks Import All]) --> B[Reset stop ref + state\nsetIngesting true\nsetProgress null]
B --> C{shouldStop?}
C -- yes --> X
C -- no --> D[ingestCodingSessions\nbounded pass]
D --> E[Accumulate progress\npasses, processed, observations\nremaining estimate\nonProgress callback]
E --> F{budget_hit?}
F -- no --> G[Drained ✓\nbreak]
F -- yes --> H{sessions_processed == 0?}
H -- yes --> I[No forward progress\nbreak]
H -- no --> J{passes >= maxPasses?}
J -- yes --> X[Break — moreRemaining=true]
J -- no --> C
G --> K{sessionsFailed > 0?}
X --> K
I --> K
K -- yes --> L[Toast: warning\npartialFailure message]
K -- no --> M{moreRemaining?}
M -- yes --> N[Toast: info\nImport paused]
M -- no --> O[Toast: success\ncompleteMessage]
L --> P[await load\nsetIngesting false]
N --> P
O --> P
Reviews (3): Last reviewed commit: "fix(memory-sources): type the drain mock..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@app/src/components/intelligence/CodingSessionsCard.tsx`:
- Around line 79-98: Update the toast logic in CodingSessionsCard.tsx around
line 79-98 to derive an incomplete outcome from result.moreRemaining before
selecting toast copy, title, and type. The current logic only recognizes a
manual stop as incomplete but treats result.moreRemaining: true without a manual
stop as complete success. Refactor the conditional chain to check for incomplete
drain (moreRemaining: true and no manual stop) as a separate case from the
failure and stopped cases, ensuring the toast type is not success when work
remains. Then add a test case in CodingSessionsCard.test.tsx around line 91-137
with a result containing moreRemaining: true without clicking Stop, and assert
that it yields a continuation or warning outcome instead of the complete success
toast.
- Line 101: Update the error handling around the coding-session drain to avoid
logging the raw cause from callCoreRpc or unwrap. Classify or redact the failure
before passing it to console.error, while preserving the existing drain-failure
context and handling behavior.
- Around line 111-114: Update stopDrain in CodingSessionsCard to set a state
value alongside stopRequestedRef.current so the Stop button reflects a pending
drain. Reset both the state and ref when ingestion starts, preserve the ref for
shouldStop checks, and add coverage confirming the Stop button is disabled until
the mocked drain resolves.
In `@app/src/lib/i18n/ar.ts`:
- Around line 7052-7056: Update the Arabic translations for
memorySources.codingSessions.progress, remaining, and stoppedMessage to use
count-neutral wording for numeric placeholders, explicitly identify the counts
as coding sessions, and include the session noun after {remaining}; preserve the
existing import-progress meaning and placeholders.
In `@app/src/lib/i18n/es.ts`:
- Around line 7362-7365: Update the Spanish translations for
memorySources.codingSessions.remaining and
memorySources.codingSessions.stoppedMessage to use count-neutral wording that
remains grammatically correct when {remaining} is 1, while preserving the
existing meaning and interpolation placeholder.
In `@app/src/lib/i18n/fr.ts`:
- Around line 7397-7398: The French translation for
memorySources.codingSessions.stoppedMessage presents the {remaining} placeholder
as an exact count when result.remaining is actually an estimate. Update the
message text to use approximate language such as "environ" or similar phrasing
around {remaining} to preserve that the remaining session count is approximate,
consistent with how the progress message indicates estimates.
In `@app/src/lib/i18n/pl.ts`:
- Around line 7321-7322: The Polish translation for
memorySources.codingSessions.stoppedMessage ends with an incomplete phrase
"pozostałe {remaining}" that lacks a noun, making it grammatically incomplete.
Add the appropriate Polish noun form for "sessions" after the {remaining}
placeholder to clarify what remains and complete the grammatical structure.
In `@app/src/lib/i18n/pt.ts`:
- Around line 7333-7334: Update the Portuguese translation value for
memorySources.codingSessions.stoppedMessage to use “continuar com as {remaining}
restantes” while preserving both {processed} and {remaining} interpolation
placeholders.
In `@app/src/services/memorySourcesService.ts`:
- Line 325: Normalize maxPasses before the drain loop using
CODING_SESSION_MAX_DRAIN_PASSES as the upper cap: convert valid finite values to
integers and clamp them to the supported range, while assigning a safe bounded
fallback for Infinity, -Infinity, and NaN. Apply this normalization at both
maxPasses initialization sites and add boundary tests covering non-finite and
over-cap inputs.
🪄 Autofix
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: CHILL
Plan: Pro
Run ID: 240138df-19d8-4f0d-87de-d795fd77b421
📒 Files selected for processing (18)
app/src/components/intelligence/CodingSessionsCard.tsxapp/src/components/intelligence/__tests__/CodingSessionsCard.test.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/services/memorySourcesService.test.tsapp/src/services/memorySourcesService.ts
…on click (tinyhumansai#5352 review) Addresses review on tinyhumansai#5352: - Treat any leftover backlog as incomplete: the toast now keys on result.moreRemaining (user Stop, pass cap, or a stalled pass) rather than only a manual stop, so a capped/stalled run no longer reports a complete success while work remains. - Mirror the stop flag in React state so the Stop button disables immediately on click instead of waiting for the next pass; the ref keeps the synchronous shouldStop poll contract. - Raise the drain pass cap 500 -> 2000 (~30k sessions/run) so the target ~7,800-file history drains fully instead of exiting capped, and normalize a non-finite/non-positive maxPasses override back to the default so the runaway guard can't be defeated. - Add a test for the cap/stall case (moreRemaining without a user stop) asserting a paused import, not a success toast.
…5352 CI) The Stop/pause test destructured the drainCodingSessions options directly (`({ onProgress }) => …`); since the param is optional, tsc --noEmit (Frontend Checks) rejected it as possibly-undefined (TS2339). Default the destructured param to `{}`. vitest (esbuild) did not surface this; the full typecheck does.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/services/memorySourcesService.ts (1)
351-353: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAccumulate failures across all passes.
Line 352 overwrites failures from earlier passes. If pass one has failures and a later pass has none,
sessionsFailedbecomes zero.CodingSessionsCardthen reports completion instead of partial failure. Use cumulative addition and add a multi-pass regression test.Proposed fix
- progress.sessionsFailed = result.sessions_failed; + progress.sessionsFailed += result.sessions_failed;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/memorySourcesService.ts` around lines 351 - 353, Update the pass-progress aggregation in the relevant memory-source processing method so `progress.sessionsFailed` accumulates `result.sessions_failed` rather than overwriting prior failures, while preserving the existing processed-session and observation totals. Add a regression test covering multiple passes where an earlier pass fails and a later pass has zero failures, verifying the final progress retains the failure count.
🤖 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 `@app/src/services/memorySourcesService.ts`:
- Around line 326-332: Update the maxPasses normalization near options.maxPasses
to truncate valid positive values and clamp the result to the inclusive
1..CODING_SESSION_MAX_DRAIN_PASSES range, so fractional values such as 0.5 still
allow one pass and larger overrides cannot exceed the hard limit. Preserve the
default fallback for non-finite or non-positive inputs, and add boundary tests
covering 0.5 and a value above 2,000.
---
Outside diff comments:
In `@app/src/services/memorySourcesService.ts`:
- Around line 351-353: Update the pass-progress aggregation in the relevant
memory-source processing method so `progress.sessionsFailed` accumulates
`result.sessions_failed` rather than overwriting prior failures, while
preserving the existing processed-session and observation totals. Add a
regression test covering multiple passes where an earlier pass fails and a later
pass has zero failures, verifying the final progress retains the failure count.
🪄 Autofix
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: CHILL
Plan: Pro
Run ID: 48242102-62c4-4e8f-977b-8cc287e17616
📒 Files selected for processing (3)
app/src/components/intelligence/CodingSessionsCard.tsxapp/src/components/intelligence/__tests__/CodingSessionsCard.test.tsxapp/src/services/memorySourcesService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/components/intelligence/CodingSessionsCard.tsx
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed against upstream/main: CI green on every lane that applies to the changed areas, no unresolved review threads, and the diff does what the title and body claim. Checked correctness, blast radius on shared seams, that the behaviour change is pinned by a test rather than only asserted, and i18n/em-dash compliance on any new user-facing string. Nothing blocking found.
Summary
Problem
The coding-session ingest capped at 15 sessions per run and never auto-continued, so users with large histories (~7,800+ files) could never fully drain their backlog — new sessions arrived faster than one manual 15-session run could clear (
budget_hit=truewith no follow-up).The 15 is not arbitrary: a single ingest RPC must stay under the core client's ~10-minute ceiling (
120s + 15*30s + 15s ≈ 585s). Raising it to the backend's 1,000-session max would blow the RPC timeout on the first call.Solution
drainCodingSessions()repeats bounded incremental passes until the backlog reports no more budget, the caller stops, or a pass distills nothing new. Incremental only, by design: each pass skips already-distilled sessions by cursor, so the backlog strictly shrinks and the loop converges. Backfill re-reads every file regardless of cursor, so looping it under the per-call budget would re-process the same oldest slice forever — and because evidence ids are content-addressed, an incremental drain reproduces the same persona anyway.CodingSessionsCardruns the full drain from one action, renders live progress, and offers a Stop that pauses cleanly between passes.maxPasses) and a no-forward-progress guard prevent an infinite loop on a stuck backlog.Submission Checklist
diff-coverenforces the gate.N/A: bug fix, no added/removed/renamed feature row.## Related—N/A: bug fix.N/A: fixes existing behaviour on an existing surface; no new release-cut surface.Closes #NNN.Impact
Related
sessions_remainingfield would let the card show an exact (not estimated) remaining count.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/coding-session-drain-5352Summary by CodeRabbit