Skip to content

fix(cli,importers): stop importer warnings from smearing the sync frame - #331

Merged
upsetbit merged 4 commits into
masterfrom
fix/sync-importer-log-scope
Aug 29, 2026
Merged

fix(cli,importers): stop importer warnings from smearing the sync frame#331
upsetbit merged 4 commits into
masterfrom
fix/sync-importer-log-scope

Conversation

@upsetbit

Copy link
Copy Markdown
Contributor

The report

prosa sync came back with a garbled progress display:

2026/08/28 20:12:14 WARN codex: malf→ local        import→ local        importing  1308 / 1575 · imported 44 …

Two independent facts produce it.

1. The source data really is torn — the warnings are correct

Codex appends to sessions/**/rollout-*.jsonl while a session runs, and a torn
write occasionally leaves a line cut mid-token:

line 521  {"timestamp":"2026-08-28T19:43:50.796Z","ordinal":520,"type":"inter_agent_communicatio
line 522  {"timestamp":"2026-08-28T19:44:59.396Z","ordinal":520,"type":"inter_agent_communication_metadata","payload":{…}}

Measured across one day's tree: 17 torn lines in 64 041 lines, hitting 15 of
125 files
. Every line number in the warning output points at a genuinely
invalid line, and errors 0 confirms the imports themselves succeeded. Not a
buffer problem either — the scanner is already at 16 MiB and the longest line
in the worst file is 1.3 MB. No parse decision changes in this PR.

2. Nothing owned the terminal

spinner.Run drives Bubble Tea on stdout with no alt-screen, so the
standard renderer repaints by moving the cursor up over its own last-frame line
count. Importers called package-level slog.Warnslog.Default()
stderr, unbuffered, while that frame was live. Each interleaved write
desyncs the renderer's line accounting.

The mechanism to prevent this already existed but covered only the pusher:
push.logger is swapped for warningCounterHandler and the tally surfaces as
the Warnings summary row. Importers had no equivalent hook.

The fix

ImportOptions gains a scoped Logger *slog.Logger, resolved through
importerutil.Logger(opts) (nil → slog.Default()). codex and claude-code
thread it into parseSession via a closure, so ParseFunc keeps its signature
and the four importers that never log stay untouched; grok-build passes it down
as a parameter because its Importer is a shared singleton. runSyncInteractive
points both the pusher and the importers at the same counter, so the existing
summary row reports both phases.

slog.SetDefault stays out of this path — #154 deliberately removed it, and
TestReconcileUsesPusherLogger locks that in.

Separately, malformed lines are now reported once per file (count + first
offending line + first error) instead of once per line, and codex/claude-code
skip blank lines instead of counting them as malformed. grok-build's own
emptiness guard is left byte-exact on purpose: the lines it collects are hashed
verbatim into the projection, and that hash is the dedup key, the sync hash, and
RawHash at once — TestImportHashUnchangedByBlankLines locks it.

Verified locally

Isolated store (scratch HOME), 67 real Codex files including all 15 damaged
ones, run under a pty so the interactive path actually engages.

before after
WARN lines inside the frame 17 0
summary Warnings 15 diagnostic logs suppressed in TTY; use --verbose to see them
sessions / turns / session_tools / session_usage / kinds 66 / 2558 / 87 / 66 / 7 identical
sha256 of all session rows (id,raw_hash,raw_size,first_prompt) 4879c863… 4879c863…
sha256 of all turn rows f850d8f7… f850d8f7…

Import output is byte-for-byte unchanged. Also confirmed:

  • --verbose emits 15 aggregated warnings summing to exactly 17 skipped
    lines, each carrying count= and first_line=.
  • A re-run is a no-op (imported 0 · skipped 67) with zero warnings —
    RunSingleFile short-circuits on the hash before Parse.
  • --json still keeps stdout pure NDJSON with diagnostics on stderr.

just ci and just quality pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SGJVWRRZLmai3UHR7cHtJ6

upsetbit and others added 4 commits August 28, 2026 21:43
Importers reached for package-level slog, so every warning went to
slog.Default() on stderr. During `prosa sync` in a TTY that lands in the
middle of a Bubble Tea frame being repainted in place on stdout, which
desyncs the renderer's line accounting and smears the output.

Give ImportOptions a scoped *slog.Logger, mirroring pusher.logger, and
resolve it through importerutil.Logger (nil falls back to slog.Default()).
codex and claude-code thread it into parseSession via a closure so
ParseFunc keeps its signature and the four importers that never log stay
untouched; grok-build passes it down as a parameter because its Importer
is a shared singleton.

peekSessionID stays silent: RunSingleFile scans the same file twice, so
logging there would double-report every bad line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SGJVWRRZLmai3UHR7cHtJ6
Only the pusher's logger was scoped, so importer warnings still hit
stderr while the Bubble Tea frame was repainting and smeared it. Point
both at the same counting logger from one named step, and let the
existing Warnings summary row report the tally for both phases.

The plain and JSON paths leave opts.Logger nil, which is what keeps the
row's "use --verbose to see them" truthful. slog.SetDefault stays out of
this path (#154).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SGJVWRRZLmai3UHR7cHtJ6
Codex appends concurrently, so a torn write leaves a line cut mid-token
every few thousand records; today each one earns its own WARN. Count them
during the scan and emit a single record per file carrying the tally, the
first offending line, and the first error, which is what distinguishes a
torn tail from real corruption. claude-code and grok-build get the same
treatment.

codex and claude-code also skip blank and whitespace-only lines instead
of reporting them as malformed, matching what grok-build already did.

grok-build's own emptiness guard stays byte-exact: the lines it collects
are hashed verbatim into the projection, and that hash is the dedup key,
the sync hash, and RawHash at once.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SGJVWRRZLmai3UHR7cHtJ6
Record that importer diagnostics travel on opts.Logger, that nothing
writes outside the renderer while an in-place frame is live, and that the
Warnings summary row now covers both the importers and the catch-up
phase. That row was already printed but missing from the summary grammar.

Corrects two claims in the CLI architecture page that the same paragraph
carried: the interactive decision needs both stdout and stderr to be TTYs,
and sync writes its summary to stderr.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SGJVWRRZLmai3UHR7cHtJ6
@upsetbit
upsetbit merged commit 89100a0 into master Aug 29, 2026
5 checks passed
@upsetbit
upsetbit deleted the fix/sync-importer-log-scope branch August 29, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant