fix(attribution): honest `why` prompts + unified blame/why line syntax by suhaanthayyil · Pull Request #1535 · entireio/cli

suhaanthayyil · 2026-06-26T15:03:33Z

Summary

Fixes two reported issues with entire blame / entire why (labs) plus a related latent bug found while auditing.

1. `entire why` printed a "Prompt:" that doesn't exist in `entire checkpoint explain`

why surfaced the session's seed prompt while explain shows the checkpoint-scoped transcript, so they disagreed (most visibly for attach/trail sessions, where the prompt is work on this trail …).

Root cause: when a session's prompt.txt is empty, attribution falls back to meta.ReviewPrompt — a session-level seed, not a prompt recorded for this checkpoint — and rendered it as a plain Prompt:.

Fix: track that provenance (PromptSessionLevel) and render it as Session prompt: with a dim caveat and a pointer to entire checkpoint explain, so the output no longer implies the seed prompt lives in this checkpoint's transcript.

2. `entire blame` and `entire why` used different line syntax

blame took --line N|N-M; why took positional <file>:line. Now unified — both accept both forms:

blame <file>:N, blame <file>:N-M (in addition to --line)
why <file> --line N (in addition to <file>:line)

They share a single splitFileLineSpec; why's positional parser is migrated onto it, so a colon-then-non-numeric suffix is treated as a filename (matching blame) and ranges get a friendly "use entire blame for a range" message. Conflicting positional + --line errors with not both.

3. Latent: `SessionFallback` caveat (found + fixed during audit)

Relaxed SessionFallback to flag single-session path mismatches (was gated on >1 session), but only when the session actually recorded paths that exclude the file — an empty FilesTouched means "unknown", not "renamed", so it must not print the misleading "may have been renamed" caveat (common for older metadata / attach sessions).

Not changed (deliberate)

blame showing python as the author on [HU] lines is faithful git blame committer data, not a bug — no code change.

Test plan

New attribution_consistency_test.go (TDD, red→green): prompt provenance, single-session fallback incl. the empty-paths non-flag case, unified blame/why syntax forms, conflict + range-rejection, and the shared splitFileLineSpec.
go test ./cmd/entire/cli/ — pass (incl. existing JSON-stability and 2-session fallback tests; new prompt_session_level field is omitempty).
mise run test:integration — 378 tests pass.
mise run lint — 0 issues; gofmt/go vet clean.
Built the binary and smoke-tested every form end-to-end on a real repo (blame <file>:N, :N-M, --line, why <file>:N, --line, conflict errors, range rejection).

Credits: the missing-metadata fix (the MetadataMissingReason guidance + determinism cleanup) is @karthik-rameshkumar's work from karthik-rameshkumar/cli#1, integrated here and credited as co-author on commit dd47f15. The session-fallback refinement is from @Soph's review.

🤖 Generated with Claude Code

blackgirlbytes

this PR is based on feedback we received from Insiders. Tested and it looks like it addresses the issues Insiders pointed out.

blackgirlbytes

hmm failing tests

`entire why <file>:<line>` printed a "Prompt:" that did not appear in `entire checkpoint explain <id>`. The prompt came from the session's seed prompt (empty prompt.txt -> meta.ReviewPrompt), which is a session-level artifact, while `explain` derives prompts from the checkpoint-scoped transcript. Track that provenance (PromptSessionLevel) and render it as "Session prompt:" with a caveat pointing at `checkpoint explain`, so the output no longer implies the seed prompt lives in this checkpoint's transcript. Also flag single-session path mismatches with SessionFallback (it was gated on >1 session), but only when the chosen session actually recorded paths that exclude the file -- an empty FilesTouched means "unknown", not "renamed", so it must not print the misleading rename caveat. Unify line selection so blame and why accept the same forms: - blame now accepts a positional <file>:N[-M] in addition to --line - why now accepts --line in addition to <file>:line Both share splitFileLineSpec; why's positional parser is migrated onto it so a colon-then-non-numeric suffix is treated as a filename (matching blame) and ranges get a friendly "use entire blame" message. Conflicting positional+flag line specs error with "not both". Tests: new attribution_consistency_test.go covers provenance, the single-session fallback (including the empty-paths non-flag case), the why/blame syntax forms, conflicts, and range rejection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zxz8rDq24LPbY7E42vYdG

suhaanthayyil · 2026-06-26T15:38:32Z

@blackgirlbytes try the tests now

… disclose approximate/ambiguous blame lines Completes the two follow-ups from the blame/why review. 1. why prompt provenance (general case): prompt.txt is session-wide (extracted from transcript offset 0; `checkpoint explain` re-derives a checkpoint-scoped prompt and only falls back to it). So on a LATER checkpoint (GetTranscriptStart() > 0) the prompt `why` shows is session-level, not this checkpoint's. Flag PromptSessionLevel there too — not just the empty-prompt.txt -> ReviewPrompt seed case — so `why` renders "Session prompt:" + the caveat + `checkpoint explain` pointer whenever the prompt cannot be scoped to this checkpoint. The first checkpoint (start 0) stays exact. 2. blame disclosure parity with why: the compact and long blame tables now show a one-char marker ("~" best-effort attribution when SessionFallback or MetadataMissing; "?" multiple candidate checkpoints) plus a legend, matching what `why` shows in prose. The marker sits after the Checkpoint column so Tag/Agent/Author/Checkpoint alignment is unchanged (only Content shifts); JSON output is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zxz8rDq24LPbY7E42vYdG

…sing; refine session-fallback caveat Addresses review feedback on entireio#1535. Karthik's report (community Discord + karthik-rameshkumar#1): `entire why` on a line whose checkpoint metadata is missing printed "Full context: entire checkpoint explain <id>", but running that command fails identically ("no checkpoint or commit found matching <id>") because the same remote fetch `why` attempts has already failed. The hint pointed at a command that immediately errors. - Resolve metadata-missing during attribution (resolveFileAttribution with fetchOnMiss) and record a MetadataMissingReason that states the cause and the actionable remedy: `git fetch <remote> entire/checkpoints/v1:...`, then re-run `entire checkpoint explain <id>`. Surfaced in the line and file `why` views and in JSON. Also drops the separate post-resolution enrich step, which left the checkpoint map non-deterministic across calls. (Mechanism and tests adapted from Karthik's PR.) - Suppress the bare "Full context: entire checkpoint explain <id>" hint when metadata is missing: it would fail the same way, and the reason line above now gives the fetch-then-explain sequence. - Refine the session-fallback caveat (Soph's review): flag the multi-session fallback whose chosen session has empty FilesTouched (one of several sessions, no path evidence) via `sessionsRead > 1 || len(FilesTouched) > 0`, while still suppressing the single-session empty-paths false positive. Tests for both. Co-authored-by: karthik-rameshkumar <6728802+karthik-rameshkumar@users.noreply.github.com>

suhaanthayyil · 2026-06-28T21:19:32Z

Pushed dd47f15 — this now addresses Karthik's issue as well as @Soph's review.

Karthik's bug (Discord + karthik-rameshkumar#1): entire why on a line with missing checkpoint metadata printed Full context: entire checkpoint explain <id>, but that command fails identically — no checkpoint or commit found matching <id> — because the same remote fetch why already attempted has failed. The hint resolved to a command that immediately errors.

Fix (mechanism + tests adapted from Karthik's PR, credited as co-author):

Metadata-missing is now resolved during attribution (resolveFileAttribution with fetchOnMiss) and records a MetadataMissingReason that states the cause and the actionable remedy — git fetch <remote> entire/checkpoints/v1:…, then re-run entire checkpoint explain <id>. Shown in the line and file why views and in JSON. This also answers the "three of us get three different results" point: missing metadata means the entire/checkpoints/v1 branch wasn't fetched/pushed, and the message now says so.
The bare Full context: entire checkpoint explain <id> hint is suppressed when metadata is missing (it would fail the same way; the reason line above already gives the fetch-then-explain sequence).
Bonus: dropping the old post-resolution enrich step also fixes a non-deterministic checkpoint map across successive why calls.

@Soph's feedback: applied the union sessionsRead > 1 || len(FilesTouched) > 0 so the multi-session empty-paths fallback is flagged while the single-session empty-paths false positive stays suppressed; added tests for both (thread resolved).

New tests: TestAttributionWhyHidesExplainHintWhenMetadataMissing, TestAttributionFlagsSessionFallbackForMultiSessionEmptyPaths, TestAttributionDoesNotFlagSingleSessionEmptyPaths, plus Karthik's missing-reason/determinism tests. mise run lint clean, full cmd/entire/cli package green.

…sing; refine session-fallback caveat Addresses review feedback on entireio#1535. Karthik's report (community Discord + karthik-rameshkumar#1): `entire why` on a line whose checkpoint metadata is missing printed "Full context: entire checkpoint explain <id>", but running that command fails identically ("no checkpoint or commit found matching <id>") because the same remote fetch `why` attempts has already failed. The hint pointed at a command that immediately errors. - Resolve metadata-missing during attribution (resolveFileAttribution with fetchOnMiss) and record a MetadataMissingReason that states the cause and the actionable remedy: `git fetch <remote> entire/checkpoints/v1:...`, then re-run `entire checkpoint explain <id>`. Surfaced in the line and file `why` views and in JSON. Also drops the separate post-resolution enrich step, which left the checkpoint map non-deterministic across calls. (Mechanism and tests adapted from Karthik's PR.) - Suppress the bare "Full context: entire checkpoint explain <id>" hint when metadata is missing: it would fail the same way, and the reason line above now gives the fetch-then-explain sequence. - Refine the session-fallback caveat (Soph's review): flag the multi-session fallback whose chosen session has empty FilesTouched (one of several sessions, no path evidence) via `sessionsRead > 1 || len(FilesTouched) > 0`, while still suppressing the single-session empty-paths false positive. Tests for both. Co-authored-by: karthik-rameshkumar <6728802+karthik-rameshkumar@users.noreply.github.com>

suhaanthayyil · 2026-06-29T13:20:20Z

Pushed 4f435384d — fixes the build failure on the previous push. My earlier commit had accidentally dropped the runAttributionWhy test-signature update (a git reset --soft committed the pre-fix index), so the test file still called the old bool signature and CI failed to compile cmd/entire/cli. The amended commit includes the signature fix + the three new tests. Verified locally: go vet clean, cmd/entire/cli tests green, mise run lint 0 issues. Co-author credit preserved. The new run is sitting in action_required — needs a maintainer to approve workflows for this push.

Copilot

Pull request overview

This PR improves the usability and correctness of the labs attribution commands (entire blame / entire why) by aligning their line-spec syntax, making why’s prompt output more honest about provenance, and improving missing-metadata diagnostics.

Changes:

Unified blame/why line targeting by introducing a shared <file>:N[-M] splitter and adding why --line.
Added prompt provenance tracking (prompt_session_level) and improved missing-metadata messaging (metadata_missing_reason) for clearer why output.
Expanded tests to cover the new parsing/provenance behavior, missing-metadata reasons, and blame-table markers.

Reviewed changes

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

File	Description
cmd/entire/cli/attribution.go	Unifies line-spec parsing across commands, adds prompt provenance + missing-reason fields, and improves blame/why rendering.
cmd/entire/cli/attribution_test.go	Updates/extends unit tests for new `why` options and missing-metadata reason behavior.
cmd/entire/cli/attribution_consistency_test.go	New focused tests covering blame/why syntax parity and prompt provenance semantics.

…ble divider width - `blame` Use string now shows range support: `<file>[:line[-line]]`, matching the Long help and parser (which accept `<file>:12` and `<file>:12-20`). - The `--long` blame-table dividers used `lineWidth+94` dashes but print with a 2-space indent, making them 2 chars wider than the header (whose content width after the indent is `lineWidth+92`). Both dividers reduced to `lineWidth+92` so the separators align with the header row.

…lame) why's Use/Long didn't mention the --line flag while blame's did, leaving the two commands asymmetric in their self-documentation. (Soph review nit)

suhaanthayyil requested a review from a team as a code owner June 26, 2026 15:03

blackgirlbytes approved these changes Jun 26, 2026

View reviewed changes

blackgirlbytes requested changes Jun 26, 2026

View reviewed changes

