feat: add structured session compaction API#671
Open
shaunak99 wants to merge 7 commits intotruffle-ai:mainfrom
Open
feat: add structured session compaction API#671shaunak99 wants to merge 7 commits intotruffle-ai:mainfrom
shaunak99 wants to merge 7 commits intotruffle-ai:mainfrom
Conversation
|
@shaunak99 is attempting to deploy a commit to the Shaunak's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a persisted session compaction subsystem: new agent APIs ( Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as DextoAgent
participant SM as SessionManager
participant CompSvc as CompactionService
participant Strategy as CompactionStrategy
participant Persist as Persistence
participant Events as EventBus
Agent->>CompSvc: runSessionCompaction({sessionId, mode, trigger, ...})
CompSvc->>SM: getSessionHistory(sessionId)
SM-->>CompSvc: history[]
CompSvc->>Events: emit context:compacting({estimatedTokens})
CompSvc->>Strategy: compact(compactionWindow)
Strategy-->>CompSvc: CompactionResult | null
alt no summary
CompSvc->>Events: emit context:compacted({reason, originalTokens, compactedTokens, ...})
CompSvc-->>Agent: null
else has summary
alt mode = continue-in-child
CompSvc->>SM: createSeededChildSession(parent, {initialMessages, title?})
SM-->>CompSvc: childSession{id}
CompSvc->>Persist: saveSessionCompaction(record with targetSessionId)
else mode = continue-in-place
CompSvc->>SM: appendMessages(sessionId, summaryMessages)
CompSvc->>Persist: saveSessionCompaction(record)
else mode = artifact-only
CompSvc->>Persist: saveSessionCompaction(record)
end
CompSvc->>Events: emit context:compacted({compactionId, mode, targetSessionId?, ...})
CompSvc-->>Agent: SessionCompactionRecord
end
sequenceDiagram
participant Client as Client
participant Router as Sessions Router
participant Agent as DextoAgent
participant Persist as Persistence
Client->>Router: POST /sessions/{id}/compact {mode, childTitle?}
Router->>Agent: compactSession({sessionId, mode, trigger: 'api', childTitle?})
Agent->>Persist: saveSessionCompaction(record)
Agent-->>Router: { compaction: mappedRecord | null }
Router-->>Client: 200 {compaction: ...}
Client->>Router: GET /sessions/compactions/{compactionId}
Router->>Agent: getSessionCompaction(compactionId)
Agent->>Persist: getSessionCompaction(compactionId)
Persist-->>Agent: record | undefined
Agent-->>Router: record | undefined
Router-->>Client: 200 {compaction: ...} | 404
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
# Conflicts: # packages/server/src/hono/routes/sessions.ts # packages/server/src/hono/schemas/responses.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
packages/server/src/hono/middleware/error.ts (1)
132-134: Consider adding a comment explaining the magic string origin.The hardcoded string
'Malformed JSON in request body'could break silently if Hono's validator middleware changes its error message. Consider adding a brief comment noting where this message originates.📝 Suggested documentation
+/** + * Detects malformed JSON errors thrown by Hono's JSON body validator middleware, + * which uses this exact error message instead of a SyntaxError. + */ function isMalformedJsonRequestError(err: unknown): err is Error { return err instanceof Error && err.message === 'Malformed JSON in request body'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/hono/middleware/error.ts` around lines 132 - 134, The function isMalformedJsonRequestError currently matches the Hono validator error by the literal string 'Malformed JSON in request body'; add a brief comment directly above isMalformedJsonRequestError explaining that this exact message originates from Hono's validator middleware (mention the middleware name/version or file if known) and that the string is relied upon intentionally so future maintainers know the dependency and risk if Hono changes its message.packages/server/src/hono/schemas/responses.ts (1)
338-344: Prefer the shared core compaction enums here.These literals already exist in core and drive runtime validation in
DextoAgent.compactSession(). Re-declaring them in the server schema creates a silent drift point between OpenAPI and the actual API surface if a mode/trigger is added later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/hono/schemas/responses.ts` around lines 338 - 344, Replace the local literal enums with the shared core enums: import the compaction enums exported from core (the enum types used by DextoAgent.compactSession) and use z.nativeEnum(<CoreSessionCompactionModeEnum>) for SessionCompactionModeSchema and z.nativeEnum(<CoreSessionCompactionTriggerEnum>) for SessionCompactionTriggerSchema so the OpenAPI schema is driven from the same runtime types as DextoAgent.compactSession; keep the existing .describe text but remove the hard-coded string arrays and ensure the imported enum symbol names match the core exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/static/openapi/openapi.json`:
- Around line 10595-10597: The GET /api/sessions/compactions/{compactionId}
route's 404 response is missing a response schema; update the route handler
definition so the 404 response includes the StandardErrorEnvelopeSchema (i.e.,
add a content: {'application/json': { schema: StandardErrorEnvelopeSchema }}
block to the 404 response for the GET compaction route referencing the
compactionId path), then regenerate the OpenAPI spec with pnpm run
sync-openapi-docs to update docs (ensure the same schema shape used by POST
/api/sessions/{sessionId}/compact is applied).
In `@packages/core/src/agent/DextoAgent.ts`:
- Around line 1877-1887: In compactSession ensure you validate the input/options
object itself before reading input.sessionId: at the start of the compactSession
method (in DextoAgent.compactSession) add a guard that input is not
null/undefined and is an object (and throw AgentError.apiValidationError with
the same message if it fails) so that calling compactSession(null) or
compactSession(undefined) yields the API validation error rather than a raw
TypeError; then keep the existing sessionId/type check and rest of logic
unchanged.
In `@packages/core/src/session/chat-session.ts`:
- Around line 304-306: SessionManager is being passed where a
SessionCompactionPersistence is expected but it lacks the required
createSeededChildSession method; implement createSeededChildSession on the
SessionManager class to satisfy the SessionCompactionPersistence interface. Add
a method named createSeededChildSession with the same signature as the
interface, implementing logic to create a new child session seeded from the
provided session data (reusing existing session creation utilities or delegating
to createSession if appropriate), and ensure it returns the created session
object; keep existing deleteSession and saveSessionCompaction implementations
intact so SessionManager fully implements SessionCompactionPersistence.
In `@packages/core/src/session/compaction-service.ts`:
- Around line 113-117: Remove the hard length-based early return that uses
input.contextManager.getHistory() and instead let the compaction strategy make
the decision; do not skip compaction purely because history.length < 4. Replace
the check with a fast-path that queries token pressure from the context manager
(e.g., a token count/usage method on input.contextManager) and only skip when
token usage is well under threshold, otherwise always pass the history into the
strategy's compact(...) decision. Keep the existing debug log using
input.logger.debug and include input.sessionId and token metrics when you
short-circuit.
- Around line 151-155: The continuation path currently appends the entire
summaryMessages array into history and relies on filterCompacted() (which only
supports one summary boundary) to produce the preserved suffix, causing earlier
summaries to be mixed into the preserved tail and reorder the continuation. Fix
by building continuationMessages explicitly: first compute the preserved suffix
from history alone (call filterCompacted([...structuredClone(history)]) and
normalize those messages), then append the entire normalized summaryMessages
array in order (map normalizeCompactionMessage over
rawSummaryMessages/summaryMessages) to form continuationMessages; alternatively,
if you prefer stricter behavior, detect if summaryMessages.length > 1 and
throw/handle an error to enforce a single boundary. Ensure you update references
to continuationMessages, summaryMessages, rawSummaryMessages, filterCompacted,
normalizeCompactionMessage, and history accordingly.
In `@packages/core/src/session/session-manager.ts`:
- Around line 342-353: deleteSession currently removes only session:* and
messages:* but leaves compacted data saved by saveSessionCompaction
(session-compaction:*), allowing deleted conversations to be recovered; update
the deleteSession method to also remove compaction entries by deleting keys
prefixed with SessionManager.SESSION_COMPACTION_KEY_PREFIX for the session
(e.g., construct and delete
`${SessionManager.SESSION_COMPACTION_KEY_PREFIX}${sessionId}` or iterate/delete
all matching compaction keys via the storageManager database API), ensuring you
reference deleteSession, saveSessionCompaction, getSessionCompaction, and
SESSION_COMPACTION_KEY_PREFIX when making the change.
In `@packages/server/src/hono/routes/sessions.ts`:
- Around line 21-32: The CompactSessionSchema currently allows childTitle
regardless of mode; add a cross-field validation using z.superRefine on
CompactSessionSchema to reject childTitle unless mode === 'continue-in-child'
(use SessionCompactionModeSchema values to compare), and when invalid call
ctx.addIssue targeting the 'childTitle' path with a clear error message so
requests that supply childTitle for other modes fail validation; keep the
existing .strict() and descriptions intact.
- Around line 255-283: The route getCompactionRoute currently documents 404 with
no body but the handler returns a bespoke { error: 'Compaction not found' }
payload; update the route's 404 response schema to use
StandardErrorEnvelopeSchema (so the OpenAPI contract documents the standard
error shape) and change the handler that returns { error: 'Compaction not found'
} to instead produce/throw the standard middleware error envelope (i.e., return
or throw the same shape validated by StandardErrorEnvelopeSchema) so the
response matches the published contract for SessionCompactionSchema endpoints.
- Around line 57-71: mapSessionCompaction currently just casts
compaction.summaryMessages and continuationMessages to the API schema which
doesn't serialize binary/URL types; replace those naive casts by normalizing
each InternalMessage into the JSON-safe shape expected by InternalMessageSchema
(transform Buffer/Uint8Array to base64 or string, call URL.toString(), and
convert any other binary unions accordingly) before returning; in practice
remove the "as" casts in mapSessionCompaction and map each message in
compaction.summaryMessages and compaction.continuationMessages through a new or
existing serializer (e.g., normalizeInternalMessage or serializeInternalMessage)
so the returned object matches z.output<typeof InternalMessageSchema> for
multimodal sessions.
---
Nitpick comments:
In `@packages/server/src/hono/middleware/error.ts`:
- Around line 132-134: The function isMalformedJsonRequestError currently
matches the Hono validator error by the literal string 'Malformed JSON in
request body'; add a brief comment directly above isMalformedJsonRequestError
explaining that this exact message originates from Hono's validator middleware
(mention the middleware name/version or file if known) and that the string is
relied upon intentionally so future maintainers know the dependency and risk if
Hono changes its message.
In `@packages/server/src/hono/schemas/responses.ts`:
- Around line 338-344: Replace the local literal enums with the shared core
enums: import the compaction enums exported from core (the enum types used by
DextoAgent.compactSession) and use z.nativeEnum(<CoreSessionCompactionModeEnum>)
for SessionCompactionModeSchema and
z.nativeEnum(<CoreSessionCompactionTriggerEnum>) for
SessionCompactionTriggerSchema so the OpenAPI schema is driven from the same
runtime types as DextoAgent.compactSession; keep the existing .describe text but
remove the hard-coded string arrays and ensure the imported enum symbol names
match the core exports.
🪄 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: 5a6500b3-9e6a-4253-aa2a-0df3f96c8bd9
📒 Files selected for processing (20)
docs/docs/tutorials/sdk/sessions.mddocs/static/openapi/openapi.jsonpackages/core/src/agent/DextoAgent.session-compaction.integration.test.tspackages/core/src/agent/DextoAgent.tspackages/core/src/events/index.tspackages/core/src/llm/executor/turn-executor.integration.test.tspackages/core/src/llm/executor/turn-executor.tspackages/core/src/llm/services/factory.tspackages/core/src/llm/services/vercel.tspackages/core/src/session/chat-session.test.tspackages/core/src/session/chat-session.tspackages/core/src/session/compaction-service.tspackages/core/src/session/compaction.tspackages/core/src/session/index.tspackages/core/src/session/session-manager.tspackages/server/src/hono/__tests__/api.integration.test.tspackages/server/src/hono/__tests__/test-fixtures.tspackages/server/src/hono/middleware/error.tspackages/server/src/hono/routes/sessions.tspackages/server/src/hono/schemas/responses.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/agent/DextoAgent.ts (1)
1898-1904:⚠️ Potential issue | 🟡 MinorValidate
inputbefore dereferencing it.Line 1903 still reads
input.sessionIdbeforeinputitself is checked, socompactSession(null as any)/compactSession(undefined as any)throws a rawTypeErrorinstead of the normal API validation error path.Read-only verification
This confirms whether
compactSession(...)dereferencesinputbefore an object guard exists.Expected result: the method body shows
const sessionId = input.sessionId;with no precedingif (!input || typeof input !== 'object')guard.#!/bin/bash sed -n '1898,1910p' packages/core/src/agent/DextoAgent.ts rg -n -C2 "const sessionId = input\\.sessionId|if \\(!input \\|\\| typeof input !== 'object'\\)" packages/core/src/agent/DextoAgent.tsSuggested fix
public async compactSession( input: SessionCompactionInput ): Promise<SessionCompactionRecord | null> { this.ensureStarted(); + if (!input || typeof input !== 'object') { + throw AgentError.apiValidationError( + 'input is required and must be an object' + ); + } + const sessionId = input.sessionId; if (!sessionId || typeof sessionId !== 'string') { throw AgentError.apiValidationError( 'sessionId is required and must be a non-empty string' );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/agent/DextoAgent.ts` around lines 1898 - 1904, The method compactSession in class DextoAgent dereferences input via const sessionId = input.sessionId before checking input; add an upfront guard that validates input is present and an object (e.g. if (!input || typeof input !== 'object') throw the same API validation error used elsewhere) and only then read const sessionId = (input as SessionCompactionInput).sessionId; ensure you reference the SessionCompactionInput type and keep the existing validation/error path and subsequent checks for sessionId being a string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/agent/DextoAgent.ts`:
- Around line 1898-1904: The method compactSession in class DextoAgent
dereferences input via const sessionId = input.sessionId before checking input;
add an upfront guard that validates input is present and an object (e.g. if
(!input || typeof input !== 'object') throw the same API validation error used
elsewhere) and only then read const sessionId = (input as
SessionCompactionInput).sessionId; ensure you reference the
SessionCompactionInput type and keep the existing validation/error path and
subsequent checks for sessionId being a string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10a52118-af4c-4bec-8699-1661a7d226b7
📒 Files selected for processing (8)
docs/static/openapi/openapi.jsonpackages/core/src/agent/DextoAgent.tspackages/core/src/events/index.tspackages/core/src/session/chat-session.tspackages/core/src/session/session-manager.tspackages/server/src/hono/__tests__/api.integration.test.tspackages/server/src/hono/routes/sessions.tspackages/server/src/hono/schemas/responses.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/src/session/chat-session.ts
- packages/server/src/hono/routes/sessions.ts
- packages/core/src/events/index.ts
- packages/server/src/hono/schemas/responses.ts
- packages/core/src/session/session-manager.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/session/compaction-service.ts (1)
412-458: Use a compaction error factory instead of inlineDextoRuntimeErrors.This new core module is constructing module-local runtime errors inline in multiple places. A small
SessionCompactionErrorfactory would keep codes/messages/context centralized with the rest ofpackages/core.As per coding guidelines, "packages/core/src/**: Use module-specific error factory pattern for new modules (reference examples: packages/core/src/config/errors.ts, packages/core/src/logger/v2/errors.ts, packages/core/src/storage/errors.ts, packages/core/src/telemetry/errors.ts)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/session/compaction-service.ts` around lines 412 - 458, Replace the inline DextoRuntimeError constructions in compaction logic with a module-specific error factory (e.g., export a SessionCompactionError factory) and use it in place of the three current DextoRuntimeError calls; create a new factory that builds/returns DextoRuntimeError instances with centralized codes/messages/context matching this file's cases (invalid_compaction_output, strategy context, originalMessageCount, etc.), import and call SessionCompactionError where the code currently throws DextoRuntimeError (the checks around summaryMessages length, summaryMessage.metadata flags, and rawOriginalMessageCount) so all compaction errors follow the shared packages/core error-factory pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/agent/DextoAgent.ts`:
- Around line 1921-1925: Add a runtime type check for input.childTitle when mode
=== 'continue-in-child' so non-string values are rejected before creating a
child session: if mode is 'continue-in-child' and input.childTitle is not
undefined, verify typeof input.childTitle === 'string' and throw
AgentError.apiValidationError with a clear message if it fails; keep the
existing check that disallows childTitle in other modes (the block using
input.childTitle and mode !== 'continue-in-child') and place this new guard next
to the other input validations in DextoAgent.ts so the invalid value cannot flow
into the child-session creation logic.
In `@packages/core/src/session/compaction-service.ts`:
- Around line 212-223: The compaction record is persisted before appending the
summary messages, so if input.contextManager.addMessage() fails the durable
artifact exists but wasn't applied; to fix, for the 'continue-in-place' branch
move the await input.persistence.saveSessionCompaction(compaction) until after
the loop that calls
input.contextManager.addMessage(cloneCompactionMessage(summary)), or if ordering
cannot change wrap the addMessage loop in try/catch and on failure perform a
compensating deletion via
input.persistence.deleteSessionCompaction(compaction.id) (or the project’s
corresponding delete/remove method) and rethrow; also ensure you still call
input.contextManager.resetActualTokenTracking() only after successful append and
persistence.
- Around line 115-158: The compaction result currently assumes the strategy's
returned summary messages map directly to the raw history index, forcing every
CompactionStrategy to know about archived turns; fix by changing the compaction
contract so compactionStrategy.compact returns both the summary messages and an
explicit boundary index (e.g., summaryBoundaryIndex) pointing into the history
(or metadata mapping), then use that returned boundary to compute
originalMessageCount/continuationMessages instead of inferring via
resolveSummaryBoundary; update callers that call compactionStrategy.compact and
the usage sites around rawSummaryMessages, originalMessageCount,
resolveSummaryBoundary, and continuationMessages to consume the returned
boundary index and slice history starting at that index.
---
Nitpick comments:
In `@packages/core/src/session/compaction-service.ts`:
- Around line 412-458: Replace the inline DextoRuntimeError constructions in
compaction logic with a module-specific error factory (e.g., export a
SessionCompactionError factory) and use it in place of the three current
DextoRuntimeError calls; create a new factory that builds/returns
DextoRuntimeError instances with centralized codes/messages/context matching
this file's cases (invalid_compaction_output, strategy context,
originalMessageCount, etc.), import and call SessionCompactionError where the
code currently throws DextoRuntimeError (the checks around summaryMessages
length, summaryMessage.metadata flags, and rawOriginalMessageCount) so all
compaction errors follow the shared packages/core error-factory pattern.
🪄 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: 5df62910-4d3b-4023-bdad-a17601e9102c
📒 Files selected for processing (8)
docs/static/openapi/openapi.jsonpackages/core/src/agent/DextoAgent.session-compaction.integration.test.tspackages/core/src/agent/DextoAgent.tspackages/core/src/session/compaction-service.tspackages/core/src/session/session-manager.test.tspackages/core/src/session/session-manager.tspackages/server/src/hono/__tests__/api.integration.test.tspackages/server/src/hono/routes/sessions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/server/src/hono/routes/sessions.ts
- packages/core/src/session/session-manager.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/core/src/context/compaction/strategies/reactive-overflow-compaction.ts (1)
154-159:⚠️ Potential issue | 🟠 MajorDon't skip re-compaction solely on
freshHistory.length <= 4.This is still a message-count gate in a token-driven path. A couple of large fresh tool/file messages can overflow the model window here, and returning
nullleaves the caller with no recovery path even though there may still be a summarizable working-history prefix.Suggested change
- const freshHistory = window.freshHistory; ... - if (window.latestSummary && freshHistory.length <= 4) { - logger.debug( - `ReactiveOverflowCompactionStrategy: Only ${freshHistory.length} fresh message(s) after latest summary, skipping re-compaction` - ); - return null; - } - // Split working history into messages to summarize and messages to keep const { toSummarize, toKeep } = this.splitHistory(workingHistory); + if (window.latestSummary && toSummarize.length === 0) { + logger.debug( + 'ReactiveOverflowCompactionStrategy: No working-history prefix is eligible for re-compaction' + ); + return null; + }
🧹 Nitpick comments (1)
packages/server/src/hono/schemas/responses.ts (1)
429-435: Use core compaction constants instead of re-declaring enum literals.Re-declaring literals here can drift from the canonical values in
packages/core/src/session/compaction.ts. ReuseSESSION_COMPACTION_MODESandSESSION_COMPACTION_TRIGGERSdirectly—this pattern is already established in the same file withLLM_PROVIDERSandLLM_PRICING_STATUSES.♻️ Proposed refactor
import { z } from 'zod'; import { LLM_PRICING_STATUSES, LLMConfigBaseSchema as CoreLLMConfigBaseSchema, LLM_PROVIDERS, + SESSION_COMPACTION_MODES, + SESSION_COMPACTION_TRIGGERS, } from '@dexto/core'; @@ export const SessionCompactionModeSchema = z - .enum(['artifact-only', 'continue-in-place', 'continue-in-child']) + .enum(SESSION_COMPACTION_MODES) .describe('How the compaction artifact should be applied'); @@ export const SessionCompactionTriggerSchema = z - .enum(['manual', 'api', 'scheduled', 'overflow']) + .enum(SESSION_COMPACTION_TRIGGERS) .describe('Why the compaction was triggered');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/hono/schemas/responses.ts` around lines 429 - 435, Replace the hard-coded enum literals in SessionCompactionModeSchema and SessionCompactionTriggerSchema with the canonical arrays from core: import and use SESSION_COMPACTION_MODES for SessionCompactionModeSchema and SESSION_COMPACTION_TRIGGERS for SessionCompactionTriggerSchema (same pattern used for LLM_PROVIDERS / LLM_PRICING_STATUSES); ensure you pass those constants into z.enum(...) so the schemas always reflect the single source of truth in core rather than redeclared strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/context/compaction/strategies/reactive-overflow.test.ts`:
- Around line 96-104: The test currently drops preserved messages without ids
using flatMap which masks production failures in materializeSummary; update the
test to first assert every preserved working message has an id (e.g., iterate
the slice from buildCompactionWindow(history).workingHistory starting at
result.preserveFromWorkingIndex and throw/expect if any message.id is missing)
and then construct preservedMessageIds by mapping to message.id (not flatMap),
so the test mirrors materializeSummary's validation; reference
buildCompactionWindow, result.preserveFromWorkingIndex, preservedMessageIds, and
materializeSummary in your change.
In `@packages/core/src/session/compaction-service.ts`:
- Around line 151-169: The preserved tail in the "continue-in-place" path is
getting new/minted IDs from normalizeCompactionMessage which causes
preservedMessageIds to not match the source session and later be dropped; change
the logic that builds preservedWorkingMessages/preservedMessageIds so it uses
the original source-session IDs (from compactionWindow.workingHistory) instead
of letting normalizeCompactionMessage synthesize IDs: either add a flag/option
to normalizeCompactionMessage to preserve/pass-through existing message.id or
build preservedMessageIds directly from compactionWindow.workingHistory.map(m =>
m.id) and supply those IDs to normalizeSummaryMessage; also move the validation
that can raise SessionCompactionError.preservedMessageMissingId to run before
any ID-minting so missing IDs are detected early (affecting
resolvePreservedMessageIds / buildCompactionWindow / filterCompacted
integration).
---
Nitpick comments:
In `@packages/server/src/hono/schemas/responses.ts`:
- Around line 429-435: Replace the hard-coded enum literals in
SessionCompactionModeSchema and SessionCompactionTriggerSchema with the
canonical arrays from core: import and use SESSION_COMPACTION_MODES for
SessionCompactionModeSchema and SESSION_COMPACTION_TRIGGERS for
SessionCompactionTriggerSchema (same pattern used for LLM_PROVIDERS /
LLM_PRICING_STATUSES); ensure you pass those constants into z.enum(...) so the
schemas always reflect the single source of truth in core rather than redeclared
strings.
🪄 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: 9c036d7d-6db2-42fe-8fb1-9b3438cf04ee
📒 Files selected for processing (16)
docs/static/openapi/openapi.jsonpackages/core/src/agent/DextoAgent.session-compaction.integration.test.tspackages/core/src/context/compaction/compaction.integration.test.tspackages/core/src/context/compaction/index.tspackages/core/src/context/compaction/strategies/noop.tspackages/core/src/context/compaction/strategies/reactive-overflow-compaction.tspackages/core/src/context/compaction/strategies/reactive-overflow.test.tspackages/core/src/context/compaction/types.tspackages/core/src/context/compaction/window.tspackages/core/src/context/utils.test.tspackages/core/src/context/utils.tspackages/core/src/llm/executor/turn-executor.integration.test.tspackages/core/src/session/compaction-service.tspackages/core/src/session/errors.tspackages/server/src/hono/__tests__/api.integration.test.tspackages/server/src/hono/schemas/responses.ts
✅ Files skipped from review due to trivial changes (3)
- packages/core/src/context/compaction/index.ts
- packages/core/src/llm/executor/turn-executor.integration.test.ts
- packages/server/src/hono/tests/api.integration.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/context/compaction/types.ts
packages/core/src/context/compaction/strategies/reactive-overflow.test.ts
Show resolved
Hide resolved
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
1 participant
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 by CodeRabbit
New Features
Documentation
API / Schema
Events
Tests