explain: address trail review comments (naming, test helpers, docs)#1569
Merged
Conversation
No behavior change — readability, consistency, and documentation only. - rename meta/metaErr -> summary/summaryErr in the --generate fast-fail; checkpoint.ReadCheckpoint returns a *CheckpointSummary, not *Metadata, so summary reads truer (and matches the post-load check it replaced). Also restore the spinner doc-comment's adjacency to its startSpinner call. - document the cheap redundant metadata-only read on the non-imported --generate path; it's an acceptable tradeoff against the larger imported-path win (skipped blob prefetch + transcript load). - comment the intentional prose-vs-JSON truncation asymmetry at the prose call site (grouped view renders up to 2*limit and notes only on a real budget hit; JSON hard-caps the flat array at limit). - TestGetBranchCheckpoints_TruncationSignal now builds commits via the testutil git helpers (which set user identity + disable GPG signing) instead of raw go-git worktree commits, matching repo convention. Entire-Checkpoint: c950a1e44d3c
Contributor
There was a problem hiding this comment.
Pull request overview
Refines entire explain implementation details and tests based on prior trail review feedback, focusing on naming clarity, documentation precision, and aligning tests with repository git-test conventions (intended to be no behavior change).
Changes:
- Renamed
meta/metaErrtosummary/summaryErrfor the--generatemetadata-only fast-fail path and expanded the rationale comment. - Repositioned the spinner pipeline comment to sit adjacent to
startSpinner. - Updated
TestGetBranchCheckpoints_TruncationSignalto build commits viatestutilhelpers and added documentation explaining the intentional truncation asymmetry between prose and JSON paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/entire/cli/explain.go | Improves naming and adds clarifying comments around imported fast-fail, spinner behavior, and truncation semantics. |
| cmd/entire/cli/explain_test.go | Switches test commit creation to repo-standard testutil helpers and clarifies intent in test comments. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Soph
approved these changes
Jul 6, 2026
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
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.
https://entire.io/gh/entireio/cli/trails/696
What
Follow-up to PR #1560: addresses the trail review comments on the
explainchanges. Readability, consistency, and documentation refinements — no behavior change.How
meta/metaErr→summary/summaryErrin the--generatefast-fail.checkpoint.ReadCheckpointreturns a*CheckpointSummary, not*Metadata;.Importedexists on both so it compiled, butsummaryreads truer and matches the post-load check it replaced. Also restored the spinner doc-comment's adjacency to itsstartSpinnercall.--generatepath — acceptable against the larger imported-path win (skipped blob prefetch + transcript load).2*limitand notes only on a real budget hit; the JSON path hard-caps the flat array atlimit).TestGetBranchCheckpoints_TruncationSignalnow builds commits via thetestutilgit helpers (user identity + GPG-signing disabled) instead of raw go-git worktree commits, matching repo convention.Verification
mise run fmt+mise run lintclean (0 issues); fullcmd/entire/cliunit suite green.Note
Cursor Bugbot is generating a summary for commit 1d4cf22. Configure here.