Repository navigation
Add history compaction command and TOON workaround tracking - #122
Conversation
This consolidates pm-3pbq and pm-idnz by adding the history-compact command surface, wiring it across runtime/contracts/docs/completions, and hardening decoder workaround traceability with guard tests while preserving strict verification evidence in linked pm items.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
@codex review |
|
/gemini review |
|
@gemini-code-assist review |
|
Warning Review limit reached
More reviews will be available in 6 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesHistory Compaction and Related Tracking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the history-compact command, enabling checkpoint-based history stream compaction into a synthetic baseline and a retained tail. The implementation spans CLI command registration, help documentation, shell completions, MCP server routing, contract definitions, and comprehensive unit and integration tests. Additionally, it introduces explicit upstream tracking metadata and guard tests for the TOON scalar-bracket workaround in toon-decode.ts. No review comments were provided for this pull request.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request introduces the history-compact command to compact long history streams into a synthetic baseline and a retained tail, along with adding explicit upstream tracking metadata and tests for the TOON scalar bracket workaround. Feedback on the changes suggests improving the timestamp boundary parsing in history-compact to break the loop upon encountering the first entry at or after the boundary, preventing correctness issues with out-of-order timestamps.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let compactCount = 0; | ||
| for (const [index, entry] of entries.entries()) { | ||
| const entryTs = Date.parse(entry.ts); | ||
| if (!Number.isFinite(entryTs)) { | ||
| throw new PmCliError( | ||
| `History for this item contains invalid timestamp at entry ${index + 1}.`, | ||
| EXIT_CODE.GENERIC_FAILURE, | ||
| ); | ||
| } | ||
| if (entryTs < beforeTs) { | ||
| compactCount += 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
When parsing the --before timestamp boundary, simply counting the entries that have a timestamp less than beforeTs can lead to correctness issues if the history stream contains any out-of-order timestamps (e.g., due to clock skew or rapid sequential writes). Since compaction must prune a contiguous prefix of the history, we should instead find the first entry that is at or after the boundary timestamp and stop there. This ensures we only compact entries that are strictly before the boundary.
| let compactCount = 0; | |
| for (const [index, entry] of entries.entries()) { | |
| const entryTs = Date.parse(entry.ts); | |
| if (!Number.isFinite(entryTs)) { | |
| throw new PmCliError( | |
| `History for this item contains invalid timestamp at entry ${index + 1}.`, | |
| EXIT_CODE.GENERIC_FAILURE, | |
| ); | |
| } | |
| if (entryTs < beforeTs) { | |
| compactCount += 1; | |
| } | |
| } | |
| let compactCount = 0; | |
| for (const [index, entry] of entries.entries()) { | |
| const entryTs = Date.parse(entry.ts); | |
| if (!Number.isFinite(entryTs)) { | |
| throw new PmCliError( | |
| `History for this item contains invalid timestamp at entry ${index + 1}.`, | |
| EXIT_CODE.GENERIC_FAILURE, | |
| ); | |
| } | |
| if (entryTs >= beforeTs) { | |
| break; | |
| } | |
| compactCount += 1; | |
| } |
There was a problem hiding this comment.
Code Review
This pull request introduces the pm history-compact command, which enables checkpoint-based history stream compaction by replaying mutations to a synthetic baseline and re-anchoring the hash chain. It also adds explicit upstream tracking metadata for the TOON scalar bracket workaround. The review feedback highlights two key issues: a potential history corruption risk when compacting by timestamp if the stream contains non-monotonic timestamps, and inaccurate entry numbers reported in error messages during re-anchoring due to a missing index offset.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let compactCount = 0; | ||
| for (const [index, entry] of entries.entries()) { | ||
| const entryTs = Date.parse(entry.ts); | ||
| if (!Number.isFinite(entryTs)) { | ||
| throw new PmCliError( | ||
| `History for this item contains invalid timestamp at entry ${index + 1}.`, | ||
| EXIT_CODE.GENERIC_FAILURE, | ||
| ); | ||
| } | ||
| if (entryTs < beforeTs) { | ||
| compactCount += 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Using a simple counter to determine compactCount based on timestamps can lead to severe history corruption or data loss if the history stream contains non-monotonic timestamps (e.g., due to clock drift or manual edits). Instead of counting all matching entries, we should find the contiguous prefix boundary by breaking the loop at the first entry that is equal to or newer than the target timestamp.
| let compactCount = 0; | |
| for (const [index, entry] of entries.entries()) { | |
| const entryTs = Date.parse(entry.ts); | |
| if (!Number.isFinite(entryTs)) { | |
| throw new PmCliError( | |
| `History for this item contains invalid timestamp at entry ${index + 1}.`, | |
| EXIT_CODE.GENERIC_FAILURE, | |
| ); | |
| } | |
| if (entryTs < beforeTs) { | |
| compactCount += 1; | |
| } | |
| } | |
| let compactCount = 0; | |
| for (const [index, entry] of entries.entries()) { | |
| const entryTs = Date.parse(entry.ts); | |
| if (!Number.isFinite(entryTs)) { | |
| throw new PmCliError( | |
| `History for this item contains invalid timestamp at entry ${index + 1}.`, | |
| EXIT_CODE.GENERIC_FAILURE, | |
| ); | |
| } | |
| if (entryTs >= beforeTs) { | |
| break; | |
| } | |
| compactCount += 1; | |
| } |
| function reanchorRetainedEntries( | ||
| retainedEntries: HistoryEntry[], | ||
| seed: ReplayDocument, | ||
| ): { | ||
| entries: HistoryEntry[]; | ||
| finalReplay: ReplayDocument; | ||
| } { | ||
| let replay = structuredClone(seed); | ||
| const rewritten: HistoryEntry[] = []; | ||
| for (const [index, entry] of retainedEntries.entries()) { | ||
| const beforeHash = replayHash(replay); | ||
| replay = applyHistoryPatch(replay, entry, index + 1); | ||
| rewritten.push({ | ||
| ...entry, | ||
| before_hash: beforeHash, | ||
| after_hash: replayHash(replay), | ||
| }); | ||
| } | ||
| return { | ||
| entries: rewritten, | ||
| finalReplay: replay, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The reanchorRetainedEntries function currently reports incorrect/misleading entry numbers in its error messages because it uses the 1-based index of the sliced retainedEntries array rather than the original entry numbers from the full history stream. To fix this, pass the original offset (boundary.compactCount) to reanchorRetainedEntries so that the reported entry numbers are accurate.
function reanchorRetainedEntries(
retainedEntries: HistoryEntry[],
seed: ReplayDocument,
offset: number,
): {
entries: HistoryEntry[];
finalReplay: ReplayDocument;
} {
let replay = structuredClone(seed);
const rewritten: HistoryEntry[] = [];
for (const [index, entry] of retainedEntries.entries()) {
const beforeHash = replayHash(replay);
replay = applyHistoryPatch(replay, entry, offset + index + 1);
rewritten.push({
...entry,
before_hash: beforeHash,
after_hash: replayHash(replay),
});
}
return {
entries: rewritten,
finalReplay: replay,
};
}| if (changed) { | ||
| const { checkpoint, finalReplay } = replayHistoryAndResolveCheckpoint(historyEntries, boundary.compactCount); | ||
| const retained = historyEntries.slice(boundary.compactCount); | ||
| const reanchored = reanchorRetainedEntries(retained, checkpoint); |
There was a problem hiding this comment.
Pass the offset (boundary.compactCount) to reanchorRetainedEntries to ensure accurate entry numbers are reported in error messages.
| const reanchored = reanchorRetainedEntries(retained, checkpoint); | |
| const reanchored = reanchorRetainedEntries(retained, checkpoint, boundary.compactCount); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/history-compact-command.spec.ts (1)
69-147: ⚡ Quick winAdd an explicit ISO timestamp boundary test for
--before.The suite validates default/version/noop paths but not the timestamp parsing branch, so regressions there could slip through.
Suggested test addition
+ it("compacts entries strictly before an ISO timestamp boundary", async () => { + await withTempPmPath(async (context) => { + const id = createItem(context, "Compact Timestamp Boundary"); + expect(context.runCli(["update", id, "--status", "in_progress"]).code).toBe(0); + expect(context.runCli(["append", id, "--body", "tail entry"]).code).toBe(0); + + const full = await runHistory(id, { full: true }, { path: context.pmPath }); + const boundaryTs = full.history[2]?.ts; + expect(typeof boundaryTs).toBe("string"); + + const compacted = await runHistoryCompact( + id, + { before: boundaryTs as string, author: "test-author" }, + { path: context.pmPath }, + ); + + expect(compacted.compact_boundary.kind).toBe("timestamp"); + expect(compacted.changed).toBe(true); + expect(compacted.compact_boundary.entries_compacted).toBeGreaterThan(0); + expect(compacted.history.verify_ok).toBe(true); + }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/history-compact-command.spec.ts` around lines 69 - 147, Add a new unit test that exercises the timestamp parsing branch of runHistoryCompact by creating an item (createItem), adding at least two subsequent history entries (e.g., update/append), then call runHistory(full: true) to read the entries' created_at timestamps and choose an ISO timestamp between entries as the --before value; invoke runHistoryCompact(id, { before: "<ISO-timestamp>", author: "test-author" }, { path: context.pmPath }) and assert the result reports compact_boundary.kind === "timestamp", correct entries_compacted/entries_retained/first_retained_entry values, changed===true (or unchanged for a noop case), and that history verification and history file state match the other tests' expectations (use existing helpers: withTempPmPath, runHistoryCompact, runHistory, getHistoryPath, readFile). Ensure the test name clearly states it validates ISO timestamp boundary parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@vitest.config.ts`:
- Line 177: Remove the duplicate coverage exclusion for the CLI history-compact
command: in vitest.config.ts locate the coverage.include and coverage.exclude
arrays and delete the string "src/cli/commands/history-compact.ts" from the
coverage.exclude array so the include entry remains effective; ensure no other
duplicate entries remain in coverage.exclude that also appear in
coverage.include.
---
Nitpick comments:
In `@tests/unit/history-compact-command.spec.ts`:
- Around line 69-147: Add a new unit test that exercises the timestamp parsing
branch of runHistoryCompact by creating an item (createItem), adding at least
two subsequent history entries (e.g., update/append), then call runHistory(full:
true) to read the entries' created_at timestamps and choose an ISO timestamp
between entries as the --before value; invoke runHistoryCompact(id, { before:
"<ISO-timestamp>", author: "test-author" }, { path: context.pmPath }) and assert
the result reports compact_boundary.kind === "timestamp", correct
entries_compacted/entries_retained/first_retained_entry values, changed===true
(or unchanged for a noop case), and that history verification and history file
state match the other tests' expectations (use existing helpers: withTempPmPath,
runHistoryCompact, runHistory, getHistoryPath, readFile). Ensure the test name
clearly states it validates ISO timestamp boundary parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e55fd1a1-870a-4543-b976-079dc51409cc
📒 Files selected for processing (23)
.agents/pm/chores/pm-idnz.toon.agents/pm/features/pm-3pbq.toon.agents/pm/history/pm-3pbq.jsonl.agents/pm/history/pm-idnz.jsonldocs/COMMANDS.mdsrc/cli/commands/completion.tssrc/cli/commands/contracts.tssrc/cli/commands/history-compact.tssrc/cli/commands/index.tssrc/cli/help-content.tssrc/cli/register-mutation.tssrc/core/item/toon-decode.tssrc/mcp/server.tssrc/sdk/cli-contracts.tssrc/sdk/cli-contracts/enum-contracts.tssrc/sdk/cli-contracts/tool-parameter-tables.tstests/fixtures/contracts/full.jsontests/integration/release-readiness-runtime.spec.tstests/unit/completion-command.spec.tstests/unit/contracts-command.spec.tstests/unit/history-compact-command.spec.tstests/unit/toon-decode.spec.tsvitest.config.ts
This addresses PR review findings by enforcing contiguous timestamp compaction semantics and accurate retained-tail entry indexing, then expands integration coverage to exercise defensive/error branches while keeping static quality gates green.
|
@codex review |
|
/gemini review |
|
@gemini-code-assist review |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Code Review
This pull request introduces the history-compact command, enabling checkpoint-based history stream compaction into a synthetic baseline and a retained tail. The implementation includes the core command logic, CLI registration, shell completions, MCP routing, contract definitions, documentation, and extensive test coverage. It also adds explicit upstream tracking metadata and tests for the TOON scalar bracket workaround. No review comments were provided, so I have no feedback to offer.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
This records final acceptance evidence by marking pm-3pbq and pm-idnz closed and released after green checks, review-loop fixes, and full verification gates.
|
@codex review |
|
/gemini review |
|
@gemini-code-assist review |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Code Review
This pull request implements the history-compact command, which compacts long history streams into a synthetic baseline plus a retained tail while preserving replay integrity. The changes include the core command implementation, CLI registration, help content, shell completions, MCP server routing, and contract schema updates. Additionally, explicit upstream tracking metadata and tests have been added for the @toon-format/toon bracket-bug workaround. Comprehensive unit and integration tests are introduced to verify the compaction behavior, boundary conditions, and rollback safety. There are no review comments to address, and no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request implements the history-compact command, allowing users to compact long-lived item history streams into a synthetic baseline entry and a retained tail while preserving replay integrity. The command is fully integrated across the CLI, shell completions, MCP routing, and contract schemas, and is backed by comprehensive integration tests. Additionally, the PR adds explicit upstream tracking metadata and tests for the @toon-format/toon scalar bracket workaround. No review comments were provided for this pull request, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
This refreshes CHANGELOG.md after closing and releasing pm-3pbq and pm-idnz so static release gates remain in sync with tracker state.
|
@codex review |
|
/gemini review |
|
@gemini-code-assist review |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Code Review
This pull request introduces the history-compact command, which compacts long history streams into a synthetic checkpoint baseline and a retained tail while preserving replay integrity. The command is fully integrated into the CLI registration, shell completions, MCP routing, and contracts. Additionally, explicit upstream tracking metadata and tests have been added for the TOON scalar bracket workaround in toon-decode.ts. Comprehensive unit and integration tests have been added to verify the compaction behavior and ensure 100% test coverage. No review comments were provided, so there is no additional feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request implements the pm history-compact command to support checkpoint-based history stream compaction, allowing long mutation streams to be compacted into a synthetic baseline with a retained tail. Additionally, it introduces explicit upstream tracking metadata for the TOON bracket-bug workaround in toon-decode.ts and updates the CLI registration, completions, contracts, documentation, and test suites accordingly. As there are no review comments provided for this pull request, I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request introduces the pm history-compact command, which compacts long history streams into a synthetic baseline checkpoint plus a retained tail, preserving replay integrity. The implementation includes CLI registration, help documentation, shell completion support, MCP routing, contract definitions, and extensive integration and unit tests. Additionally, explicit upstream tracking metadata and tests have been added for the TOON scalar bracket workaround. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
pm history-compact <id>with boundary parsing (--beforeversion/timestamp), synthetic checkpoint baseline generation, retained-tail hash re-anchoring, integrity verification, rollback-safe atomic writes, and audit marker creationtoon-decodeplus guard tests, and record end-to-end evidence onpm-3pbq+pm-idnzTest plan
pnpm buildpnpm typechecknode scripts/run-tests.mjs coveragepnpm contracts:checkpnpm changelog:pm:checkpnpm security:scanpm validate --check-resolution --check-history-driftpm health --check-onlynode scripts/run-tests.mjs test -- tests/unit/toon-decode.spec.tsnpm pack-> install tarball ->pm init-> create/update/history/history-compact(dry+apply)/restore/history --verify