Skip to content

feat(explain): phase progress for fetch + data-loading pipeline#1222

Draft
peyton-alt wants to merge 4 commits into
mainfrom
feat/explain-fetch-progress
Draft

feat(explain): phase progress for fetch + data-loading pipeline#1222
peyton-alt wants to merge 4 commits into
mainfrom
feat/explain-fetch-progress

Conversation

@peyton-alt
Copy link
Copy Markdown
Contributor

@peyton-alt peyton-alt commented May 15, 2026

https://entire.io/gh/entireio/cli/trails/388

Summary

Replaces the two coarse spinners in entire explain with phase-aware progress events so users can see which fetch/load sub-step is running, and surfaces stderr from failed fetch attempts inline rather than silently swallowing them.

Before: Loading checkpoints spinner → silent fetch chain → Loading checkpoint X spinner (covering 4-5 internal sub-steps). On a teammate's checkpoint lookup, you saw an opaque "loading" stretch with no information about what was happening or why a fetch failed.

After (TTY):

✓ Loaded local checkpoints (3.1s)
→ Fetching v1 metadata from checkpoint_remote... ✓ (701ms)
→ Treeless fetch of v2 /main from origin... ✗ fatal: couldn't find remote ref refs/entire/checkpoints/v2/main
→ Reading v2 /main from local... ✓ (1ms)
→ Loading checkpoint f50ffa8e636d — associated commits...    ← single line ticking metadata → content → commits
✓ Loading checkpoint f50ffa8e636d (80ms)

After (non-TTY / | cat): same line content, no \r rewriting, each sub-step is its own line. ACCESSIBLE=1: becomes ->, becomes [ok], becomes [fail]. NO_COLOR=1: no ANSI escapes.

Architecture

  • FetchAttemptError (git_operations.go) typed wrapper around formatFilteredFetchError's flat string. Carries Prefix, RedactedTarget, Stderr, Err. Existing callers see the same .Error() string; new diagnostic callers can errors.As for structured fields. Per repo memory on error classification: no speculative phrase-matching — verbatim git stderr passthrough.
  • explainProgressWriter (explain_progress.go) the renderer. Three methods (StartPhase, FinishPhase, UpdateSublabel) backed by shared printLine/updateLine primitives. TTY detection via interactive.IsTerminalWriter; visual style matches existing statusStyles glyphs/colours.
  • checkpoint.AttemptHooks (checkpoint/attempt.go) primitive type with OnStart/OnFinish callbacks and a WithLabel helper. Defined in the checkpoint package so it doesn't import cli; zero value is a no-op so entire resume/entire attach (which also use getMetadataTree/getV2MetadataTree) keep their existing silent behaviour.
  • getMetadataTreeWithHooks / getV2MetadataTreeWithHooks (resume.go) and checkpoint.GetV2MetadataTreeWithHooks the hook-aware variants of the fetch helpers. Backward-compatible wrappers getMetadataTree and GetV2MetadataTree delegate with empty hooks.
  • newPhaseProgressHooks (explain_progress.go) adapter that pipes AttemptHooks events into the writer, unwrapping FetchAttemptError.Stderr for the inline excerpt.
  • loadCheckpointForExplain gained an onSubStep func(string) callback parameter; the caller in runExplainCheckpointWithLookup ticks through prefetching blobs → metadata → content → associated commits on a single in-place line.

Surface area

File Δ
cmd/entire/cli/git_operations.go +29 / -3
cmd/entire/cli/git_operations_test.go +54 / 0
cmd/entire/cli/explain_progress.go new, +119
cmd/entire/cli/explain_progress_test.go new, +113
cmd/entire/cli/checkpoint/attempt.go new, +30
cmd/entire/cli/checkpoint/v2_resolve.go +52 / -16
cmd/entire/cli/resume.go +172 / -52
cmd/entire/cli/explain.go +43 / -22
cmd/entire/cli/explain_export.go +6 / -6

No behaviour change for the resume/attach paths — they continue to call the legacy (no-hooks) wrappers and pass zero-value AttemptHooks where the variant signatures require them.

