fix(attribution): deterministic why metadata + interactive viewer + honest tag legend - #1613
Open
suhaanthayyil wants to merge 4 commits into
Open
fix(attribution): deterministic why metadata + interactive viewer + honest tag legend#1613suhaanthayyil wants to merge 4 commits into
suhaanthayyil wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a non-determinism bug in entire why <file>[:<line>] where equivalent clones could resolve different checkpoint context depending on which metadata/session blobs were locally available. It restructures attribution’s checkpoint metadata resolution to perform a memoized, once-per-run remote refresh when local metadata reads are incomplete, and improves miss reasons so users can distinguish “not fetched”, “fetch failed”, and “remote lacks checkpoint”.
Changes:
- Added an explicit
FetchURLWithSourceAPI so callers can reliably tell whether the fetch URL came fromcheckpoint_remotevs an origin fallback. - Reworked attribution checkpoint context reads to detect incomplete local reads (including partially/fully unreadable session records), perform at-most-once durable refresh + repo reopen, and emit deterministic
MetadataMissingReason. - Updated attach/error guidance to treat fetch suggestions as either a pasteable command or prose (when settings/URL derivation prevents a safe command), and added a comprehensive regression test suite for the refresh behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/checkpoint/remote/util.go | Adds FetchURLWithSource and routes FetchURL through it to expose a sound “source” signal. |
| cmd/entire/cli/checkpoint/remote/util_test.go | Adds coverage for the new source-signal behavior, including token rewrite and “checkpoint remote equals origin” cases. |
| cmd/entire/cli/attribution.go | Implements memoized remote refresh + store reopen, stronger evidence-gated miss reasons, and debug logging for diagnosis. |
| cmd/entire/cli/attribution_test.go | Extends reader stubs and pins default backend env to keep attribution tests hermetic. |
| cmd/entire/cli/attribution_refresh_test.go | Adds a comprehensive suite validating determinism, refresh semantics, evidence gating, and error messaging. |
| cmd/entire/cli/attach.go | Improves missing-checkpoint guidance by distinguishing command vs prose suggestions and introduces branch-vs-refs refspec suggestions. |
suhaanthayyil
force-pushed
the
fix-why-metadata-consistency
branch
from
July 7, 2026 16:01
e31f619 to
c1d8ecd
Compare
Adds an opt-in TUI for `entire why <file>` on top of the current attribution subsystem: master-detail Bubble Tea viewer over the already-resolved attribution (no git/network I/O inside the TUI) with [AI]/[MX]/[HU] line tags, a per-line explanation pane (agent/model, session with an OSC 8 entire.io link, checkpoint, prompt, intent, metadata-missing reason, candidates), n/p agent-line jumps, and enter to expand the full prompt. Agent-safe (Agent-Safe CLI Fallbacks): --tui is opt-in AND TTY-gated (IsTerminalWriter) and skipped in accessible mode; non-TTY --tui falls through to the identical plain output, and --json always wins. Rebased onto current main: the determinism/refresh rework this branch originally carried has since landed on main (fetch-on-miss, missingCheckpointError, suggestCheckpointFetchCommand), so this branch is now purely the interactive viewer. Co-authored-by: Cursor <cursoragent@cursor.com>
The interactive why viewer padded the left column with %-*s, which counts byte length — once color escape codes are present the separator column and detail pane drift (tests missed it: they render with color off). Fit each row and join the panes ANSI-aware (xansi.Truncate + lipgloss.Width + JoinHorizontal), mirroring the experts viewer. Also removes the now-unused whyMarkerLegendNote constant (unused lint) — main's plain why/blame output carries its own legend note; the TUI footer shows the one-line legend. Co-authored-by: Cursor <cursoragent@cursor.com>
Trail 737 finding: sessionWebURL hardcoded https://entire.io, so on staging/self-hosted/dev the rendered session hyperlink pointed at production. Delegate to expertsSessionURL, which builds the same /gh/:org/:repo/session/:id route via expertsWebBaseURL() (honoring ENTIRE_WEB_BASE_URL / the active API host) — matching the experts command. Co-authored-by: Cursor <cursoragent@cursor.com>
The interactive `why --tui` path took a `<file>:line` / `--line` target and opened the viewer at line 1 when the line was outside the file, silently diverging from the plain path which errors. Validate the requested start line before launching the TUI so both paths report "line N is outside <file>". Co-authored-by: Cursor <cursoragent@cursor.com>
suhaanthayyil
force-pushed
the
fix-why-metadata-consistency
branch
from
July 15, 2026 02:46
6b2411a to
05d8642
Compare
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.
Trail: https://entire.io/gh/entireio/cli/trails/737
What
entire why <file>[:<line>]could show three different answers on three equivalent clones of the same repo at the same commit — full metadata on one, partial on another, bareCheckpointIDon a third. This makes the resolution deterministic: given the same remote state, every clone converges on the same output, and when it can't, the miss reason says exactly why and how to recover.Root cause (what remained after #1535 /
4f435384d)Earlier work already added
MetadataMissingReason, file-level enrichment, and the fetch suggestion. Two residual defects kept the issue alive:git gc-pruned objects — see KNOWN_LIMITATIONS ENT-241), the resolver setMetadataMissingwith no reason and no remote refresh. Worse, on multi-session checkpoints the session loop silentlycontinued over unreadable records, so which fallback session got picked depended on which blobs a clone happened to have — different agent/model/prompt per clone, unflagged.fetchCheckpointContext→ fresh lookup →getMetadataTree) and discarded the fetch error. A file with N missing checkpoints paid up to N network timeouts, and the reason couldn't distinguish "fetch failed (offline/auth/no remote)" from "the remote genuinely lacks this checkpoint".Fix
Restructured the resolver around a memoized once-per-resolver refresh:
entire/checkpoints/v1; on success the store is re-pointed at a fresh repo handle (post-fetch packfiles are invisible to the already-open handle) and the read retried. Retries are gated on the store having actually changed during the call (no wasted double reads), and a retry that reads worse than the local pass (an unpushed local-only checkpoint) keeps the local data.getMetadataTree, whose local-branch fallback reports success when every actual fetch failed — that would have made an offline run claim "the remote doesn't have this checkpoint" (found by the audit; regression-pinned by a test that fails on that seam).MetadataMissingReasonnow distinguishes all the recovery situations the issue reporters couldn't tell apart:git fetch … entire/checkpoints/v1to run;entire/checkpoints/v1" (no pointless fetch suggestion);.entire/logs/for field diagnosis (issue ask Improved entire enable messaging #6).entire blamestays local-only/fast by design (unchanged semantics; it now also gets a reason for the sessions-unreadable case). File-level and line-levelwhyshare the same resolution path (both enrich).Audit
Adversarially audited in a fix-until-clean loop (multi-lens review; every finding independently adversarially verified before being acted on; exit = two consecutive clean passes). Eleven fix rounds landed; highlights of what the loop caught in my own cuts, all fixed and regression-pinned:
getMetadataTree's local-branch fallback reported success while offline (an offline run would claim "not pushed"); an origin fallback masked a configured checkpoint remote's failure; a URL-string-equality guard was evadable by token rewrites and false-positived on legitimately-equal URLs → replaced with an explicitFetchURLWithSourcesource signal (mirrorsPushURL).RefFetchercapability parity with the initial store, and the strong "the remote's entire/checkpoints/v1 does not contain it" evidence is gated on the git-branch backend (the v1 refresh proves nothing about per-checkpoint refs — refs-backend misses get neutral wording).ENTIRE_CHECKPOINTS_PRIMARY), and the real-seam tests clearENTIRE_CHECKPOINT_TOKENso no developer/CI environment leaks in (one test provably failed under a refs-backend env before the pin).Testing
attribution_refresh_test.go: unreadable-session reason on the no-fetch path; refresh restores unreadable sessions; refreshed-but-absent says "not pushed" (and does not suggest a fetch); at-most-once refresh on both the failure and success paths (fetch/reopen counted across misses); local-data-kept when the refreshed read is worse; differently-damaged clones converge on the identical context; partial multi-session reads trigger refresh and resolve the file-matching session; real-seam regression test (local metadata branch + no remote → "remote refresh failed", not "not pushed") — verified to fail on the pre-fix seam.mise run lint— 0 issues;mise run test:ci— unit + integration + Vogon canary green on the committed tree.Related
entire whydeterministic across clones and surface actionable checkpoint-metadata miss reasons karthik-rameshkumar/cli#1 (thanks — the fetchOnMiss-parity and reason-propagation direction was right and much of it had landed via4f435384d); this PR fixes the deeper residual non-determinism it didn't reach: the partial-blob/session-selection divergence, the per-miss fetch churn, and the unsound refresh-success signal.Part 2: interactive viewer + insider-feedback fixes
An interactive viewer for
entire why <file>plus the readability fixes insiders asked for — built strictly under the new Agent-Safe CLI Fallbacks rules (#1596).entire why <file> --tuiMaster-detail Bubble Tea viewer over the already-resolved attribution (no git/network I/O inside the TUI):
[AI]/[MX]/[HU]/[??]tags and~/?markers, windowed scrolling for large files.Session promptcaveat), intent, metadata-missing reason, candidate checkpoints, and thecheckpoint explainhint (suppressed when metadata is missing, matching the text view).n/pnext/previous agent-attributed line (jump straight to the interesting lines), enter expands the full prompt/candidates, pgup/pgdn, g/G, q.why <file>:12 --tuiopens positioned at line 12.Agent-safe by construction
--tui) and TTY-gated (interactive.IsTerminalWriter) and skipped in accessible mode — the exactexperts --tuipattern the guidelines cite as good.--tuifalls through to the same deterministic plain output;--jsonalways wins over--tuiso scripted callers never block. Both pinned by tests against a non-TTY writer.Insider-feedback fixes (text output)
blame, why file view, TUI footer):[AI] all agent · [MX] agent+human mixed · [HU] no agent · [??] uncommitted— wording per soph's correction (AI = 100% agent-authored checkpoint work; MX = agent + human edits; careful not to overclaim). Full sentences in the TUI detail pane.Testing
Update/View, color off): render + summary, navigation → prompt detail, start-line positioning,n/pagent-line jumps (including exhaustion + status message), expand toggle, missing-metadata reason + suppressed explain hint, quit keys, empty file, tiny window.--tuion a non-TTY writer produces the plain file summary;--tui --jsonproduces JSON.n/pfor paging — the viewer rebinds paging to pgup/pgdn son/pcan do agent-line jumps (commented).mise run lint0 issues; fullcmd/entire/clisuite green;mise run test:cigreen.Notes
entire whystays hidden/labs. The TUI addresses the insiders' "why is hard to read" feedback and pfleidi's browse-the-file/summit-linking ideas without touching the default output contract.Part 3:
entire blame— what actually changes (and what doesn't)To be explicit about the two commands, since they share the resolver:
entire blamebehavior is unchanged: local-only and fast, never fetches (now pinned by seam-trap tests that fail if the blame path ever refreshes). It gains only diagnostics + the legend:MetadataMissingReason(before: flagged missing with no reason and no recovery hint);entire whyis where the behavior changes live: remote enrichment on miss (Part 1) and the opt-in TUI (Part 2).Tag legend — per-commit inference, spelled out
Per the internal wording guidance: the
[AI]/[MX]/[HU]tags are a per-commit inference, not a per-line truth. It shows[AI]only when 100% of the commit's checkpointed work was agent-authored; otherwise[MX];[HU]when the commit had no agent checkpoint. Shown inblame, thewhyfile view, and the TUI footer:The note line exists because the rule is not what people infer on their own (a mostly-agent commit with one human tweak renders every line
[MX], including the purely agent-written ones).[??]appears on a conditional marker line only when uncommitted lines exist.