fix(compaction): drop label/branch_summary entries referencing removed messages

Addresses Codex review feedback: label and branch_summary entries whose
parentId points to a removed message are now also dropped during
truncation to avoid dangling metadata. This is consistent with the
approach in tool-result-truncation.ts.
This commit is contained in:
Thirumalesh
2026-03-17 10:39:44 +05:30
committed by Josh Lehman
parent 579d732341
commit f3e1f21a28
2 changed files with 70 additions and 4 deletions

View File

@@ -229,6 +229,55 @@ describe("truncateSessionAfterCompaction", () => {
expect(ctx.thinkingLevel).toBe("high");
});
it("drops label and branch_summary entries that reference removed messages", async () => {
const dir = await createTmpDir();
const sm = SessionManager.create(dir, dir);
// Messages before compaction
sm.appendMessage({ role: "user", content: "hello", timestamp: 1 });
sm.appendMessage(makeAssistant("hi", 2));
sm.appendMessage({ role: "user", content: "do task", timestamp: 3 });
sm.appendMessage(makeAssistant("done", 4));
// Get a pre-compaction message ID to attach label to
const branch = sm.getBranch();
const preCompactionMsgId = branch[1].id; // "hi" message
// Compaction summarizing the conversation
const firstKeptId = branch[branch.length - 1].id;
sm.appendCompaction("Summary.", firstKeptId, 5000);
// Post-compaction messages
sm.appendMessage({ role: "user", content: "next", timestamp: 5 });
const sessionFile = sm.getSessionFile()!;
// Inject a label entry referencing a pre-compaction message into the
// raw JSONL before the compaction line (SessionManager has no appendLabel).
const lines = (await fs.readFile(sessionFile, "utf-8")).trimEnd().split("\n");
const compactionLineIdx = lines.findIndex((l) => l.includes('"compaction"'));
const labelLine = JSON.stringify({
type: "label",
id: "label-001",
parentId: preCompactionMsgId,
timestamp: 2500,
targetId: preCompactionMsgId,
label: "my-label",
});
lines.splice(compactionLineIdx, 0, labelLine);
await fs.writeFile(sessionFile, lines.join("\n") + "\n", "utf-8");
const result = await truncateSessionAfterCompaction({ sessionFile });
expect(result.truncated).toBe(true);
// Verify label entry was dropped (its parent message was removed)
const smAfter = SessionManager.open(sessionFile);
const allAfter = smAfter.getEntries();
const labels = allAfter.filter((e) => e.type === "label");
expect(labels).toHaveLength(0);
});
it("preserves unsummarized sibling branches during truncation", async () => {
const dir = await createTmpDir();
const sm = SessionManager.create(dir, dir);

View File

@@ -16,7 +16,9 @@ import { log } from "./logger.js";
* This function rewrites the file keeping:
* 1. The session header
* 2. All non-message session state (custom, model_change, thinking_level_change,
* session_info, label, branch_summary, custom_message, compaction entries)
* session_info, custom_message, compaction entries)
* Note: label and branch_summary entries referencing removed messages are
* also dropped to avoid dangling metadata.
* 3. All entries from sibling branches not covered by the compaction
* 4. The compaction entry and all entries after it in the current branch
*
@@ -81,9 +83,13 @@ export async function truncateSessionAfterCompaction(params: {
const allEntries = sm.getEntries();
// Only remove message-type entries that the compaction actually summarized.
// Non-message entries (custom, model_change, thinking_level_change,
// session_info, label, branch_summary, custom_message) are preserved even
// if they sit in the summarized portion of the branch.
// Non-message session state (custom, model_change, thinking_level_change,
// session_info, custom_message) is preserved even if it sits in the
// summarized portion of the branch.
//
// label and branch_summary entries that reference removed message IDs are
// also dropped to avoid dangling metadata (consistent with the approach in
// tool-result-truncation.ts).
const removedIds = new Set<string>();
for (const entry of allEntries) {
if (summarizedBranchIds.has(entry.id) && entry.type === "message") {
@@ -91,6 +97,17 @@ export async function truncateSessionAfterCompaction(params: {
}
}
// Drop label/branch_summary entries whose parent points to a removed message
for (const entry of allEntries) {
if (
(entry.type === "label" || entry.type === "branch_summary") &&
entry.parentId !== null &&
removedIds.has(entry.parentId)
) {
removedIds.add(entry.id);
}
}
if (removedIds.size === 0) {
return { truncated: false, entriesRemoved: 0, reason: "no entries to remove" };
}