feat(harness-node): context-compaction v2 + console /compact UX - #163
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (73)
📝 WalkthroughWalkthroughAdds UI slash-command for compaction, system markers, token usage chip, history translation, backend compact/session APIs, worker async/sync compaction with prune/template/selection/leases, session-tree ops, orchestrator preflight, model catalog context windows, and extensive tests/fixtures/docs. ChangesContext Compaction End-to-End
Sequence Diagram(s)sequenceDiagram
rect rgba(66, 135, 245, 0.5)
participant User
participant ChatView
end
rect rgba(46, 204, 113, 0.5)
participant WebBackend as realBackend
end
rect rgba(155, 89, 182, 0.5)
participant Worker as Context-Compaction
participant Tree as Session-Tree
participant LLM as Provider
end
User->>ChatView: Type "/compact"
ChatView->>WebBackend: compactSession(sessionId, model, history)
WebBackend->>Worker: context-compaction::compact_session
Worker->>Tree: acquire lease, load messages
Worker->>LLM: summarize(head) stream
LLM-->>Worker: summary_text (done)
Worker->>Tree: compact(summary, tail_start_id), append synthetic
Worker-->>WebBackend: {status, summary_text, tokens_before}
WebBackend-->>ChatView: CompactResult
ChatView-->>User: Replace convo with compaction marker
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
skill-check — worker0 verified, 10 skipped (no docs/).
Three for three. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
console/web/src/components/chat/ChatView.tsx (1)
129-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
/compactbefore appending a user turn.Right now the
/compacttext is appended as a normal user message before the local-command branch. On non-okpaths (unsupported/busy/overflow/error), that command stays in history and can be sent to the model on subsequent turns, which contradicts the “never sent to the model” intent.Proposed fix
- /* user turn */ - const userMsg: UserMessage = { - id: uid(), - role: 'user', - content: payload.text, - attachments: - payload.attachments.length > 0 ? payload.attachments : undefined, - createdAt: Date.now(), - } - onAppendMessage(conversationId, userMsg) - /* Slash commands: handled locally, never sent to the model. */ const trimmed = payload.text.trim() if (trimmed === '/compact' || trimmed.startsWith('/compact ')) { if (!backend.compactSession) { onAppendMessage( conversationId, makeSystemNotice( '/compact not supported by this backend.', 'error', ), ) return } ... return } + + /* user turn */ + const userMsg: UserMessage = { + id: uid(), + role: 'user', + content: payload.text, + attachments: + payload.attachments.length > 0 ? payload.attachments : undefined, + createdAt: Date.now(), + } + onAppendMessage(conversationId, userMsg)Also applies to: 188-207
🤖 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 `@console/web/src/components/chat/ChatView.tsx` around lines 129 - 152, The code appends the user message (userMsg) before handling the local "/compact" slash command, causing the command to be persisted and potentially sent to the model on error; update ChatView.tsx so the trimmed payload.text is checked for "/compact" (and similar local commands) before creating/appending userMsg and only append a user message for non-local commands or after the local command has been handled (use backend.compactSession, onAppendMessage, makeSystemNotice, and conversationId to locate logic), and ensure on the non-ok paths (unsupported/busy/overflow/error) you append only the system notice and return without adding userMsg.
🧹 Nitpick comments (1)
console/web/src/hooks/use-conversations.ts (1)
57-62: ⚡ Quick winNarrow
compactConversationinput type to a system compaction marker.The API currently accepts any
Message, but this method semantically requires a compaction/system marker. Tightening the type prevents accidental misuse that could replace a conversation with a user/assistant message.Proposed fix
+import type { SystemMessage } from '`@/types/chat`' ... - compactConversation: (id: string, marker: Message) => void + compactConversation: (id: string, marker: SystemMessage) => void ... - const compactConversation = useCallback( - (id: string, marker: Message) => + const compactConversation = useCallback( + (id: string, marker: SystemMessage) => patchConversation(id, (c) => ({ ...c, messages: [marker], updatedAt: Date.now(), })),Also applies to: 221-223
🤖 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 `@console/web/src/hooks/use-conversations.ts` around lines 57 - 62, The compactConversation signature currently accepts any Message but should be restricted to a system compaction marker to avoid replacing conversations with user/assistant content; change the compactConversation parameter type from Message to the specific compaction/system marker type used in your types (e.g., SystemCompactionMarker or SystemMessage with the compaction flag), and update all call sites (including the other occurrences referenced around lines 221-223) to pass that narrowed type; ensure related types/imports are updated so the function, its callers, and any tests compile with the tighter type.
🤖 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 `@harness-node/docs/workers/context-compaction.md`:
- Around line 33-35: Add explicit language tags to the Markdown fenced code
blocks that are currently untagged: mark the block containing "{ groupId |
group_id: string, event: { data: EventObj } | data: EventObj }" as json, the
block with "usable = max(0, model.input_limit − COMPACT_RESERVED_TOKENS)" as
text (or ts if you prefer), and the block containing the "## Goal / ##
Constraints / ..." headers as md; apply the same fix to the other untagged
fences noted around lines 130-132 and 145-154 so all three types of fences
include appropriate language identifiers.
In `@harness-node/src/context-compaction/model-resolver.ts`:
- Around line 88-94: The current loop in model-resolver.ts latches providerID
and modelID independently from different assistant messages (using variables
providerID and modelID), which can pair a provider from one message with a model
from another; instead, iterate messages from newest to oldest and for each
assistant message check that both m.provider and m.model are non-empty strings,
and only then set providerID and modelID together from that single m and break;
update the loop that references messages, m, providerID and modelID so the pair
comes from the same assistant message.
In `@harness-node/src/context-compaction/replay.ts`:
- Around line 27-37: The current call to iii.trigger for 'session-tree::append'
silently falls back to parent_id when resp?.entry_id is missing, dropping the
replay without an error; update the code around the iii.trigger(...) call (the
block that currently returns resp?.entry_id ?? parent_id) to treat a missing
resp.entry_id as a write failure by validating resp?.entry_id after the call and
throwing a descriptive Error (include context such as function_id, session_id
and parent_id or replay.message) instead of returning parent_id so failures are
surfaced to callers.
In `@harness-node/src/context-compaction/summarize.ts`:
- Around line 117-129: The current call to iii.trigger for function_id
'session-tree::compact' returns resp?.entry_id ?? '' which allows an empty
compaction_entry_id; instead detect missing resp or entry_id and fail-fast by
throwing a clear error (include session_id and indication this came from the
'session-tree::compact' response) rather than returning ''. Update the code path
that calls iii.trigger (the resp variable and its return) so it validates
resp.entry_id and throws on absence, and apply the same fix to the other
occurrence handling compaction responses (the block around lines 206-220) so no
code path yields an empty string compaction_entry_id.
In `@harness-node/src/session/tree/operations.ts`:
- Around line 360-371: The code currently sets parent_id = opts.parent_id ??
null which creates root entries when parent_id is omitted; change the default to
the session's active leaf id instead of null (e.g., parent_id = opts.parent_id
?? <currentActiveLeafId>), where <currentActiveLeafId> should be obtained from
the module's active-path/state accessor (for example a function like
getActiveLeafId() or a property like sessionState.activeLeafId), and use that
parent_id when constructing the SessionEntry so synthetic appends attach to the
active leaf rather than creating a new root.
In `@harness-node/src/session/tree/store.ts`:
- Around line 170-179: In updateEntry, avoid unconditionally calling state::set
which creates missing entries and can desync IDs; first read the existing entry
via this.iii.trigger with function_id 'state::get' and scope
entriesScope(session_id) and key entry_id, return immediately (no-op) if not
found, then set updated.id = entry_id (normalize) before calling
this.iii.trigger with 'state::set' to persist; reference the updateEntry method,
entriesScope(session_id), and the entry_id/updated.id fields when making these
changes.
In `@harness-node/tests/context-compaction/e2e/full-session.test.ts`:
- Around line 354-355: The test is restoring process.env.COMPACT_RESERVED_TOKENS
by assigning undefined which sets the literal string "undefined" in Node;
instead, when prev is undefined remove the env var with delete
process.env.COMPACT_RESERVED_TOKENS, otherwise restore it by assigning
prev—change the branch that currently does `process.env.COMPACT_RESERVED_TOKENS
= undefined` to use `delete process.env.COMPACT_RESERVED_TOKENS` while keeping
the existing restore path that assigns prev.
In `@harness-node/tests/context-compaction/handler-sync.test.ts`:
- Around line 103-114: The test sets process.env.COMPACT_BUSY_TIMEOUT_MS to '1'
then attempts to restore it incorrectly by assigning undefined; change the test
around handleSync so it saves const original =
process.env.COMPACT_BUSY_TIMEOUT_MS before mutating, and in the finally block
restore the original value if it was defined or delete
process.env.COMPACT_BUSY_TIMEOUT_MS to properly unset it; update the test that
calls handleSync (handler-sync.test.ts) to use this save-and-restore pattern to
avoid leaking "undefined" as a string across tests.
In `@harness-node/tests/context-compaction/integration/flow-prune.test.ts`:
- Around line 88-113: The test currently only checks that protected shell::run
entries are not in updatePartCalls, which can pass if prune() did nothing;
modify the test around buildPruneMock(entries) / prune(iii, ...) to also assert
that pruning actually occurred by verifying updatePartCalls contains at least
one entry (e.g., expect(updatePartCalls.length).toBeGreaterThan(0)) or that
prunedIds is non-empty, and/or assert that some non-protected entry IDs were
pruned; keep the existing protected-tool assertions for shellRunEntries as-is so
you prove pruning happened while still protecting those tools.
In `@harness-node/tests/context-compaction/prune.test.ts`:
- Around line 83-94: The test currently only asserts the protected tool 't1' was
not updated which still passes if nothing was pruned; add assertions to prove
pruning actually occurred by checking that updates is non-empty and that the
unprotected tool entry 't2' was pruned/updated. Specifically, in the 'skips
parts in protectedTools' test (where entries, makeIii, prune, and updates are
used), add assertions like expect(updates.length).toBeGreaterThan(0) and
expect(updates.some(u => u.entry_id === 't2')).toBe(true) so the test verifies
at least one prune update happened and that the unprotected tool was affected
while 't1' remains untouched.
In `@harness-node/tests/session/tree-compactions.test.ts`:
- Around line 26-36: The test is flaky because compactionEntries sorts only by
timestamp and ties can produce nondeterministic order; update compactionEntries
to break ties deterministically by comparing a secondary key (e.g.,
compaction.id) after timestamp so entries are sorted by (timestamp asc, id asc).
Locate the compactionEntries implementation and modify its sort comparator to
first compare timestamp and, when equal, compare id (string/number) to ensure
stable deterministic ordering for id1/id2 in the test.
---
Outside diff comments:
In `@console/web/src/components/chat/ChatView.tsx`:
- Around line 129-152: The code appends the user message (userMsg) before
handling the local "/compact" slash command, causing the command to be persisted
and potentially sent to the model on error; update ChatView.tsx so the trimmed
payload.text is checked for "/compact" (and similar local commands) before
creating/appending userMsg and only append a user message for non-local commands
or after the local command has been handled (use backend.compactSession,
onAppendMessage, makeSystemNotice, and conversationId to locate logic), and
ensure on the non-ok paths (unsupported/busy/overflow/error) you append only the
system notice and return without adding userMsg.
---
Nitpick comments:
In `@console/web/src/hooks/use-conversations.ts`:
- Around line 57-62: The compactConversation signature currently accepts any
Message but should be restricted to a system compaction marker to avoid
replacing conversations with user/assistant content; change the
compactConversation parameter type from Message to the specific
compaction/system marker type used in your types (e.g., SystemCompactionMarker
or SystemMessage with the compaction flag), and update all call sites (including
the other occurrences referenced around lines 221-223) to pass that narrowed
type; ensure related types/imports are updated so the function, its callers, and
any tests compile with the tighter type.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a3842de-c4a7-47e0-9e34-c91e5e8896ab
📒 Files selected for processing (73)
console/web/src/components/chat/ChatView.tsxconsole/web/src/components/chat/ContextUsage.tsxconsole/web/src/components/chat/LexicalShell.tsxconsole/web/src/components/chat/Message.tsxconsole/web/src/components/chat/lexical/SlashCommandsPlugin.tsxconsole/web/src/hooks/use-conversations.tsconsole/web/src/lib/backend/history.test.tsconsole/web/src/lib/backend/history.tsconsole/web/src/lib/backend/mock.tsconsole/web/src/lib/backend/real.tsconsole/web/src/lib/backend/types.tsconsole/web/src/lib/functions.tsconsole/web/src/lib/models-catalog.tsconsole/web/src/lib/slash-commands.tsconsole/web/src/lib/token-estimate.test.tsconsole/web/src/lib/token-estimate.tsconsole/web/src/pages/Chat.tsxconsole/web/src/pages/Playground/index.tsxconsole/web/src/types/chat.tsharness-node/docs/workers/context-compaction.mdharness-node/src/context-compaction/config.tsharness-node/src/context-compaction/flat-state.tsharness-node/src/context-compaction/handler-async.tsharness-node/src/context-compaction/handler-sync.tsharness-node/src/context-compaction/handler.tsharness-node/src/context-compaction/lease.tsharness-node/src/context-compaction/model-resolver.tsharness-node/src/context-compaction/overflow.tsharness-node/src/context-compaction/prune.tsharness-node/src/context-compaction/register.tsharness-node/src/context-compaction/replay.tsharness-node/src/context-compaction/selection.tsharness-node/src/context-compaction/stream-collect.tsharness-node/src/context-compaction/strip-media.tsharness-node/src/context-compaction/summarize.tsharness-node/src/context-compaction/template.tsharness-node/src/context-compaction/threshold.tsharness-node/src/session/tree/operations.tsharness-node/src/session/tree/register.tsharness-node/src/session/tree/store.tsharness-node/src/session/tree/types.tsharness-node/src/turn-orchestrator/errors.tsharness-node/src/turn-orchestrator/estimate.tsharness-node/src/turn-orchestrator/preflight.tsharness-node/src/turn-orchestrator/states/assistant.tsharness-node/tests/context-compaction/compact-session-registered.test.tsharness-node/tests/context-compaction/compact-session.test.tsharness-node/tests/context-compaction/e2e/full-session.test.tsharness-node/tests/context-compaction/flat-state-key.test.tsharness-node/tests/context-compaction/handler-async.test.tsharness-node/tests/context-compaction/handler-sync.test.tsharness-node/tests/context-compaction/integration/backward-compat.test.tsharness-node/tests/context-compaction/integration/flow-async.test.tsharness-node/tests/context-compaction/integration/flow-prune.test.tsharness-node/tests/context-compaction/integration/flow-sync.test.tsharness-node/tests/context-compaction/lease.test.tsharness-node/tests/context-compaction/overflow.test.tsharness-node/tests/context-compaction/prune.test.tsharness-node/tests/context-compaction/replay.test.tsharness-node/tests/context-compaction/selection.test.tsharness-node/tests/context-compaction/strip-media.test.tsharness-node/tests/context-compaction/summarize.test.tsharness-node/tests/context-compaction/template.test.tsharness-node/tests/fixtures/load.tsharness-node/tests/fixtures/sessions/large-with-media.jsonharness-node/tests/fixtures/sessions/medium-with-tools.jsonharness-node/tests/fixtures/sessions/tiny.jsonharness-node/tests/session/tree-append-synthetic.test.tsharness-node/tests/session/tree-compact-tail-start.test.tsharness-node/tests/session/tree-compactions.test.tsharness-node/tests/session/tree-update-part.test.tsharness-node/tests/turn-orchestrator/estimate.test.tsharness-node/tests/turn-orchestrator/preflight.test.ts
💤 Files with no reviewable changes (2)
- harness-node/src/context-compaction/threshold.ts
- harness-node/src/context-compaction/handler.ts
Comment on lines
+33
to
+35
| ``` | ||
| { groupId | group_id: string, event: { data: EventObj } | data: EventObj } | ||
| ``` |
There was a problem hiding this comment.
Add language tags to fenced code blocks to satisfy MD040.
These new fences should specify a language (json, text, or ts) to keep markdown lint clean.
Suggested patch
-```
+```json
{ groupId | group_id: string, event: { data: EventObj } | data: EventObj }- +text
usable = max(0, model.input_limit − COMPACT_RESERVED_TOKENS)
-```
+```md
## Goal
## Constraints
## Progress
## Key Decisions
## Tool Calls Made
## Next Steps
## Critical Context
## Relevant Files
</details>
Also applies to: 130-132, 145-154
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @harness-node/docs/workers/context-compaction.md around lines 33 - 35, Add
explicit language tags to the Markdown fenced code blocks that are currently
untagged: mark the block containing "{ groupId | group_id: string, event: {
data: EventObj } | data: EventObj }" as json, the block with "usable = max(0,
model.input_limit − COMPACT_RESERVED_TOKENS)" as text (or ts if you prefer), and
the block containing the "## Goal / ## Constraints / ..." headers as md; apply
the same fix to the other untagged fences noted around lines 130-132 and 145-154
so all three types of fences include appropriate language identifiers.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+88
to
+94
| for (let i = messages.length - 1; i >= 0; i--) { | ||
| const m = messages[i]?.message; | ||
| if (!m || m.role !== 'assistant') continue; | ||
| if (!providerID && typeof m.provider === 'string' && m.provider) providerID = m.provider; | ||
| if (!modelID && typeof m.model === 'string' && m.model) modelID = m.model; | ||
| if (providerID && modelID) break; | ||
| } |
There was a problem hiding this comment.
Avoid pairing provider and model across different assistant messages.
Line 91 and Line 92 latch fields independently, so providerID and modelID can come from different messages. That can resolve the wrong model limit and skew compaction/overflow behavior.
🔧 Proposed fix
- let providerID: string | null = null;
- let modelID: string | null = null;
-
for (let i = messages.length - 1; i >= 0; i--) {
const m = messages[i]?.message;
if (!m || m.role !== 'assistant') continue;
- if (!providerID && typeof m.provider === 'string' && m.provider) providerID = m.provider;
- if (!modelID && typeof m.model === 'string' && m.model) modelID = m.model;
- if (providerID && modelID) break;
+ if (
+ typeof m.provider === 'string' &&
+ m.provider &&
+ typeof m.model === 'string' &&
+ m.model
+ ) {
+ return fetchModelLimit(iii, m.provider, m.model);
+ }
}
-
- if (!providerID || !modelID) return null;
-
- return fetchModelLimit(iii, providerID, modelID);
+ return null;🤖 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 `@harness-node/src/context-compaction/model-resolver.ts` around lines 88 - 94,
The current loop in model-resolver.ts latches providerID and modelID
independently from different assistant messages (using variables providerID and
modelID), which can pair a provider from one message with a model from another;
instead, iterate messages from newest to oldest and for each assistant message
check that both m.provider and m.model are non-empty strings, and only then set
providerID and modelID together from that single m and break; update the loop
that references messages, m, providerID and modelID so the pair comes from the
same assistant message.
Comment on lines
+27
to
+37
| const resp = await iii.trigger<unknown, { entry_id?: string }>({ | ||
| function_id: 'session-tree::append', | ||
| payload: { | ||
| session_id, | ||
| parent_id, | ||
| message: replay.message, | ||
| }, | ||
| timeoutMs: 10_000, | ||
| }); | ||
| return resp?.entry_id ?? parent_id; | ||
| } |
There was a problem hiding this comment.
Fail fast when replay append returns no entry_id.
Line 37 silently falls back to parent_id, which can drop the replay message without surfacing an error. This should be treated as a write failure.
🔧 Proposed fix
const resp = await iii.trigger<unknown, { entry_id?: string }>({
@@
- return resp?.entry_id ?? parent_id;
+ if (!resp?.entry_id) {
+ throw new Error('context-compaction::reinjectReplay: session-tree::append returned no entry_id');
+ }
+ return resp.entry_id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const resp = await iii.trigger<unknown, { entry_id?: string }>({ | |
| function_id: 'session-tree::append', | |
| payload: { | |
| session_id, | |
| parent_id, | |
| message: replay.message, | |
| }, | |
| timeoutMs: 10_000, | |
| }); | |
| return resp?.entry_id ?? parent_id; | |
| } | |
| const resp = await iii.trigger<unknown, { entry_id?: string }>({ | |
| function_id: 'session-tree::append', | |
| payload: { | |
| session_id, | |
| parent_id, | |
| message: replay.message, | |
| }, | |
| timeoutMs: 10_000, | |
| }); | |
| if (!resp?.entry_id) { | |
| throw new Error('context-compaction::reinjectReplay: session-tree::append returned no entry_id'); | |
| } | |
| return resp.entry_id; | |
| } |
🤖 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 `@harness-node/src/context-compaction/replay.ts` around lines 27 - 37, The
current call to iii.trigger for 'session-tree::append' silently falls back to
parent_id when resp?.entry_id is missing, dropping the replay without an error;
update the code around the iii.trigger(...) call (the block that currently
returns resp?.entry_id ?? parent_id) to treat a missing resp.entry_id as a write
failure by validating resp?.entry_id after the call and throwing a descriptive
Error (include context such as function_id, session_id and parent_id or
replay.message) instead of returning parent_id so failures are surfaced to
callers.
Comment on lines
+117
to
129
| const resp = await iii.trigger<unknown, { entry_id?: string }>({ | ||
| function_id: 'session-tree::compact', | ||
| payload: { | ||
| session_id, | ||
| summary, | ||
| tokens_before, | ||
| tail_start_id, | ||
| details: { read_files: [], modified_files: [] }, | ||
| }, | ||
| timeoutMs: 10_000, | ||
| }); | ||
| await stampLastCompaction(iii, session_id); | ||
| return resp?.entry_id ?? ''; | ||
| } |
There was a problem hiding this comment.
Fail fast when compaction append returns no entry_id.
At Line 128, returning '' allows kind: 'ok' with an invalid compaction_entry_id, which can break parent chaining in sync compaction flows.
💡 Proposed fix
async function appendCompaction(
iii: ISdk,
session_id: string,
summary: string,
tokens_before: number,
tail_start_id: string | null,
): Promise<string> {
const resp = await iii.trigger<unknown, { entry_id?: string }>({
@@
},
timeoutMs: 10_000,
});
- return resp?.entry_id ?? '';
+ const entry_id = resp?.entry_id;
+ if (!entry_id) throw new Error('session-tree::compact returned empty entry_id');
+ return entry_id;
}Also applies to: 206-220
🤖 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 `@harness-node/src/context-compaction/summarize.ts` around lines 117 - 129, The
current call to iii.trigger for function_id 'session-tree::compact' returns
resp?.entry_id ?? '' which allows an empty compaction_entry_id; instead detect
missing resp or entry_id and fail-fast by throwing a clear error (include
session_id and indication this came from the 'session-tree::compact' response)
rather than returning ''. Update the code path that calls iii.trigger (the resp
variable and its return) so it validates resp.entry_id and throws on absence,
and apply the same fix to the other occurrence handling compaction responses
(the block around lines 206-220) so no code path yields an empty string
compaction_entry_id.
Comment on lines
+360
to
+371
| const parent_id = opts.parent_id ?? null; | ||
| const entry: SessionEntry = { | ||
| type: 'message', | ||
| id, | ||
| parent_id, | ||
| message: { | ||
| role: 'user', | ||
| content: [{ type: 'text', text: opts.text }], | ||
| timestamp: Date.now(), | ||
| }, | ||
| timestamp: Date.now(), | ||
| }; |
There was a problem hiding this comment.
Use active leaf as default parent for synthetic appends.
At Line 360, defaulting to null can create a new root entry and effectively reset the active path if parent_id is omitted.
💡 Proposed fix
export async function appendSynthetic(
store: SessionStore,
session_id: string,
@@
): Promise<string> {
const id = randomUUID();
- const parent_id = opts.parent_id ?? null;
+ const parent_id =
+ opts.parent_id ?? (await activePath(store, session_id)).at(-1) ?? null;
const entry: SessionEntry = {
type: 'message',
id,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const parent_id = opts.parent_id ?? null; | |
| const entry: SessionEntry = { | |
| type: 'message', | |
| id, | |
| parent_id, | |
| message: { | |
| role: 'user', | |
| content: [{ type: 'text', text: opts.text }], | |
| timestamp: Date.now(), | |
| }, | |
| timestamp: Date.now(), | |
| }; | |
| const parent_id = | |
| opts.parent_id ?? (await activePath(store, session_id)).at(-1) ?? null; | |
| const entry: SessionEntry = { | |
| type: 'message', | |
| id, | |
| parent_id, | |
| message: { | |
| role: 'user', | |
| content: [{ type: 'text', text: opts.text }], | |
| timestamp: Date.now(), | |
| }, | |
| timestamp: Date.now(), | |
| }; |
🤖 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 `@harness-node/src/session/tree/operations.ts` around lines 360 - 371, The code
currently sets parent_id = opts.parent_id ?? null which creates root entries
when parent_id is omitted; change the default to the session's active leaf id
instead of null (e.g., parent_id = opts.parent_id ?? <currentActiveLeafId>),
where <currentActiveLeafId> should be obtained from the module's
active-path/state accessor (for example a function like getActiveLeafId() or a
property like sessionState.activeLeafId), and use that parent_id when
constructing the SessionEntry so synthetic appends attach to the active leaf
rather than creating a new root.
Comment on lines
+354
to
+355
| if (prev === undefined) process.env.COMPACT_RESERVED_TOKENS = undefined; | ||
| else process.env.COMPACT_RESERVED_TOKENS = prev; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "full-session.test.ts" -type fRepository: iii-hq/workers
Length of output: 123
🏁 Script executed:
sed -n '340,365p' ./harness-node/tests/context-compaction/e2e/full-session.test.tsRepository: iii-hq/workers
Length of output: 1035
🌐 Web query:
Node.js process.env undefined assignment semantics behavior
💡 Result:
In Node.js, assigning undefined to a key on process.env does not remove the key. Instead, the value is implicitly converted to a string, so process.env.foo = undefined results in process.env.foo === 'undefined'.[1] What the spec/docs say 1) process.env is an object of environment variables that can be modified.[1] 2) When you assign a property on process.env, Node implicitly converts the assigned value to a string (this implicit conversion behavior is deprecated and future versions may throw if the value is not a string/number/boolean).[1][2] 3) Therefore, env.test = undefined yields the string 'undefined' when read back.[1] How to truly “unset” / remove the variable entry - Use delete process.env.<name>; the docs explicitly recommend this for removing a property from process.env.[1] Example - env.test = undefined; console.log(env.test); // 'undefined'.[1] - delete env.TEST; console.log(env.TEST); // undefined.[1] Related nuance - process.env is per-process; modifying it affects the current Node process, but (unless explicitly requested) won’t be reflected in other Worker threads.[1][2]
Citations:
Unset COMPACT_RESERVED_TOKENS with delete, not = undefined.
In Node.js, assigning undefined to process.env results in the string value 'undefined' being stored, which leaks state across tests. Use delete instead to properly remove the variable.
Suggested fix
- if (prev === undefined) process.env.COMPACT_RESERVED_TOKENS = undefined;
+ if (prev === undefined) delete process.env.COMPACT_RESERVED_TOKENS;
else process.env.COMPACT_RESERVED_TOKENS = prev;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (prev === undefined) process.env.COMPACT_RESERVED_TOKENS = undefined; | |
| else process.env.COMPACT_RESERVED_TOKENS = prev; | |
| if (prev === undefined) delete process.env.COMPACT_RESERVED_TOKENS; | |
| else process.env.COMPACT_RESERVED_TOKENS = prev; |
🤖 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 `@harness-node/tests/context-compaction/e2e/full-session.test.ts` around lines
354 - 355, The test is restoring process.env.COMPACT_RESERVED_TOKENS by
assigning undefined which sets the literal string "undefined" in Node; instead,
when prev is undefined remove the env var with delete
process.env.COMPACT_RESERVED_TOKENS, otherwise restore it by assigning
prev—change the branch that currently does `process.env.COMPACT_RESERVED_TOKENS
= undefined` to use `delete process.env.COMPACT_RESERVED_TOKENS` while keeping
the existing restore path that assigns prev.
Comment on lines
+103
to
+114
| process.env.COMPACT_BUSY_TIMEOUT_MS = '1'; | ||
| try { | ||
| const result = await handleSync(iii, { | ||
| session_id: 'test-session-busy', | ||
| projected_tokens: 50_000, | ||
| last_user_message_id: 'msg-1', | ||
| model: defaultModel, | ||
| }); | ||
| expect(result.status).toBe('busy'); | ||
| } finally { | ||
| process.env.COMPACT_BUSY_TIMEOUT_MS = undefined; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js, does assigning process.env.MY_VAR = undefined delete the variable or set it to the string "undefined"? What is the recommended cleanup pattern in tests?
💡 Result:
Assigning process.env.MY_VAR = undefined does not “delete” the variable. Node implicitly converts assigned values on process.env to strings, so the value becomes the string "undefined" [1]. The recommended cleanup pattern is to delete the property from process.env (and in tests, restore any original env values afterward). Node’s docs explicitly say: “Use delete to delete a property from process.env” [1]. For example: const original = process.env.MY_VAR; process.env.MY_VAR = undefined; // results in string "undefined" [1] // cleanup: delete process.env.MY_VAR; if (original === undefined) delete process.env.MY_VAR; else process.env.MY_VAR = original; This “delete to unset” behavior is also discussed in practice: using delete process.env.MYVAR makes process.env.MYVAR become undefined (and removes the key) [2].
Citations:
- 1: https://nodejs.org/api/process.html
- 2: https://stackoverflow.com/questions/42170365/how-do-i-remove-a-value-in-process-env/42170366
🏁 Script executed:
# Check if the test file exists and verify lines 103-114
if [ -f "harness-node/tests/context-compaction/handler-sync.test.ts" ]; then
echo "=== File found. Showing lines 103-114: ==="
sed -n '103,114p' "harness-node/tests/context-compaction/handler-sync.test.ts"
else
echo "=== File not found at expected path. Searching for similar files: ==="
find . -name "handler-sync.test.ts" 2>/dev/null
fiRepository: iii-hq/workers
Length of output: 475
Restore COMPACT_BUSY_TIMEOUT_MS safely after the test.
Line 113 assigns process.env.COMPACT_BUSY_TIMEOUT_MS = undefined, which does not delete the environment variable. Instead, it sets it to the string "undefined", potentially leaking state across tests. Save the original value and use delete to properly unset it in finally.
Suggested fix
- process.env.COMPACT_BUSY_TIMEOUT_MS = '1';
+ const original = process.env.COMPACT_BUSY_TIMEOUT_MS;
+ process.env.COMPACT_BUSY_TIMEOUT_MS = '1';
try {
const result = await handleSync(iii, {
session_id: 'test-session-busy',
projected_tokens: 50_000,
last_user_message_id: 'msg-1',
model: defaultModel,
});
expect(result.status).toBe('busy');
} finally {
- process.env.COMPACT_BUSY_TIMEOUT_MS = undefined;
+ if (original === undefined) {
+ delete process.env.COMPACT_BUSY_TIMEOUT_MS;
+ } else {
+ process.env.COMPACT_BUSY_TIMEOUT_MS = original;
+ }
}🤖 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 `@harness-node/tests/context-compaction/handler-sync.test.ts` around lines 103
- 114, The test sets process.env.COMPACT_BUSY_TIMEOUT_MS to '1' then attempts to
restore it incorrectly by assigning undefined; change the test around handleSync
so it saves const original = process.env.COMPACT_BUSY_TIMEOUT_MS before
mutating, and in the finally block restore the original value if it was defined
or delete process.env.COMPACT_BUSY_TIMEOUT_MS to properly unset it; update the
test that calls handleSync (handler-sync.test.ts) to use this save-and-restore
pattern to avoid leaking "undefined" as a string across tests.
Comment on lines
+88
to
+113
| it('does not prune protected tools', async () => { | ||
| const entries = largeFixture.entries.map((e) => ({ | ||
| entry_id: e.id, | ||
| message: e.message as AgentMessage, | ||
| })); | ||
|
|
||
| // Find the entry IDs of shell::run function results (which exist in the fixture) | ||
| const shellRunEntries = entries.filter( | ||
| (e) => | ||
| e.message.role === 'function_result' && | ||
| (e.message as { function_id?: string }).function_id === 'shell::run', | ||
| ); | ||
|
|
||
| const { iii, updatePartCalls } = buildPruneMock(entries); | ||
|
|
||
| await prune(iii, largeFixture.session_id, { | ||
| protectTokens: 0, | ||
| minFree: PRUNE_MIN_FREE, | ||
| protectedTools: ['shell::run'], | ||
| }); | ||
|
|
||
| // None of the protected shell::run entries should appear in update_part calls | ||
| const prunedIds = new Set(updatePartCalls.map((c) => c.entry_id)); | ||
| for (const e of shellRunEntries) { | ||
| expect(prunedIds.has(e.entry_id)).toBe(false); | ||
| } |
There was a problem hiding this comment.
Strengthen the protected-tools test to avoid false positives.
This case only asserts “protected entries were not updated”; it can still pass if pruning regresses to a no-op. Add an assertion that pruning did occur.
Suggested fix
- await prune(iii, largeFixture.session_id, {
+ const result = await prune(iii, largeFixture.session_id, {
protectTokens: 0,
minFree: PRUNE_MIN_FREE,
protectedTools: ['shell::run'],
});
+ expect(result.pruned_parts).toBeGreaterThan(0);
+ expect(updatePartCalls.length).toBeGreaterThan(0);
// None of the protected shell::run entries should appear in update_part calls
const prunedIds = new Set(updatePartCalls.map((c) => c.entry_id));🤖 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 `@harness-node/tests/context-compaction/integration/flow-prune.test.ts` around
lines 88 - 113, The test currently only checks that protected shell::run entries
are not in updatePartCalls, which can pass if prune() did nothing; modify the
test around buildPruneMock(entries) / prune(iii, ...) to also assert that
pruning actually occurred by verifying updatePartCalls contains at least one
entry (e.g., expect(updatePartCalls.length).toBeGreaterThan(0)) or that
prunedIds is non-empty, and/or assert that some non-protected entry IDs were
pruned; keep the existing protected-tool assertions for shellRunEntries as-is so
you prove pruning happened while still protecting those tools.
Comment on lines
+83
to
+94
| it('skips parts in protectedTools', async () => { | ||
| const big = 'x'.repeat(200_000); | ||
| const entries: Array<{ entry_id: string; message: AgentMessage }> = []; | ||
| for (let i = 0; i < 3; i++) { | ||
| entries.push({ entry_id: `u${i}`, message: user() }); | ||
| } | ||
| entries.push({ entry_id: 't1', message: tool('skill::ask', big) }); | ||
| entries.push({ entry_id: 't2', message: tool('shell::fs::cat', big) }); | ||
| const { iii, updates } = makeIii(entries); | ||
| await prune(iii, 'sid', { protectTokens: 0, minFree: 1000, protectedTools: ['skill::ask'] }); | ||
| expect(updates.some((u) => u.entry_id === 't1')).toBe(false); | ||
| }); |
There was a problem hiding this comment.
Make the protected-tools test prove pruning still happens.
Current assertions pass even if no entries are pruned. Add checks that at least one update happened and that the unprotected tool entry is pruned.
Suggested fix
- await prune(iii, 'sid', { protectTokens: 0, minFree: 1000, protectedTools: ['skill::ask'] });
+ await prune(iii, 'sid', { protectTokens: 0, minFree: 1000, protectedTools: ['skill::ask'] });
+ expect(updates.length).toBeGreaterThan(0);
expect(updates.some((u) => u.entry_id === 't1')).toBe(false);
+ expect(updates.some((u) => u.entry_id === 't2')).toBe(true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('skips parts in protectedTools', async () => { | |
| const big = 'x'.repeat(200_000); | |
| const entries: Array<{ entry_id: string; message: AgentMessage }> = []; | |
| for (let i = 0; i < 3; i++) { | |
| entries.push({ entry_id: `u${i}`, message: user() }); | |
| } | |
| entries.push({ entry_id: 't1', message: tool('skill::ask', big) }); | |
| entries.push({ entry_id: 't2', message: tool('shell::fs::cat', big) }); | |
| const { iii, updates } = makeIii(entries); | |
| await prune(iii, 'sid', { protectTokens: 0, minFree: 1000, protectedTools: ['skill::ask'] }); | |
| expect(updates.some((u) => u.entry_id === 't1')).toBe(false); | |
| }); | |
| it('skips parts in protectedTools', async () => { | |
| const big = 'x'.repeat(200_000); | |
| const entries: Array<{ entry_id: string; message: AgentMessage }> = []; | |
| for (let i = 0; i < 3; i++) { | |
| entries.push({ entry_id: `u${i}`, message: user() }); | |
| } | |
| entries.push({ entry_id: 't1', message: tool('skill::ask', big) }); | |
| entries.push({ entry_id: 't2', message: tool('shell::fs::cat', big) }); | |
| const { iii, updates } = makeIii(entries); | |
| await prune(iii, 'sid', { protectTokens: 0, minFree: 1000, protectedTools: ['skill::ask'] }); | |
| expect(updates.length).toBeGreaterThan(0); | |
| expect(updates.some((u) => u.entry_id === 't1')).toBe(false); | |
| expect(updates.some((u) => u.entry_id === 't2')).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 `@harness-node/tests/context-compaction/prune.test.ts` around lines 83 - 94,
The test currently only asserts the protected tool 't1' was not updated which
still passes if nothing was pruned; add assertions to prove pruning actually
occurred by checking that updates is non-empty and that the unprotected tool
entry 't2' was pruned/updated. Specifically, in the 'skips parts in
protectedTools' test (where entries, makeIii, prune, and updates are used), add
assertions like expect(updates.length).toBeGreaterThan(0) and
expect(updates.some(u => u.entry_id === 't2')).toBe(true) so the test verifies
at least one prune update happened and that the unprotected tool was affected
while 't1' remains untouched.
Comment on lines
+26
to
+36
| it('returns all compaction entries sorted by timestamp ascending', async () => { | ||
| const store = new InMemoryStore(); | ||
| const sid = await createSession(store); | ||
| const e1 = await appendMessage(store, sid, null, userMsg('a')); | ||
| const id1 = await compact(store, sid, 'first compaction', { read_files: ['a.ts'] }, e1, 200); | ||
| const id2 = await compact(store, sid, 'second compaction', {}, null, 400); | ||
| const result = await compactionEntries(store, sid); | ||
| expect(result).toHaveLength(2); | ||
| expect(result[0]?.id).toBe(id1); | ||
| expect(result[1]?.id).toBe(id2); | ||
| }); |
There was a problem hiding this comment.
Make the ordering test deterministic when timestamps tie.
Line 26-36 relies on id1 then id2 ordering via timestamp sort, but both compact calls can share the same millisecond timestamp and make this assertion flaky.
Suggested fix
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
it('returns all compaction entries sorted by timestamp ascending', async () => {
const store = new InMemoryStore();
const sid = await createSession(store);
const e1 = await appendMessage(store, sid, null, userMsg('a'));
- const id1 = await compact(store, sid, 'first compaction', { read_files: ['a.ts'] }, e1, 200);
- const id2 = await compact(store, sid, 'second compaction', {}, null, 400);
+ const nowSpy = vi.spyOn(Date, 'now');
+ nowSpy.mockReturnValueOnce(1_700_000_000_000);
+ const id1 = await compact(store, sid, 'first compaction', { read_files: ['a.ts'] }, e1, 200);
+ nowSpy.mockReturnValueOnce(1_700_000_000_001);
+ const id2 = await compact(store, sid, 'second compaction', {}, null, 400);
+ nowSpy.mockRestore();
+
const result = await compactionEntries(store, sid);
expect(result).toHaveLength(2);
expect(result[0]?.id).toBe(id1);
expect(result[1]?.id).toBe(id2);
});🤖 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 `@harness-node/tests/session/tree-compactions.test.ts` around lines 26 - 36,
The test is flaky because compactionEntries sorts only by timestamp and ties can
produce nondeterministic order; update compactionEntries to break ties
deterministically by comparing a secondary key (e.g., compaction.id) after
timestamp so entries are sorted by (timestamp asc, id asc). Locate the
compactionEntries implementation and modify its sort comparator to first compare
timestamp and, when equal, compare id (string/number) to ensure stable
deterministic ordering for id1/id2 in the test.
Out-of-band session-history compactor with async + sync + prune + UI
entry points. Keeps sessions alive indefinitely by summarising older
turns into a structured Compaction entry; the next turn reads only the
summary plus a configurable tail of recent turns.
Compaction worker (harness-node/src/context-compaction/):
- on_agent_event — async TurnEnd-driven; cheap if not overflowing
- compact_now — sync pre-turn path called by turn-orchestrator
- prune_tool_outputs — cheap prune-only path (no summarisation)
- compact_session — UI-initiated /compact wrapper (resolves model + last
user message internally; forces unconditional summary)
Implementation modules:
- overflow.ts — model-adaptive usable() / isOverflow() / preserveRecentBudget()
- selection.ts — turns / splitTurn / select / selectWithEntryIds / completedCompactions
- template.ts — eight-section structured summary template + anchored update prompt
- summarize.ts — load → select tail → strip media → summarise → append
- prune.ts — walk function_result entries newest→oldest, null outputs above protect window
- strip-media.ts — strip images, truncate oversized tool outputs before summariser
- replay.ts — extract user message before summarisation, reinject after
- lease.ts — nonce-and-readback single-writer lease (compaction + prune kinds), with-wait variant
- model-resolver.ts — shared catalog lookup (fetchModelLimit, resolveModelFromSession, resolveModelFromRunRequest)
- config.ts — env-driven knobs (COMPACT_*); deprecates COMPACT_TRIGGER_TOKENS
- flat-state.ts — rewrites session/{sid}/messages after compaction so
turn-orchestrator's flat-state mirror stays compact
- stream-collect.ts — channel.reader.stream.resume() (matches assistant.ts)
- handler-async.ts / handler-sync.ts — entry points wired to register.ts
Turn-orchestrator (harness-node/src/turn-orchestrator/):
- preflight.ts — pre-flight projected-overflow check; triggers compact_now;
surfaces ContextOverflowError / CompactionBusyError
- estimate.ts — chars/4 token estimator for pre-flight
- errors.ts — typed errors for context-overflow / compaction-busy
Session-tree (harness-node/src/session/tree/):
- compact endpoint now records tail_start_id (last kept entry boundary)
- compactions endpoint — list all Compaction entries for prior-summary anchor
- append_synthetic endpoint — synthetic user prompt after sync compaction
- update_part / update_parts endpoints — null out pruned tool outputs in-place
(batch form for prune's single-shot call)
- store.updateEntry — refreshes meta.updated_at on every part mutation
Observability:
- compaction.async / compaction.sync / compaction.prune OTel spans
- iii.session.id baggage inheritance via instrumentHandler
Console UI (console/web/):
- @-mention picker lists compact_session, prune_tool_outputs,
session-tree::compactions
- New /-slash picker (SlashCommandsPlugin) sibling of @mentions;
triggers on `/` at column 0; lists slash commands like /compact
- /compact slash command end-to-end:
- compact_session returns summary_text on the wire
- ChatView replaces shed messages with a CompactionMarker carrying
the summary text + tokensBefore; rendered as a centered divider
with a <details> expansion of the captured summary
- translateUiHistoryForBackend ships <conversation-summary>{text}
in place of the shed turns so the next run::start does NOT
overwrite the orchestrator's compacted flat-state back to the
pre-compact transcript (Option A bypass fix)
- useConversations.compactConversation primitive for the wholesale
message-list replacement
- ContextUsage chip in the chat header:
- char/4 token estimator (matches harness-node pre-flight)
- progress bar with normal / warn (75%) / danger (90%) bands
- reads contextWindow from models-catalog (now surfaces the field)
- degrades to a plain count when the model has no known window
- ModelOption gains contextWindow; STATIC_MODEL_OPTIONS backfilled
- SystemMessage gains kind?: 'notice' | 'compaction' + summaryText +
tokensBefore (used by the marker variant)
Tests:
- unit: lease, overflow, prune, replay, selection, strip-media, summarize, template, handler-async, handler-sync, compact-session, compact-session-registered, flat-state-key (drift guard for flatMessagesKey ↔ messagesKey)
- integration: flow-async, flow-sync, flow-prune, backward-compat
- session-tree: tree-append-synthetic, tree-compact-tail-start, tree-compactions, tree-update-part, tree-update-parts
- turn-orchestrator: estimate, preflight
- e2e: full-session (live registerTree + InMemoryStore)
- console/web: history (round-trip + compaction-marker shedding + multi-marker last-wins), token-estimate (estimator + formatter + marker counting)
- fixtures: tiny.json, medium-with-tools.json, large-with-media.json
Backward compat: pre-v2 free-form summaries are used as previousSummary
anchor regardless of format, so they get re-templated on next cycle.
real.ts CompactResult mapper accepts both `message` and `reason` shapes
for the overflow variant during the field rename rollout.
Docs: harness-node/docs/workers/context-compaction.md documents the four
endpoints, model-adaptive threshold math, summarisation template, env
config, state keys, OTel spans, and source layout.
andersonleal
force-pushed
the
feat/harness-node-context-compaction-v2
branch
from
May 19, 2026 19:51
a61da83 to
9f6802f
Compare
sergiofilhowz
approved these changes
May 19, 2026
This was referenced May 19, 2026
Merged
5 tasks
Merged
5 tasks
Merged
This was referenced Jul 3, 2026
6 tasks
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
2 participants
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.
Summary
Out-of-band context compaction worker (async + sync + prune + UI-initiated) plus the console UX that consumes it. Sessions stay alive indefinitely by summarising older turns into a structured Compaction entry; subsequent turns read only the summary plus a configurable tail.
Backend —
harness-node/src/context-compaction/on_agent_event(async, TurnEnd-driven),compact_now(sync pre-turn from orchestrator),prune_tool_outputs(cheap path, no summarisation),compact_session(UI/compact, resolves model + last-user internally).overflow.ts,selection.ts,template.ts,summarize.ts,prune.ts,strip-media.ts,replay.ts,lease.ts(nonce-and-readback single-writer lease),model-resolver.ts,flat-state.ts(rewritessession/{sid}/messagesso the orchestrator's flat-state mirror stays compact),stream-collect.ts.preflight.tsprojected-overflow check + chars/4 estimator + typedContextOverflowError/CompactionBusyError.tail_start_idoncompact, newcompactions/append_synthetic/update_part/update_partsendpoints.compaction.async/compaction.sync/compaction.prune, withiii.session.idbaggage.Console —
console/web//compactslash command, end-to-end:compact_sessionreturnssummary_texton the wire.ChatViewreplaces the shed messages with aCompactionMarkercarrying the summary text + tokensBefore. Rendered as a centered divider with a<details>expansion of the captured summary.translateUiHistoryForBackendships<conversation-summary>{text}</conversation-summary>in place of the shed turns — fixes the Option-A bypass whererun::startwould otherwise overwrite the orchestrator's compacted flat-state back to the pre-compact transcript./-slash typeahead picker (SlashCommandsPlugin), sibling of@mentions; triggers on/at column 0; lists/compact.contextWindowfrom the models catalog (now surfaced throughfetchModelsCatalogandModelOption).SystemMessagegainskind?: 'notice' \| 'compaction'+summaryText+tokensBefore.useConversations.compactConversationprimitive for the wholesale message-list replacement.Backward compat
previousSummaryanchor; they get re-templated on the next cycle.real.tsCompactResultmapper accepts bothmessageandreasonfor the overflow variant during the rollout.Test plan
bun run test), typecheck clean, lint cleanflatMessagesKey === messagesKey)registerTree+InMemoryStore)bun run test), typecheck clean, lint cleanhistory(round-trip + compaction-marker shedding + multi-marker last-wins + missing-summaryText fallback)token-estimate(estimator + formatter + marker counting)/compactagainst a real session — confirm CTX chart drops,<details>shows captured summary, next turn does NOT regrow context back to pre-compact size/in composer — confirm the slash picker pops with/compactSummary by CodeRabbit
New Features
/compactto compress conversation history and manage context limits.Improvements