Test plan

  • go test ./cmd/entire/cli/ ./cmd/entire/cli/checkpoint/ passes (incl. 3 new typed-error tests and 8 new renderer tests)
  • mise run lint clean (no new issues)
  • Smoke-tested locally:
    • Local checkpoint: 4 sub-step transitions then ✓ Loading checkpoint X (XXms)
    • Teammate-style ace97f8 not-found: full fetch-chain narration with ✗ Treeless fetch of v2 /main from origin: fatal: couldn't find remote ref... inline before the final "no checkpoint or commit found" error
    • | cat: each sub-step on its own line, no \r rewriting
    • ACCESSIBLE=1: ASCII glyph substitution
    • entire resume --help: unchanged

Deferred follow-up

The final no checkpoint or commit found error itself is still terse — the diagnostic appears in the inline lines above it. A follow-up PR could wrap it with the per-attempt block (matching PR #964's cause/try row style) for users whose terminals truncate scrollback or for CI logs that grep only the final line.

🤖 Generated with Claude Code


Note

Cursor Bugbot is generating a summary for commit 9390f45. Configure here.

peyton-alt and others added 4 commits May 15, 2026 16:19
Wraps formatFilteredFetchError's flat string error in a typed struct that
carries the redacted target, trimmed/redacted stderr, and underlying exec
error. Callers can extract these via errors.As to build per-attempt
diagnostics rather than parsing the message; the Error() output is
intentionally identical to the previous flat format so existing log/test
expectations keep working.

Pure addition — no behaviour change yet. Future commits will use this to
attribute fetch-chain failures in `entire explain`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds explainProgressWriter for status-flip phase events (→ label / ✓ label /
✗ label) plus an in-place sublabel ticker for sub-step transitions during
the data-loading pipeline. Shares visual conventions with the streaming
summary UX: cyan arrow, green check, red cross, ACCESSIBLE mode strips
ANSI and substitutes [ok]/[fail], non-TTY writers get one line per event.

No callers yet — wiring follows in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the single "Loading checkpoints" / "Fetching checkpoint metadata
from remote" spinners with per-attempt phase lines, so users see which
strategy is running (checkpoint_remote, treeless origin, local,
full origin, remote-tracking — plus the v2 variants) and which one
errored on failure.

Mechanics:
- checkpoint.AttemptHooks (new): OnStart/OnFinish notifications around
  each fallback strategy. Zero-value is a no-op so existing callers
  (resume, attach) are unaffected.
- checkpoint.GetV2MetadataTreeWithHooks: variant that emits hook events
  around the 3 internal strategies; backward-compatible GetV2MetadataTree
  delegates with empty hooks.
- getMetadataTreeWithHooks / getV2MetadataTreeWithHooks (resume.go):
  same pattern at the cli layer; wrap the 5-strategy v1 chain and the
  v2 chain (including checkpoint_remote tier).
- newPhaseProgressHooks (explain_progress.go): adapter that pipes hook
  events into explainProgressWriter, rendering → label / ✓ label
  (duration) / ✗ label: <first stderr line> with FetchAttemptError
  unwrapping for accurate diagnostics.
- runExplainAuto + matchCheckpointPrefixWithRemoteFallback: use the
  hooks pipeline instead of startSpinner.

No behaviour change for the resume/attach paths (they continue to pass
zero-value hooks, getting the same no-op rendering as before).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the single opaque "Loading checkpoint <id>" spinner with a
phase whose sublabel ticks through the 4 internal sub-steps:
prefetching blobs → metadata → content → associated commits. On TTY
this is one line that updates in place; on non-TTY each sub-step
emits its own line so logs stay readable. The phase finishes with the
total elapsed time.

Threading is a simple onSubStep callback into loadCheckpointForExplain;
nil callers (none currently) get the old silent behaviour.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 15, 2026 21:00
@peyton-alt peyton-alt requested a review from a team as a code owner May 15, 2026 21:00
@peyton-alt peyton-alt marked this pull request as draft May 15, 2026 21:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the entire explain user experience by replacing coarse spinners with phase-aware progress output across the metadata fetch + checkpoint data-loading pipeline, while also surfacing captured git fetch stderr (redacted) inline via a typed error wrapper.

Changes:

  • Introduces a typed FetchAttemptError to preserve redacted fetch target + trimmed stderr while keeping the same Error() string and errors.Is behavior.
  • Adds a small progress renderer (explainProgressWriter) and a hook mechanism (checkpoint.AttemptHooks) to emit per-attempt start/finish events during fetch/read fallback chains.
  • Updates explain and metadata-tree resolution paths to use hook-aware variants and to provide sub-step ticking during checkpoint load; adds unit tests for both the typed error and renderer.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
cmd/entire/cli/git_operations.go Adds FetchAttemptError and returns it from formatFilteredFetchError for structured diagnostics.
cmd/entire/cli/git_operations_test.go Adds tests for typed fetch error behavior, redaction, and errors.Is/As.
cmd/entire/cli/explain_progress.go New progress writer + hook adapter to render phase lines and sub-step updates.
cmd/entire/cli/explain_progress_test.go New tests covering non-TTY output, accessible glyphs, and duration formatting.
cmd/entire/cli/checkpoint/attempt.go New AttemptHooks primitive (OnStart/OnFinish) with WithLabel helper.
cmd/entire/cli/checkpoint/v2_resolve.go Adds hook-aware v2 metadata tree resolution with labeled attempts.
cmd/entire/cli/resume.go Adds hook-aware v1/v2 metadata tree helpers and wraps fallback strategies with attempt hooks.
cmd/entire/cli/explain.go Replaces spinners with phase progress; adds sub-step ticking during checkpoint load.
cmd/entire/cli/explain_export.go Updates remote-fallback matching to use hook-aware metadata fetch and render phase attempts.

Comment thread cmd/entire/cli/resume.go
@@ -381,6 +381,14 @@ func resolveLatestCheckpoint(ctx context.Context, checkpointIDs []id.CheckpointI
//
// Fallback order: treeless fetch → local → checkpoint_remote → full origin fetch → remote tree.
Comment thread cmd/entire/cli/resume.go
Comment on lines +543 to +552
return nil, nil, repoErr
}
logRefHash(remoteRepo, "remote-tracking")
remoteTree, err := strategy.GetRemoteMetadataBranchTree(remoteRepo)
if err != nil {
remoteErr = err
logging.Debug(logCtx, "remote metadata tree also not available",
slog.String("error", err.Error()),
)
return nil, nil, fmt.Errorf("read remote v1 metadata tree: %w", err)
Comment on lines +41 to +45
var (
tree *object.Tree
repo *git.Repository
runErr error
started bool
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9390f45. Configure here.

Comment thread cmd/entire/cli/explain.go
// Handle raw transcript output
if rawTranscript {
stopLoad(false)
loadPW.FinishPhase(loadPrefix, true, formatPhaseDuration(time.Since(loadStart)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate progress line when generate and rawTranscript both set

Low Severity

When both --generate and --raw-transcript flags are set, loadPW.FinishPhase(loadPrefix, true, ...) is called twice for the same loadPrefix — once at the start of the if generate block and again in the if rawTranscript block. The old code avoided this by reassigning stopLoad to a new spinner inside the generate block, so the rawTranscript block stopped the reload spinner instead. The new code uses the same loadPW/loadPrefix throughout, producing a duplicate "✓ Loading checkpoint X" line with a misleading duration.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9390f45. Configure here.

Comment thread cmd/entire/cli/resume.go
if remoteErr != nil {
return nil, nil, fmt.Errorf("metadata branch not available: %w", remoteErr)
}
return nil, nil, errors.New("metadata branch not available")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final error loses cause when openRepository fails last

Low Severity

In getMetadataTreeWithHooks, the remoteErr variable is only set when GetRemoteMetadataBranchTree fails, but not when openRepository fails in the remote-tracking attempt. If openRepository fails, remoteErr stays nil, and the function falls through to errors.New("metadata branch not available") — losing the underlying cause. The old code returned fmt.Errorf("failed to open repository: %w", repoErr) with the specific error.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9390f45. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants