Entire

/simplify → 4 cleanup agents in parallel → apply the fixes

You are improving the quality of the changed code, not hunting for bugs. Review it for reuse, simplification, efficiency, and altitude issues, then fix what you find. Do not look for correctness bugs — that is what /code-review is for.

Phase 0 — Gather the diff

Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1 if there's no upstream) to get the unified diff under review. If there are uncommitted changes, or the range diff is empty, also run git diff HEAD and include the working-tree changes in scope — the review often runs before the commit. If a PR number, branch name, or file path was passed as an argument, review that target instead. Treat this diff as the review scope.

Phase 1 — Review (4 cleanup agents in parallel)

Launch 4 independent review agents via the Agent tool, all in a single message so they run concurrently. Pass each agent the diff and one of the four angles below. Each returns its findings with file, line, a one-line summary, and the concrete cost (what is duplicated, wasted, or harder to maintain).

Reuse

Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.

Simplification

Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.

Efficiency

Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.

Altitude

Check that each change is implemented at the right depth, not as a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer generalizing the underlying mechanism over adding special cases.

Phase 2 — Apply the fixes

Wait for all four agents to complete, dedup findings that point at the same line or mechanism, and fix each remaining one directly. Skip any finding whose fix would change intended behavior, require changes well outside the reviewed diff, or that you judge to be a false positive — note the skip rather than arguing with it. Finish with a brief summary of what was fixed and what was skipped (or confirm the code was already clean).

3mo ago·14m

Tests pass and the package builds. (The committed.go:911 WriteString diagnostic is pre-existing, unrelated to this diff — I only touched committed_reader_resolve.go — so I'm leaving it.)

Summary

Four cleanup agents (reuse, simplification, efficiency, altitude) reviewed the checkpoint_version metadata diff. After verifying each finding against source:

Fixed (1):

Redundant re-normalization at the wrong layer — committed_reader_resolve.go:53 called normalizeCheckpointSummary on a summary that GitStore.ReadCommitted (the only production CommittedReader) already normalizes. Reverted to return summary, nil. Legacy-version defaulting now lives solely in the storage/deserialization layer where on-disk data is read; consumers trust the reader's contract. Nil handling is unaffected — it comes from the explicit if summary == nil guard one line above. No production behavior change; no resolver test depended on resolver-level version defaulting.

Skipped, with reasons:

Altitude agent's headline fix — move defaulting into CheckpointSummary.UnmarshalJSON — Rejected. The codebase deliberately preserves a raw vs. normalized distinction: TestReadCommittedDefaultsLegacyCheckpointVersion reads the raw blob (expecting empty) and asserts ReadCommitted surfaces branch-v1. A custom UnmarshalJSON would normalize every deserialization, erasing the ability to read the true persisted version — which future format-migration code (the planned refs-v1 work) will need. It would also break that test.
Dead nil-guard in normalizeCheckpointSummary — Kept. Though no current caller passes nil, the nil pass-through is what lets callers write the concise return normalizeCheckpointSummary(summary), nil idiom; removing it would force more verbose call sites.
Write-side checkpointVersion := CheckpointVersionBranchV1 "unreachable" — False positive. It's the live default for brand-new checkpoints (no existing root metadata entry); only overwritten when an existing summary is present.
readCommittedInfoFromCheckpointTree (line 1261) unnormalized — Not a gap. It produces CommittedInfo, which doesn't carry the version field, so normalization is irrelevant there.

The change is otherwise well-factored: the two remaining normalize calls (readSummaryFromBlob, ReadCommitted) each serve distinct direct-caller populations, and the write path correctly reuses a single existing-summary read for all preserved fields.