Merge Main and Resolve Transcript Conflicts

Claude Code·Fable 5·computermode·3mo ago·30hr 15min·5 Checkpoints·14 file changes·+474/-184·425.6K tokens

Pull in main to this branch and resolve conflicts

3mo ago·15m

Merge complete. Summary:

Merged origin/main into push-compact-for-v1

Pulled 367 commits from main into your 1-commit branch. The auto-merge handled all code files cleanly; two doc files conflicted, both for the same underlying reason.

Key finding

Main has removed the v1.1 checkpoint read-mirror feature entirely — the merge deleted cmd/entire/cli/strategy/v1_custom_ref_mirror.go (+test), and the new checkpoint store IgnoresCheckpointsVersion. Main also deleted all v1.1 mirror documentation. Your branch's base still carried it.

Conflicts resolved

Both conflicts mixed two independent things; I split them by what the merged code actually contains:

ContentDecisionWhy
Compact transcript (transcript.jsonl) docs — your commit's featureKeptThe code (writeCompactTranscript etc.) survived the merge into the refactored committed.go intact
v1.1 local read-mirror docs — pre-existing contextDroppedFeature was removed from main's code; keeping the docs would describe behavior that no longer exists
  • CLAUDE.md — kept compact-transcript bullet, dropped v1.1 bullet
  • docs/architecture/sessions-and-checkpoints.md — kept compact-transcript paragraph, dropped #### v1.1 local read mirror section

Verification

  • go build ./... — clean
  • checkpoint, strategy, paths package tests — all pass
  • No conflict markers or stray v1.1 references remain (the remaining "mirror" mentions are the unrelated repo mirror list command)

Merge commit d7e855b1d created on push-compact-for-v1. Not pushed.

One thing worth flagging: your compact-transcript work was the v1 counterpart to a versioning scheme that main has since reworked (the v1.1 mirror is gone, and the checkpoint store now ignores checkpoints_version). The code merged and compiles, but you may want to confirm the compact-transcript feature still fits main's new checkpoint-store design before pushing.

For now, we just want to make sure compact transcripts are pushed simultaneously as the full.jsonl transcripts: don't point metadata.json to it yet as part of this PR. Make those changes and adjust the PR title and description accordingly when ready (and push the changes up too of course)

3mo ago·12m

Done. Summary of what changed and shipped:

Behavior change

transcript.jsonl (the compact transcript) is still generated and written into the checkpoint tree on every committed write and during finalization, so it's pushed alongside full.jsonl with the entire/checkpoints/v1 branch. The metadata.json sessions[].transcript pointer now stays on full.jsonl — pointing it at the compact transcript is deferred.

Code

  • committed.go: writeTranscript keeps calling writeCompactTranscript but always returns full.jsonl as the pointer. writeCompactTranscript no longer returns the now-unused bool (fixed the unparam lint hit). Updated the doc comments in writeTranscript, replaceTranscript, and the directory-tree diagram.
  • committed_compact_transcript_test.go: the one assertion that expected the pointer to target the compact file now expects full.jsonl; the tests that the compact file is written, scoped, and regenerated stay as-is.
  • Docs (CLAUDE.md, sessions-and-checkpoints.md): pointer now documented as staying on full.jsonl, compact pushed but not yet referenced.

Verification

  • mise run fmt + mise run lint → 0 issues
  • mise run test:ci (unit + integration + e2e canary) → exit 0, all green

Git / PR

  • Committed 2b5c661e8 on push-compact-for-v1 and pushed.
  • PR #1419 retitled to "Store and push compact transcript.jsonl in v1 checkpoints (metadata pointer unchanged)" and the description rewritten: scope narrowed to write-and-push only, pointer-flip called out as a follow-up, and the stale v1.1 mirror references removed (main deleted that feature).

One note for awareness: this branch's merge commit (d7e855b1d) pulled 367 commits from main, and the prior turn's conflict resolution dropped the v1.1 mirror docs because main removed that feature from the code. That's all part of what's now on the PR.

Address the review comments on the PR if they are still applicable

3mo ago·4m

Now let me add a focused test. First, let me look at existing Codex test fixtures/helpers to build a minimal transcript with content that sanitization strips.

if it's already sanitized, we don't want to sanitize it again. it's a very expensive process

3mo ago·18m

Both threads addressed and resolved.

Final summary

ThreadReviewerFindingStatusReplyResolved
PRRT_…SpQcursor (Bugbot)Codex sanitize skipped on finalizeFixed in 847d4754aYesYes
PRRT_…SpLcursor (Bugbot)Summary pointer not updatedNot applicable (pointer always full.jsonl by design)YesYes

