Sign up
View stargazersSign up

fix(server): import valid conversations within each transcript record limit#23168

merged8
main
fix/issue-17484
opened13d agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

Importing one project with Codex transcripts of 20,000, 20,000, 20,000 and 58,000 JSONL records discovers all four conversations but imports only three. Earlier files consume a shared 100,000-record allowance, so the individually valid final conversation is skipped until a retry.

Change

Apply the existing 100,000-record limit independently to each transcript. Remove the shared record allowance and its accounting; keep the transcript-count, byte, and retained-history limits. Add one regression test for the four-conversation import.

Scope and approval

Fixes #17484 GitCafe: https://git.cafe/pingdotgg/t3code/issues/17484 Triaged bug via the automated issue flow; GitCafe issue #17484. Scope is the shared record allowance, without configurable limits or changes to onboarding. No authentication, permissions, modes, sandboxing, or MCP changes.

Verification

  • Reproduced through the real web onboarding wizard and Linux dev server on 70e5e384da: discovered four, imported three; retry imported the missing conversation.
  • Red regression output: expected the fourth session, received Skipped.
  • ./node_modules/.bin/vp test run apps/server/src/project/AgentSessionScanner.test.ts after merging main: 54 passed on the final head. The existing streaming test now proves exactly 100,000 records import and 100,001 records are skipped.
  • Focused green output: four passed, covering the regression, streaming excessive-record rejection, and parser exact/overflow cases.
  • Targeted scanner lint and diff whitespace checks passed. Server typecheck was stopped after prolonged memory pressure; no local typecheck result is claimed.
  • Fresh isolated database, same fixture files: after-runtime transcript shows discovery of four and first-import success for all four, independently confirmed by a read-only database query.
  • Both independent reviewers (GPT-6.1 Sol and Claude Opus 5.5) returned MERGE with no findings.

The final proof audit returned SUFFICIENT. Final-head proof supplement supersedes the index’s older exact-cap coverage note. Current main was merged cleanly as f9c2de5e61; scanner implementation is byte-identical to the runtime-proof revision.

Evidence: index, proof matrix. The matrix has zero render rows: this changes only server logic and its test. Required proof is the red test and runtime transcript; screenshots below are supplemental context.

Before: first import, base 70e5e384daAfter: first import, fix f750ae4cf4
Before: three imported and one skippedAfter: all four imported

Rendering environment: T3 Browser panel, Linux Chromium, 1280 × 800 CSS pixels. No render code changed. Aggregate processing can now reach 100 transcripts × 100,000 records within the unchanged 4 GiB total read budget; processing remains sequential with bounded retained history.

Windows desktop shell, mobile, and Android were not exercised. The defect and fix were verified through the Linux server and web client. PR remains unmerged for the dispatcher.

Models and harnesses: GPT-6.1 Sol through Codex; Claude Opus 5.5 through Claude Agent, orchestrated in T3 Code.

Checks

20 successful · 19 skipped
All checks passed
9efb27d

Added 4 commits

  • test(server): reproduce issue 17484 shared import record budget skipa67624f
  • test(server): stream exact record limit boundaries in the Codex import scanb8e5118
  • fix(server): import every Codex transcript within the per-file record limitf750ae4
  • Merge main into fix/issue-17484f9c2de5
MmacroscopeappBot13d ago
Synced from GitHub

Approvability

Verdict: Approved at f9c2de5

Macroscope's review found this PR approvable — This is a narrowly scoped server bug fix that changes the existing import path to enforce the 100,000-record limit per transcript rather than across all transcripts. Existing byte, transcript-count, file-size, and history limits remain, and focused tests cover both boundary behavior and the reported multi-transcript case.

No code changes detected at 9efb27d. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot13d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire5.0 KiB5.0 KiB+16 B (+0.3%)6.8 KiB✅
CodexThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
CodexLive turn WebSocket wire1.2 KiB1.2 KiB+16 B (+1.3%)2.0 KiB✅
CodexLive turn WebSocket decoded21.5 KiB21.6 KiB+41 B (+0.2%)24.4 KiB✅
CodexLive turn messages12+1 (+100.0%)8✅
ClaudeTotal thread wire5.0 KiB5.0 KiB0 B (0.0%)6.8 KiB✅
ClaudeThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
ClaudeLive turn WebSocket wire1.2 KiB1.2 KiB0 B (0.0%)2.0 KiB✅
ClaudeLive turn WebSocket decoded21.9 KiB21.9 KiB0 B (0.0%)24.4 KiB✅
ClaudeLive turn messages220 (0.0%)8✅

Baseline: 07a1e79 · PR result: 9efb27d · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot13d ago
Synced from GitHub

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 48ec6c004df2aa6af8735198ad3dfe35e8434e1d and 9efb27dea364b69612e3ff243298cf01fadb6385.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The scanner now applies the 100,000-record limit separately to each transcript. Transcript-count and byte budgets remain. Tests cover oversized transcripts and multiple valid transcripts whose combined record count exceeds the limit.

Changes: Transcript record limits

Layer / File(s)Summary
Enforce per-transcript record limits
apps/server/src/project/AgentSessionScanner.ts, apps/server/src/project/AgentSessionScanner.test.ts		The reader applies the fixed record limit to each transcript. Recent-thread scanning no longer tracks a shared record budget. Tests verify that an oversized transcript is skipped, an exact-limit transcript remains importable, and four valid transcripts are imported when their combined count exceeds the limit.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: t3dotgg, mikerpen22

Merge Risk: External image Minimal · previous review 48ec6

Eligible conversations can be imported independently of their combined record count. No merge-blocking issue was identified.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title is concise, uses the required conventional format, and clearly describes the main change: applying the transcript record limit independently to each conversation.
Description checkPassedThe description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the bug, fix, scope, tests, runtime validation, limitations, and supporting evidence.
Linked Issues checkPassedIssue #17484 requires a valid later transcript to avoid rejection caused by the shared 100,000-record budget. AgentSessionScanner.ts removes shared record accounting and applies MAX_IMPORT_RECORDS…
Out of Scope Changes checkPassedThe whole diff changes only AgentSessionScanner.ts and its test. The source change removes the shared record allowance without changing configurable limits, onboarding, or unrelated product areas. T…
Full details: Linked Issues check

Explanation

Issue #17484 requires a valid later transcript to avoid rejection caused by the shared 100,000-record budget. AgentSessionScanner.ts removes shared record accounting and applies MAX_IMPORT_RECORDS independently inside each transcript read. The scanner retains transcript-count and byte limits. The regression test covers four transcripts with 20,000, 20,000, 20,000, and 58,000 records and expects all four to import. The existing boundary test covers 100,000 records as importable and 100,001 records as skipped.

Full details: Out of Scope Changes check

Explanation

The whole diff changes only AgentSessionScanner.ts and its test. The source change removes the shared record allowance without changing configurable limits, onboarding, or unrelated product areas. The tests directly cover the per-transcript limit and issue #17484 regression.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 401b306504475843942cd29d80543956919ee045 and 48ec6c004df2aa6af8735198ad3dfe35e8434e1d.

External image Files selected for processing: 2
  • apps/server/src/project/AgentSessionScanner.test.ts
  • apps/server/src/project/AgentSessionScanner.ts

Limit details: You’ve used all 10 included reviews currently available.

Added 4 commits

  • Merge remote-tracking branch 'origin/main' into HEAD91ca5bd
  • Merge remote-tracking branch 'origin/main' into HEAD401b306
  • Merge remote-tracking branch 'github/main' into HEAD48ec6c0
  • Merge branch 'main' into fix/issue-174849efb27d