feat(enable): show progress while importing existing sessions - #1848
Closed
ecgang wants to merge 3 commits into
Closed
feat(enable): show progress while importing existing sessions#1848ecgang wants to merge 3 commits into
ecgang wants to merge 3 commits into
Conversation
Run previously gave callers no visibility into a potentially long import (sessions x turns with a documented O(turns^2) recompute), so first-run 'entire enable' sat silent for its whole duration. Add an optional, UI-agnostic Progress struct on Options: SessionStart fires per session after turn splitting, TurnWritten after each turn actually written (never for skips or dry runs). Nil stays byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01KY93365A3ZBK69V2948N4V4W
First-run 'entire enable' could sit silent for the whole session import (an hour on a large corpus) between the setup summary and the final Imported line. Wire the agentimport Progress reporter into user-visible output via one shared helper: - interactive terminals get the standard spinner, extended with an updatable message (startUpdatableSpinner; startSpinner now delegates to it) tracking 'session i/N - turn j/M' live - non-TTY and ACCESSIBLE runs get one plain ANSI-free line per session, keeping output complete for agents and screen readers - the standalone 'entire import <agent>' command gets the same wiring The final summary line is unchanged, and the spinner stops before any error path prints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01KY93WZV7TJ0P4CSY40NYGZ1Q
The progress counter only advanced on TurnWritten, so a fully idempotent re-import or a --dry-run pass sat at 'turn 0/M' and then rendered that stale count into the completion line. Add Progress.TurnSkipped, fired for every turn Run processes without writing (already imported, or DryRun), and drive the UI counter from both callbacks: exactly one of TurnWritten/TurnSkipped now fires per turn, so the counter always sweeps to M/M truthfully. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01KY95QCD5YDCST7YNYKW063FA
gtrrz-victor
added a commit
that referenced
this pull request
Jul 28, 2026
fix(import): gate import progress ANSI on ShouldStyle (review follow-up to #1848)
Contributor
|
@copilot resolve the merge conflicts in this pull request |
Contributor
|
Update on next steps: rather than have you chase the I've opened #1868 in this repo starting from your branch — your commits and credit are preserved intact. I've resolved the Thanks again — genuinely nice, well-tested contribution. 🙏 We'll close this one in favor of #1868. |
Contributor
tkfor21
pushed a commit
to tkfor21/entire-cli
that referenced
this pull request
Jul 29, 2026
…soles The import progress spinner (entireio#1848) drew its animated frames with cursor-control escapes (\r\033[K) whenever the writer was a terminal, gating only on IsTerminalWriter and bypassing interactive.ShouldStyle — the repo's single gate for writer-scoped ANSI. On a console that can't render ANSI (TERM=cygwin renders the ESC byte as a literal "←", GH entireio#1267) or when NO_COLOR is set, every frame emitted "←[K" garbage. The per-frame \033[K this PR added widened a previously stop-line-only escape to the whole animation, so legacy Windows consoles regressed from clean bare-\r frames to visible garbage. - startUpdatableSpinner: fall back to the completion-line-only (ANSI-free) path unless w both is a terminal and ShouldStyle(w) — fixes it at the source for every spinner caller, not just import. - newImportProgressReporter: route non-styleable terminals to the plain per-session line path (same as non-TTY/ACCESSIBLE), so NO_COLOR/cygwin users still get per-session progress instead of a lone final line. - setup_import_test.go: correct a comment naming a test (TestNewImportProgressReporter_TTYAdvancesOnSkip) that does not exist. The terminal+!ShouldStyle branch is only observable against a real TTY, so it carries no unit test — matching the PR's existing PTY-bound coverage limits; the shouldStyle decision itself is already unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01KYM4JTVVADFMT258B3020DV9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1847.
First-run
entire enableimports existing agent sessions with zero output between the setup summary and the finalImported N turn(s)line — on a large corpus that's an hour of bare cursor with no way to tell working from hung. This wires live progress into that import, following the CLI's existingstartSpinnerconventions.Honesty note on the demo: both gifs use a synthetic fixture (6 sessions × 60 turns, ~15s import) so the behavior is visible in a short recording; the real-world case that motivated this was ~1h of silence.
What changed
agentimport: optional, UI-agnosticProgressreporter onOptions—SessionStartfires per session after turn-splitting; exactly one ofTurnWritten/TurnSkippedfires per turn (skips cover already-imported turns and--dry-run), so callbacks always sum toturnCount. Nil reporter is byte-identical to before. No printing or logging inside the package.startUpdatableSpinner— the smallest variant of the existing spinner whose message can change while it runs;startSpinnernow delegates to it. Same glyphs, tick rate, 250 ms initial-draw delay, and TTY gating. One deliberate change: animated frames draw\r\033[Kinstead of bare\r, so a shrinking message can't leave stale trailing characters (final stop lines are byte-identical).newImportProgressReporter) used by both import surfaces — theentire enablefirst-run import and the standaloneentire import <agent>:Importing Claude Code sessions... (session i/N · turn j/M)updating liveACCESSIBLE=1: one plain ANSI-free line per session (Importing Claude Code session i/N (M turns)...), keeping output complete for agents and screen readers per the repo's agent-safe CLI fallback guidanceImported N turn(s) from M session(s)summary line is unchanged, and the spinner is stopped before any error path prints.Tests
agentimport: callback contract (counts, ordering, exact tuples), nil-reporter equivalence, skipped-turn and dry-run invariants (written + skipped == turnCount).cli: non-TTY progress lines (one per session, zero ESC bytes, unchanged summary line), spinner-variant behavior (fallback line, update-before-first-draw, stop semantics).mise run check(fmt, lint, unit + integration + Vogon canary) green; also manually verified against a real built binary: piped output,ACCESSIBLE=1under a PTY, and live-TTY spinner animation.Happy to adjust the message format or UX approach if maintainers prefer something different.
🤖 Generated with Claude Code