What changed (847d4754a):

  • replaceTranscript now sanitizes Codex transcripts (codex.SanitizePortableTranscript) before generating the compact transcript.jsonl, matching the initial-write path. Sanitization runs once per path — per your note, the already-sanitized initial-write input is not re-sanitized (writeCompactTranscript's doc now states callers pass pre-sanitized bytes).
  • Added TestUpdateCommitted_CodexCompactSanitizedLikeInitialWrite — verified it fails on raw bytes and passes with the fix.

Verification: mise run lint (0 issues), go test ./cmd/entire/cli/checkpoint/... (ok), revert-test confirmed the new test is a real regression guard.

Pushed to push-compact-for-v1; both review threads replied to and resolved on PR #1419.

3mo ago·10m

Summary

Ran 4 parallel cleanup agents (reuse, simplification, efficiency, altitude) over the compact-transcript diff (origin/main...HEAD: committed.go, checkpoint.go, paths.go, and the new test file).

Fixed: nothing — the code was already clean on three of four angles.

Skipped (1 finding): Altitude flagged the two inline if agentType == AgentTypeCodex { SanitizePortableTranscript } sites and proposed a TranscriptNormalizer agent interface. Skipped because:

  • The fix lands well outside the reviewed diff (new interface + codex-agent implementation in the agent package).
  • One of the two sites is pre-existing code from main, not introduced here.
  • The two sites apply sanitization to different downstream scopes (initial write: chunks + hash + compact; finalize: compact only). A shared helper would hide that deliberate boundary — which is why the reuse and simplification agents independently judged the inline form acceptable.

No code changes were applied. The branch is unchanged from the pushed state (847d4754a).

why are the changes in cmd/entire/cli/checkpoint/committed_compact_transcript_test.go in this PR? theoretically we already have tests for compact transcripts in the code already

3mo ago·1m

Here's what I found.

The file is new to this PR, and it tests a different layer

  • committed_compact_transcript_test.go was added by this PR (commit afd94281e); it does not exist on origin/main (git diff shows status A).
  • The "existing compact-transcript tests" are in cmd/entire/cli/transcript/compact/*_test.go (compact_test.go, codex_test.go, etc.). Those test the Compact() transformation in isolation — given raw agent JSONL, does it produce the right compact format.
  • This new file tests the checkpoint-store integration — that GitStore.WriteCommitted/UpdateCommitted actually generate transcript.jsonl, write it into the branch tree, scope it to the checkpoint start, set the metadata pointer to full.jsonl, and regenerate/sanitize on finalize. That behavior is exactly what this PR introduces, and nothing on main covered it (checkpoint_test.go's only transcript.jsonl hits are the unrelated on-disk agent-transcript.jsonl).

So the file belongs here — it's the unit coverage for the new store-layer code. It's complementary to, not a duplicate of, the compact-package tests.

But you're partly right — some assertions over-reach

One test re-validates the compact format, which is the compact package's job, not the store's. In TestWriteCommitted_WritesCompactTranscript:

The v, agent, and per-line type shape are already asserted in transcript/compact/codex_test.go etc. The store test only needs to prove it wrote and scoped the file (exists in tree, non-empty, pointer correct, scoped content present) — which the other tests already do. This matches your own guidance ([[feedback_test_scope_to_unit.md]]: don't re-test a dependency's output from a consumer's tests).

The store-specific behavior in the file (writes into tree, scoping by start line, pointer = full.jsonl, non-compactable fallback, finalize regen, Codex sanitize-on-finalize) is not redundant and should stay.

Want me to trim the format-level assertions down to the store-layer ones? That'd tighten the file to its actual unit without losing real coverage.

trim it down

3mo ago·19m

Trimmed and pushed (c4b837ba6).

What changed

Removed the assertions that re-tested the compact format (already covered by transcript/compact/*_test.go):

  • v == 1, agent == "claude-code", per-line type, and exact line count
  • the now-unused parseCompactLines helper + compactTranscriptLine type
  • the now-unused encoding/json import and the two blank agent imports (only needed for the slug assertion)

Net: −44 / +3 lines.

What stayed (the store's own behavior)

  • full.jsonl and transcript.jsonl are written into the checkpoint tree
  • the compact holds real derived content ("reply two" present)
  • scoping to CheckpointTranscriptStart (excludes pre-start content)
  • non-compactable transcript → no transcript.jsonl, pointer falls back
  • metadata pointer stays on full.jsonl
  • finalize regeneration + Codex sanitize-on-finalize

Verification

  • Removed the imports and confirmed tests still pass → proved they were vestigial (compaction keys off content; Codex sanitization uses the production package import, not the test registry)
  • mise run lint (0 issues), go test ./cmd/entire/cli/checkpoint/... (ok)

The tests now cover only the checkpoint-store unit, with format validation left to the package that owns it.

it seems undesired to me that cmd/entire/cli/checkpoint/committed.go we went from returning true or false to returning ""

3mo ago·9m

Done — pushed as aeb61502e.

You were right: that was a regression with no upside. On main, writeTranscript returned (bool, error) meaning "was a transcript written." The compact-transcript change had switched it to (string, error) — the pointer filename, with "" as the "nothing written" sentinel — only so the pointer could one day point at transcript.jsonl. Since that pointer move was deferred and the metadata pointer is always full.jsonl now, the string return carried exactly the same information as the old bool, just dressed up as an empty-string sentinel.

Reverted to match main:

  • writeTranscript returns (bool, error) again; all error paths return false.
  • Caller uses wroteTranscript + paths.TranscriptFileName directly (as on main).
  • Dropped the now-pointless pointerFile local and the constant transcript_pointer debug-log field.

The only remaining change to writeTranscript vs main is the actual feature: the writeCompactTranscript call (+ comment). Verified with mise run lint (clean) and the full checkpoint package tests (pass).