suhaanthayyil force-pushed the fix/blame-why-attribution branch from e8758fa to 0dd9b98 Compare June 26, 2026 15:34

suhaanthayyil requested a review from blackgirlbytes June 26, 2026 15:37

Merge branch 'main' into fix/blame-why-attribution

67b0841

blackgirlbytes previously approved these changes Jun 26, 2026

View reviewed changes

suhaanthayyil dismissed blackgirlbytes’s stale review via a1e791e June 26, 2026 16:14

blackgirlbytes previously approved these changes Jun 26, 2026

View reviewed changes

Soph reviewed Jun 28, 2026

View reviewed changes

Comment thread cmd/entire/cli/attribution.go Outdated

blackgirlbytes mentioned this pull request Jun 28, 2026

entire why returns inconsistent checkpoint metadata across equivalent clones #1551

Closed

suhaanthayyil dismissed blackgirlbytes’s stale review via dd47f15 June 28, 2026 21:18

suhaanthayyil mentioned this pull request Jun 28, 2026

Make entire why deterministic across clones and surface actionable checkpoint-metadata miss reasons karthik-rameshkumar/cli#1

Closed

suhaanthayyil force-pushed the fix/blame-why-attribution branch from dd47f15 to 4f43538 Compare June 29, 2026 13:13

Merge branch 'main' into fix/blame-why-attribution

5837b97

Copilot AI review requested due to automatic review settings June 29, 2026 16:00

Copilot started reviewing on behalf of suhaanthayyil June 29, 2026 16:01 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

Comment thread cmd/entire/cli/attribution.go Outdated

Comment thread cmd/entire/cli/attribution.go

Comment thread cmd/entire/cli/attribution.go Outdated

Copilot AI mentioned this pull request Jun 29, 2026

fix(blame): correct Use string range syntax and long-table divider width #1557

Closed

blackgirlbytes previously approved these changes Jun 29, 2026

View reviewed changes

docs(attribution): document --line in why's Use/Long (symmetry with b…

96138dc

…lame) why's Use/Long didn't mention the --line flag while blame's did, leaving the two commands asymmetric in their self-documentation. (Soph review nit)

suhaanthayyil dismissed blackgirlbytes’s stale review via 96138dc June 29, 2026 17:47

Soph approved these changes Jun 29, 2026

View reviewed changes

suhaanthayyil merged commit cf70a36 into entireio:main Jun 29, 2026
9 checks passed

suhaanthayyil mentioned this pull request Jul 2, 2026

fix(attribution): deterministic why metadata + interactive viewer + honest tag legend #1613

Open

suhaanthayyil mentioned this pull request Jul 16, 2026

Attribution support? #674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(attribution): honest `why` prompts + unified blame/why line syntax#1535

fix(attribution): honest `why` prompts + unified blame/why line syntax#1535
suhaanthayyil merged 7 commits into
entireio:mainfrom
suhaanthayyil:fix/blame-why-attribution

suhaanthayyil commented Jun 26, 2026 •

edited

Loading

Uh oh!

blackgirlbytes left a comment

Uh oh!

blackgirlbytes left a comment

Uh oh!

suhaanthayyil commented Jun 26, 2026

Uh oh!

Uh oh!

suhaanthayyil commented Jun 28, 2026

Uh oh!

suhaanthayyil commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

suhaanthayyil commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. entire why printed a "Prompt:" that doesn't exist in entire checkpoint explain

2. entire blame and entire why used different line syntax

3. Latent: SessionFallback caveat (found + fixed during audit)

Not changed (deliberate)

Test plan

Uh oh!

blackgirlbytes left a comment

Choose a reason for hiding this comment

Uh oh!

blackgirlbytes left a comment

Choose a reason for hiding this comment

Uh oh!

suhaanthayyil commented Jun 26, 2026

Uh oh!

Uh oh!

suhaanthayyil commented Jun 28, 2026

Uh oh!

suhaanthayyil commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

suhaanthayyil commented Jun 26, 2026 •

edited

Loading

1. `entire why` printed a "Prompt:" that doesn't exist in `entire checkpoint explain`

2. `entire blame` and `entire why` used different line syntax

3. Latent: `SessionFallback` caveat (found + fixed during audit)