Track attach reason on checkpoints#1199
Open
Soph wants to merge 4 commits into
Open
Conversation
PostCommit decides per session whether to condense it into the current commit's checkpoint. The decision had multiple branches (active session with recent interaction, file overlap, read-only-active skip, etc.) but logged only true/false, making it hard to trace why multiple sessions ended up under one checkpoint. Make the reason a first-class concept: - session.AttachReason names each branch (attach + skip variants) - shouldCondenseWithOverlapCheck returns (bool, AttachReason) - A single greppable "post-commit: attach decision" debug log records trigger (which Handle* method ran), reason, attached, and the surrounding signal fields, replacing two divergent debug logs - attach_reason flows from condenseOpts through CondenseSession to CommittedMetadata.AttachReason, persisting the decision branch on metadata.json so "entire checkpoint explain" and downstream tooling can answer "why is this session here?" without rerunning with debug logs Skip reasons stay log-only; only attach reasons are persisted, since condenseAndUpdateState only runs on attach. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Entire-Checkpoint: fb24917ec31f
`entire attach` and `entire review attach` import past sessions and mark them AttachedManually. At PostCommit time, these sessions previously flowed through the same heuristic gates as hook-driven sessions — file overlap, content match, files-touched gate — which could reject them based on heuristic mismatch even though the user explicitly imported them. The "no tracked files" gate in particular blocked review-attach sessions that legitimately have no files touched, just a transcript worth recording against the next commit. Treat AttachedManually as a first-class signal: - shouldCondenseWithOverlapCheck returns AttachReasonManual when state.AttachedManually is set and there is new content, bypassing the read-only-active, file-overlap, and content-match branches - HandleCondenseIfFilesTouched no longer requires len(FilesTouched) > 0 for manually-attached sessions - AttachReasonManual is persisted on CommittedMetadata.AttachReason alongside the other attach reasons, so "entire checkpoint explain" can show that a session came in via explicit user import rather than via the heuristic The "hasNew" gate still applies — manual attach without new content is not a useful checkpoint to write. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Entire-Checkpoint: 55abcbede3df
Four cleanups from code review of the attach_reason commits: - shouldCondenseWithOverlapCheck now takes *session.State directly instead of three unrelated booleans (isActive, lastInteraction, attachedManually) pulled from State at both call sites. Body reads state.Phase.IsActive() / state.LastInteractionTime / state.AttachedManually inline; no behavior change. - HandleCondenseIfFilesTouched: the two-arm switch was a glorified if; collapsed back to a single if/else. - condenseAndUpdateState: dropped the attachReason re-extraction from opts[0] that only fed the "session condensed" Info log. The greppable debug decision log and the persisted CommittedMetadata.AttachReason already carry the reason — the third copy in the Info log was redundant. - Trimmed comments in attach_reason.go: each constant had a paraphrase of its name plus a "Log-only" trailer; both are redundant with the type doc and IsAttach(). Kept the non-obvious WHYs (read-only-active rationale, the active-recent-interaction trailer interplay). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Entire-Checkpoint: 3c976be8c8d1
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes PostCommit’s “attach to checkpoint” decision auditable by introducing a typed attach/skip reason, consolidating debug logging into one greppable event, and persisting attach reasons into committed checkpoint metadata for later “explain” tooling.
Changes:
- Introduces
session.AttachReasonconstants to name PostCommit decision branches (attach + skip reasons). - Replaces per-handler decision logs with a single
"post-commit: attach decision"debug log carrying trigger, attached, attach_reason, and supporting signals. - Plumbs
AttachReasonthrough condensation intoCommittedMetadata.AttachReason(metadata.json) for both v1 and v2 checkpoint stores, and treats manual attach as a first-class attach reason that bypasses heuristic gates (excepthasNew).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/manual_commit_hooks.go | Computes attach decisions with typed reasons, logs a unified decision event, and passes attach reason into condensation options (incl. manual-attach bypass). |
| cmd/entire/cli/strategy/manual_commit_condensation.go | Adds attachReason to condensation options and persists it into WriteCommittedOptions for committed metadata writing. |
| cmd/entire/cli/session/attach_reason.go | Adds the AttachReason typed enum/constants and an IsAttach() helper. |
| cmd/entire/cli/checkpoint/committed.go | Persists opts.AttachReason into v1 session CommittedMetadata. |
| cmd/entire/cli/checkpoint/v2_committed.go | Persists opts.AttachReason into v2 session CommittedMetadata. |
| cmd/entire/cli/checkpoint/checkpoint.go | Adds AttachReason to WriteCommittedOptions and CommittedMetadata JSON schema/docs. |
Three follow-ups from Copilot's review on #1199: - Fix incorrect files_touched=0 in HandleCondense's call to logAttachDecision — the greppable decision log emitted a misleading signal for the ACTIVE/IDLE trigger. Now passes len(state.FilesTouched). - Fix a routing gap in the manual-attach bypass: for ENDED phase, the session state machine routes EventGitCommit to ActionDiscardIfNoFiles when HasFilesTouched=false, never reaching HandleCondenseIfFilesTouched. This meant my previous commit's "manual attach with no files still attaches" was only true for IDLE/ACTIVE — ENDED sessions imported via `entire review attach` (the case where empty FilesTouched is most realistic) silently went the discard route. Fix at the transition-context boundary: HasFilesTouched is OR'd with state.AttachedManually, routing manual imports to the condense action where shouldCondenseWithOverlapCheck already short-circuits to AttachReasonManual. - Add two tests covering the new diagnostic and behavior: - TestWriteCommitted_PersistsAttachReason: full Write → Read flow confirms AttachReason survives onto metadata.json and reads back through ReadSessionContent. TestCommittedMetadata_AttachReason additionally pins the on-disk JSON key as "attach_reason" with omitempty. - TestPostCommit_ManualAttach_BypassesFilesTouchedGate: ENDED session with AttachedManually=true and empty FilesTouched is condensed, and the persisted metadata records "manual_attach". Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Entire-Checkpoint: 1813bcf854a2
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/368
Track attach reason on checkpoints
PostCommit decides per session whether to condense it into the current commit's checkpoint, with several decision branches (active-recent-interaction, file overlap, read-only-active skip, etc.). The decision logged only true/false, making it hard to trace why multiple sessions ended up under one checkpoint.
This PR makes the decision a first-class signal:
session.AttachReasontyped string names each branch (active_recent_interaction,file_overlap,manual_attach, plus skip variants)"post-commit: attach decision") replaces two divergent decision logs, carryingtrigger(whichHandle*ran),attach_reason,attached, and surrounding signalsCommittedMetadata.AttachReason(metadata.json) soentire checkpoint explainand downstream tooling can answer "why is this session here?" without rerunning with debug logsmanual_attachis a first-class reason:entire attach/entire review attachimports were previously subject to heuristic gates that could reject them despite explicit user intent. Manually-attached sessions now bypass the file-overlap, read-only-active, content-match, and files-touched gates; only thehasNewgate still applies.Skip reasons stay log-only; only attach reasons are persisted.
Commits
7514af3dfTrack attach reason on checkpoint metadata and logs65018f258Track manual_attach as an attach reasondfb60a8ecSimplify attach-reason plumbing per reviewNote
Medium Risk
Changes PostCommit condensation behavior by allowing manually-attached sessions to bypass several heuristic gates, which could affect which sessions are recorded under a checkpoint. Also adds a new persisted metadata field, so downstream consumers must tolerate the new
attach_reasonvalue.Overview
PostCommit condensation now records why a session was (or wasn’t) attached. It introduces
session.AttachReason(attach + skip reasons), replaces prior boolean decision logs with a single greppable debug log includingattach_reasonandtrigger, and threads the reason through condensation.Checkpoint session metadata now persists this decision.
CommittedMetadata/WriteCommittedOptionsgainAttachReasonand both v1 (committed.go) and v2 (v2_committed.go) writers store it asattach_reasonin per-sessionmetadata.json.Manual imports are treated as first-class attaches. Sessions marked
AttachedManuallybypass thefiles_touchedgate and other heuristic checks (overlap/content-match/read-only-active), attaching with reasonmanual_attachas long as there’s new content.Reviewed by Cursor Bugbot for commit dfb60a8. Configure here.