[feat] Extract the headless chat core into @agenta/chat (3/12) - #5682
[feat] Extract the headless chat core into @agenta/chat (3/12)#5682ardaerzin wants to merge 19 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded the private ChangesShared chat package
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-chat/tests/unit/model/toolSummary.test.ts (1)
1-144: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Prettier formatting failure reported by CI.
The pipeline log reports a Prettier formatting check failure for this file. Run
prettier --writeon this file and commit the result before merge.As per pipeline failure logs, "Prettier formatting check failed. Run 'prettier --write' to fix code style issues."
Source: Pipeline failures
🧹 Nitpick comments (8)
web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the batch coverage to the tool-part branch.
The suite covers only a text batch reply. The replay loop also handles
reasoningparts andtool_use/tool_resultblocks, which is the branch with the empty-tool-name defect flagged inweb/packages/agenta-chat/src/transport/AgentChatTransport.ts. Add a case whose batch body contains atool_useblock and a matchingtool_resultblock, then assert the emittedtool-input-availablename and thetool-output-availablepayload. Also assert the SSE pass-through path, so a regression inresolvedModeselection fails a test.The first test asserts only
instanceof. Assert thatbaseFetchreceives the request to prove the negotiator wraps the caller-supplied fetch.Also applies to: 65-73
web/packages/agenta-chat/tests/unit/state/sessionMessages.test.ts (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClear the persisted storage key between tests.
sessionMessagesAtomwrites to the reallocalStoragekeyagenta:agent-chat:messagesunder jsdom, andgetOnInit: truereads it back on init. State therefore leaks between tests and between files in the same worker. The current tests pass because each test uses distinct session ids. Add abeforeEachthat callslocalStorage.clear()to keep the suite order-independent.web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (1)
37-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
queuedBySessionis not covered byclearSessionEphemera.
clearSessionEphemeradeletescomposerDraftBySession,attachmentsBySession, andfreshSessionIdsonly (seeweb/packages/agenta-chat/src/state/sessionEphemera.tslines 44-48). This module-level queue map is not cleared. After a session is closed or deleted, its held messages stay in memory for the page lifetime, and a remount under the same session id restores them. Consider moving the map intosessionEphemeraso one cleanup helper owns every per-session ephemeral store.Note: the file header states byte-parity with the OSS original, so apply the same change on both sides if you take it.
web/packages/agenta-chat/src/model/parts.ts (1)
3-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the repeated provenance comment into one note per file.
Six model files repeat the same 3-line "Copied verbatim from ... Keep byte-parity if either side changes" comment before nearly every exported type and function. State it once, at the top of each file, instead of duplicating it per export.
web/packages/agenta-chat/src/model/parts.ts#L3-L56: keep one top-of-file note; drop the four repeated blocks beforeisToolPart,toolIdentity,isVisiblePart/isEmptyAssistantTurn, andpartToolName.web/packages/agenta-chat/src/model/approvals.ts#L5-L41: keep one top-of-file note; drop the three repeated blocks beforePendingApproval,ApprovalRef, andgetPendingApprovals.web/packages/agenta-chat/src/model/attachments.ts#L10-L17: fold the single repeated block into the existing file-level doc comment.web/packages/agenta-chat/src/model/error.ts#L1-L40: keep one top-of-file note; drop the repeated block beforeparseAgentRunError.web/packages/agenta-chat/src/model/renderModel.ts#L5-L82: keep one top-of-file note; drop the repeated blocks beforeRenderItem,executedToolIdentities,isSupersededGate, andbuildTurnRenderItems.web/packages/agenta-chat/src/model/sessionStatus.ts#L1-L23: keep one top-of-file note; drop the repeated block beforederiveSessionRunStatus.As per coding guidelines, "Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements."
Source: Coding guidelines
web/packages/agenta-chat/src/model/toolSummary.ts (1)
16-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the repeated "Copied verbatim" comment block.
The identical 3-line comment "Copied verbatim from ... Keep byte-parity if either side changes." appears six times in this file (Lines 16-18, 24-26, 36-38, 44-46, 52-54, 83-85). The coding guideline for this path asks for comments of at most one short line unless documenting a genuinely surprising constraint. A single top-of-file note covering the whole "copied verbatim, keep byte-parity" provenance for this module would satisfy the same intent with far less repetition.
As per coding guidelines, "Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements."
Source: Coding guidelines
web/packages/agenta-chat/src/assets/attachmentRules.ts (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTrack the hand-maintained copy parity with the OSS originals. Five modules are verbatim copies of files under
web/oss/src/components/AgentChatSlice/assets/, and each header asks a future author to keep byte-parity manually. Manual parity drifts, and the two copies then diverge silently until the re-plumb PR lands. Record the deletion of the OSS originals as a tracked follow-up, and consider a CI parity check for the duplicated pairs until then.
web/packages/agenta-chat/src/assets/attachmentRules.ts#L1-L6: note the tracking issue or parity check forAgentChatSlice/assets/attachments.ts.web/packages/agenta-chat/src/assets/files.ts#L1-L3: note the same forAgentChatSlice/assets/files.ts.web/packages/agenta-chat/src/assets/loadSession.ts#L1-L6: note the same forAgentChatSlice/assets/loadSession.ts.web/packages/agenta-chat/src/assets/rewind.ts#L1-L3: note the same forAgentChatSlice/assets/rewind.ts.web/packages/agenta-chat/src/assets/transcriptToMessages.ts#L1-L3: note the same forAgentChatSlice/assets/transcriptToMessages.ts.I can open an issue that lists the duplicated pairs and the removal steps. Do you want me to open it?
web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts (1)
17-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the approval replay branches.
The suite covers text, tool, and boundary paths. It omits
interaction_request/interaction_response, the synthesized tool part for a parked call, the sentinel-error re-open path, and theusagemerge. Those branches carry the most state transitions inapplyEvent, and the sentinel comparison is the subject of a separate comment ontranscriptToMessages.ts. Add cases for them.web/packages/agenta-chat/tests/unit/assets/files.test.ts (1)
6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover
filesToPartswith a jsdom per-file environment.
filesToPartsis the send path throughfileToPart, butfiles.test.tsand the package vitest config leave it under the node environment without coverage. Add//@vitest-environmentjsdomto coverFileReaderencoding isolated from the package default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 399b409c-d85d-4882-8eec-15a69028f1c7
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (76)
web/ee/package.jsonweb/oss/next.config.tsweb/oss/package.jsonweb/packages/agenta-chat/.gitignoreweb/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/attachmentRules.tsweb/packages/agenta-chat/src/assets/files.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/loadSession.tsweb/packages/agenta-chat/src/assets/rewind.tsweb/packages/agenta-chat/src/assets/toolFormat.tsweb/packages/agenta-chat/src/assets/trace.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/hooks/index.tsweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.tsweb/packages/agenta-chat/src/hooks/useApprovalDock.tsweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsweb/packages/agenta-chat/src/index.tsweb/packages/agenta-chat/src/model/actions.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/attachments.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/grouping.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/parts.tsweb/packages/agenta-chat/src/model/renderModel.tsweb/packages/agenta-chat/src/model/sessionStatus.tsweb/packages/agenta-chat/src/model/toolSummary.tsweb/packages/agenta-chat/src/model/turnStatus.tsweb/packages/agenta-chat/src/model/turnViewModel.tsweb/packages/agenta-chat/src/skin/index.tsweb/packages/agenta-chat/src/skin/registry.tsweb/packages/agenta-chat/src/skin/types.tsweb/packages/agenta-chat/src/state/expandState.tsweb/packages/agenta-chat/src/state/index.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/src/state/sessionMessages.tsweb/packages/agenta-chat/src/transport/AgentChatTransport.tsweb/packages/agenta-chat/src/transport/index.tsweb/packages/agenta-chat/tests/unit/assets/attachmentRules.test.tsweb/packages/agenta-chat/tests/unit/assets/files.test.tsweb/packages/agenta-chat/tests/unit/assets/loadSession.test.tsweb/packages/agenta-chat/tests/unit/assets/rewind.test.tsweb/packages/agenta-chat/tests/unit/assets/toolFormat.test.tsweb/packages/agenta-chat/tests/unit/assets/trace.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/fixtures/approvalTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/emptyTurns.jsonweb/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/supersededGate.jsonweb/packages/agenta-chat/tests/unit/fixtures/toolTurn.jsonweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.tsweb/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.tsweb/packages/agenta-chat/tests/unit/model/approvals.test.tsweb/packages/agenta-chat/tests/unit/model/attachments.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/grouping.test.tsweb/packages/agenta-chat/tests/unit/model/parts.test.tsweb/packages/agenta-chat/tests/unit/model/renderModel.test.tsweb/packages/agenta-chat/tests/unit/model/sessionStatus.test.tsweb/packages/agenta-chat/tests/unit/model/toolSummary.test.tsweb/packages/agenta-chat/tests/unit/model/turnStatus.test.tsweb/packages/agenta-chat/tests/unit/model/turnViewModel.test.tsweb/packages/agenta-chat/tests/unit/package.test.tsweb/packages/agenta-chat/tests/unit/skin/registry.test.tsweb/packages/agenta-chat/tests/unit/state/expandState.test.tsweb/packages/agenta-chat/tests/unit/state/sessionEphemera.test.tsweb/packages/agenta-chat/tests/unit/state/sessionMessages.test.tsweb/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.tsweb/packages/agenta-chat/tsconfig.jsonweb/packages/agenta-chat/vitest.config.tsweb/turbo.json
4c9c4be to
48a843f
Compare
2bdbdca to
1976012
Compare
1976012 to
a10ef27
Compare
a10ef27 to
af8398c
Compare
|
Went through these against the code. Two were real and are fixed; the rest are upstream, and I'd rather say why than quietly patch a copy. The framing that decides most of them: this package is copy-extracted from Fixed — Fixed — duplicated park sentinels in Not taking — sentinel matched by equality rather than prefix ( Not taking — same-tick Not taking — rejected background revalidation ( Where these are genuine defects they should be fixed in |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/agenta-chat/src/state/expandState.ts (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten this comment.
Lines 18-19 describe normal key ownership. Use one short comment. Reserve longer comments for surprising constraints.
Proposed change
-// ── Key builders: the SINGLE source of truth for the key format, used by BOTH the widgets and the -// pruner below, so the two can never drift out of sync. ── +// Key builders are shared by widgets and the pruner.As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 64348a9e-40b5-4cbb-8ce9-252abf45933e
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (76)
web/ee/package.jsonweb/oss/next.config.tsweb/oss/package.jsonweb/packages/agenta-chat/.gitignoreweb/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/attachmentRules.tsweb/packages/agenta-chat/src/assets/files.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/loadSession.tsweb/packages/agenta-chat/src/assets/rewind.tsweb/packages/agenta-chat/src/assets/toolFormat.tsweb/packages/agenta-chat/src/assets/trace.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/hooks/index.tsweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.tsweb/packages/agenta-chat/src/hooks/useApprovalDock.tsweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsweb/packages/agenta-chat/src/index.tsweb/packages/agenta-chat/src/model/actions.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/attachments.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/grouping.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/parts.tsweb/packages/agenta-chat/src/model/renderModel.tsweb/packages/agenta-chat/src/model/sessionStatus.tsweb/packages/agenta-chat/src/model/toolSummary.tsweb/packages/agenta-chat/src/model/turnStatus.tsweb/packages/agenta-chat/src/model/turnViewModel.tsweb/packages/agenta-chat/src/skin/index.tsweb/packages/agenta-chat/src/skin/registry.tsweb/packages/agenta-chat/src/skin/types.tsweb/packages/agenta-chat/src/state/expandState.tsweb/packages/agenta-chat/src/state/index.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/src/state/sessionMessages.tsweb/packages/agenta-chat/src/transport/AgentChatTransport.tsweb/packages/agenta-chat/src/transport/index.tsweb/packages/agenta-chat/tests/unit/assets/attachmentRules.test.tsweb/packages/agenta-chat/tests/unit/assets/files.test.tsweb/packages/agenta-chat/tests/unit/assets/loadSession.test.tsweb/packages/agenta-chat/tests/unit/assets/rewind.test.tsweb/packages/agenta-chat/tests/unit/assets/toolFormat.test.tsweb/packages/agenta-chat/tests/unit/assets/trace.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/fixtures/approvalTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/emptyTurns.jsonweb/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/supersededGate.jsonweb/packages/agenta-chat/tests/unit/fixtures/toolTurn.jsonweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.tsweb/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.tsweb/packages/agenta-chat/tests/unit/model/approvals.test.tsweb/packages/agenta-chat/tests/unit/model/attachments.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/grouping.test.tsweb/packages/agenta-chat/tests/unit/model/parts.test.tsweb/packages/agenta-chat/tests/unit/model/renderModel.test.tsweb/packages/agenta-chat/tests/unit/model/sessionStatus.test.tsweb/packages/agenta-chat/tests/unit/model/toolSummary.test.tsweb/packages/agenta-chat/tests/unit/model/turnStatus.test.tsweb/packages/agenta-chat/tests/unit/model/turnViewModel.test.tsweb/packages/agenta-chat/tests/unit/package.test.tsweb/packages/agenta-chat/tests/unit/skin/registry.test.tsweb/packages/agenta-chat/tests/unit/state/expandState.test.tsweb/packages/agenta-chat/tests/unit/state/sessionEphemera.test.tsweb/packages/agenta-chat/tests/unit/state/sessionMessages.test.tsweb/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.tsweb/packages/agenta-chat/tsconfig.jsonweb/packages/agenta-chat/vitest.config.tsweb/turbo.json
🚧 Files skipped from review as they are similar to previous changes (73)
- web/oss/package.json
- web/packages/agenta-chat/src/assets/index.ts
- web/packages/agenta-chat/src/skin/index.ts
- web/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.json
- web/ee/package.json
- web/packages/agenta-chat/tests/unit/fixtures/toolTurn.json
- web/packages/agenta-chat/src/model/attachments.ts
- web/packages/agenta-chat/src/transport/index.ts
- web/oss/next.config.ts
- web/packages/agenta-chat/tests/unit/model/approvals.test.ts
- web/packages/agenta-chat/src/hooks/index.ts
- web/packages/agenta-chat/tests/unit/model/grouping.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/approvalTurn.json
- web/packages/agenta-chat/tests/unit/assets/trace.test.ts
- web/packages/agenta-chat/src/model/grouping.ts
- web/packages/agenta-chat/tests/unit/state/sessionMessages.test.ts
- web/packages/agenta-chat/vitest.config.ts
- web/packages/agenta-chat/tests/unit/assets/toolFormat.test.ts
- web/packages/agenta-chat/src/index.ts
- web/packages/agenta-chat/.gitignore
- web/packages/agenta-chat/tests/unit/model/parts.test.ts
- web/packages/agenta-chat/src/model/sessionStatus.ts
- web/packages/agenta-chat/src/model/index.ts
- web/packages/agenta-chat/src/state/index.ts
- web/packages/agenta-chat/src/hooks/useComposerAttachments.ts
- web/packages/agenta-chat/tests/unit/fixtures/supersededGate.json
- web/packages/agenta-chat/src/assets/rewind.ts
- web/packages/agenta-chat/tests/unit/model/error.test.ts
- web/packages/agenta-chat/tsconfig.json
- web/packages/agenta-chat/tests/unit/assets/files.test.ts
- web/packages/agenta-chat/tests/unit/model/turnStatus.test.ts
- web/packages/agenta-chat/tests/unit/assets/rewind.test.ts
- web/packages/agenta-chat/src/model/actions.ts
- web/packages/agenta-chat/src/skin/types.ts
- web/packages/agenta-chat/src/assets/toolFormat.ts
- web/packages/agenta-chat/src/model/approvals.ts
- web/turbo.json
- web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts
- web/packages/agenta-chat/tests/unit/state/expandState.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/emptyTurns.json
- web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
- web/packages/agenta-chat/tests/unit/state/sessionEphemera.test.ts
- web/packages/agenta-chat/src/assets/loadSession.ts
- web/packages/agenta-chat/tests/unit/skin/registry.test.ts
- web/packages/agenta-chat/src/model/turnStatus.ts
- web/packages/agenta-chat/src/model/toolSummary.ts
- web/packages/agenta-chat/src/model/parts.ts
- web/packages/agenta-chat/tests/unit/model/renderModel.test.ts
- web/packages/agenta-chat/tests/unit/model/attachments.test.ts
- web/packages/agenta-chat/package.json
- web/packages/agenta-chat/tests/unit/model/sessionStatus.test.ts
- web/packages/agenta-chat/src/transport/AgentChatTransport.ts
- web/packages/agenta-chat/tests/unit/assets/loadSession.test.ts
- web/packages/agenta-chat/src/model/renderModel.ts
- web/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.ts
- web/packages/agenta-chat/src/skin/registry.ts
- web/packages/agenta-chat/src/model/turnViewModel.ts
- web/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.ts
- web/packages/agenta-chat/tests/unit/assets/attachmentRules.test.ts
- web/packages/agenta-chat/tests/unit/model/toolSummary.test.ts
- web/packages/agenta-chat/src/assets/trace.ts
- web/packages/agenta-chat/src/state/sessionMessages.ts
- web/packages/agenta-chat/tests/unit/model/turnViewModel.test.ts
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- web/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.ts
- web/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.ts
- web/packages/agenta-chat/src/state/sessionEphemera.ts
- web/packages/agenta-chat/src/assets/attachmentRules.ts
- web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
- web/packages/agenta-chat/src/assets/transcriptToMessages.ts
- web/packages/agenta-chat/src/hooks/useApprovalDock.ts
- web/packages/agenta-chat/src/assets/files.ts
- web/packages/agenta-chat/src/hooks/useAgentConversation.ts
48a843f to
a024cad
Compare
af8398c to
abe74aa
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Read all six against the code. Five are real and are fixed in this PR; one is stale. A Detached revalidation with no rejection handler — real, fixed. The
Same-tick Approved-execution sentinel matched by equality — real, fixed. Each of the five has a test that I confirmed fails without the fix. Prettier on the two new test files — stale. Both files pass |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/packages/agenta-chat/src/state/sessionEphemera.ts (1)
1-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the in-code comments to short single-line comments.
Move implementation history and extended test rationale to package documentation or test names.
web/packages/agenta-chat/src/state/sessionEphemera.ts#L1-L43: Replace the block and wrapped comments with concise runtime constraints only.web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts#L36-L38: Reduce the fallback explanation to one short line.web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts#L52-L53: Reduce the header explanation to one short line.web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts#L75-L76: Reduce the tool-part explanation to one short line.As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
web/packages/agenta-chat/src/hooks/useAgentConversation.ts (1)
155-160: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a dev-time guard for the documented
sessionId-stability assumption.
initialMessagesis seeded once withuseState, so it keeps the first session's cache for the life of the hook instance. The hydration effect at Line 258 and the revalidate effect at Line 306 both branch oninitialMessages.length. If a consumer changessessionIdon a mounted instance, both effects use the previous session's cache and can pick the wrong branch (hydrate a cached session, or skip hydration for an uncached one).restoredIdsRef, the initialisHydratingvalue, andexecutedForalso keep the previous session's data.The comments state that
sessionIdis stable per instance, but the public API does not enforce it. Add a development-only warning so a future skin that reuses the instance fails loudly instead of silently restoring the wrong transcript.♻️ Proposed guard
+ // The seeded state below is per-session; reusing one instance across sessions is a bug. + const initialSessionIdRef = useRef(sessionId) + if (process.env.NODE_ENV !== "production" && initialSessionIdRef.current !== sessionId) { + console.error( + "[useAgentConversation] sessionId changed on a mounted instance; remount with key={sessionId}.", + ) + }Also applies to: 249-261, 305-306
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4fd77928-c4f7-4ac7-86d1-d164d550cf07
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (76)
web/ee/package.jsonweb/oss/next.config.tsweb/oss/package.jsonweb/packages/agenta-chat/.gitignoreweb/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/attachmentRules.tsweb/packages/agenta-chat/src/assets/files.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/loadSession.tsweb/packages/agenta-chat/src/assets/rewind.tsweb/packages/agenta-chat/src/assets/toolFormat.tsweb/packages/agenta-chat/src/assets/trace.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/hooks/index.tsweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.tsweb/packages/agenta-chat/src/hooks/useApprovalDock.tsweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsweb/packages/agenta-chat/src/index.tsweb/packages/agenta-chat/src/model/actions.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/attachments.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/grouping.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/parts.tsweb/packages/agenta-chat/src/model/renderModel.tsweb/packages/agenta-chat/src/model/sessionStatus.tsweb/packages/agenta-chat/src/model/toolSummary.tsweb/packages/agenta-chat/src/model/turnStatus.tsweb/packages/agenta-chat/src/model/turnViewModel.tsweb/packages/agenta-chat/src/skin/index.tsweb/packages/agenta-chat/src/skin/registry.tsweb/packages/agenta-chat/src/skin/types.tsweb/packages/agenta-chat/src/state/expandState.tsweb/packages/agenta-chat/src/state/index.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/src/state/sessionMessages.tsweb/packages/agenta-chat/src/transport/AgentChatTransport.tsweb/packages/agenta-chat/src/transport/index.tsweb/packages/agenta-chat/tests/unit/assets/attachmentRules.test.tsweb/packages/agenta-chat/tests/unit/assets/files.test.tsweb/packages/agenta-chat/tests/unit/assets/loadSession.test.tsweb/packages/agenta-chat/tests/unit/assets/rewind.test.tsweb/packages/agenta-chat/tests/unit/assets/toolFormat.test.tsweb/packages/agenta-chat/tests/unit/assets/trace.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/fixtures/approvalTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/emptyTurns.jsonweb/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/supersededGate.jsonweb/packages/agenta-chat/tests/unit/fixtures/toolTurn.jsonweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.tsweb/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.tsweb/packages/agenta-chat/tests/unit/model/approvals.test.tsweb/packages/agenta-chat/tests/unit/model/attachments.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/grouping.test.tsweb/packages/agenta-chat/tests/unit/model/parts.test.tsweb/packages/agenta-chat/tests/unit/model/renderModel.test.tsweb/packages/agenta-chat/tests/unit/model/sessionStatus.test.tsweb/packages/agenta-chat/tests/unit/model/toolSummary.test.tsweb/packages/agenta-chat/tests/unit/model/turnStatus.test.tsweb/packages/agenta-chat/tests/unit/model/turnViewModel.test.tsweb/packages/agenta-chat/tests/unit/package.test.tsweb/packages/agenta-chat/tests/unit/skin/registry.test.tsweb/packages/agenta-chat/tests/unit/state/expandState.test.tsweb/packages/agenta-chat/tests/unit/state/sessionEphemera.test.tsweb/packages/agenta-chat/tests/unit/state/sessionMessages.test.tsweb/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.tsweb/packages/agenta-chat/tsconfig.jsonweb/packages/agenta-chat/vitest.config.tsweb/turbo.json
🚧 Files skipped from review as they are similar to previous changes (69)
- web/ee/package.json
- web/turbo.json
- web/packages/agenta-chat/vitest.config.ts
- web/oss/next.config.ts
- web/packages/agenta-chat/src/hooks/index.ts
- web/oss/package.json
- web/packages/agenta-chat/tsconfig.json
- web/packages/agenta-chat/tests/unit/package.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.json
- web/packages/agenta-chat/tests/unit/assets/toolFormat.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/approvalTurn.json
- web/packages/agenta-chat/package.json
- web/packages/agenta-chat/tests/unit/assets/files.test.ts
- web/packages/agenta-chat/src/transport/index.ts
- web/packages/agenta-chat/tests/unit/model/attachments.test.ts
- web/packages/agenta-chat/tests/unit/assets/trace.test.ts
- web/packages/agenta-chat/tests/unit/model/error.test.ts
- web/packages/agenta-chat/src/model/attachments.ts
- web/packages/agenta-chat/src/assets/rewind.ts
- web/packages/agenta-chat/tests/unit/state/expandState.test.ts
- web/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.ts
- web/packages/agenta-chat/src/assets/toolFormat.ts
- web/packages/agenta-chat/src/model/actions.ts
- web/packages/agenta-chat/tests/unit/fixtures/supersededGate.json
- web/packages/agenta-chat/tests/unit/skin/registry.test.ts
- web/packages/agenta-chat/src/model/grouping.ts
- web/packages/agenta-chat/tests/unit/assets/rewind.test.ts
- web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
- web/packages/agenta-chat/tests/unit/model/grouping.test.ts
- web/packages/agenta-chat/tests/unit/model/approvals.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/toolTurn.json
- web/packages/agenta-chat/tests/unit/model/toolSummary.test.ts
- web/packages/agenta-chat/src/index.ts
- web/packages/agenta-chat/tests/unit/model/turnViewModel.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/emptyTurns.json
- web/packages/agenta-chat/tests/unit/state/sessionMessages.test.ts
- web/packages/agenta-chat/src/assets/trace.ts
- web/packages/agenta-chat/src/assets/files.ts
- web/packages/agenta-chat/tests/unit/state/sessionEphemera.test.ts
- web/packages/agenta-chat/.gitignore
- web/packages/agenta-chat/src/assets/loadSession.ts
- web/packages/agenta-chat/tests/unit/assets/attachmentRules.test.ts
- web/packages/agenta-chat/src/transport/AgentChatTransport.ts
- web/packages/agenta-chat/src/model/parts.ts
- web/packages/agenta-chat/tests/unit/model/turnStatus.test.ts
- web/packages/agenta-chat/tests/unit/model/parts.test.ts
- web/packages/agenta-chat/src/model/approvals.ts
- web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
- web/packages/agenta-chat/src/assets/attachmentRules.ts
- web/packages/agenta-chat/src/assets/transcriptToMessages.ts
- web/packages/agenta-chat/src/state/index.ts
- web/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.ts
- web/packages/agenta-chat/src/hooks/useApprovalDock.ts
- web/packages/agenta-chat/tests/unit/model/renderModel.test.ts
- web/packages/agenta-chat/tests/unit/model/sessionStatus.test.ts
- web/packages/agenta-chat/src/skin/index.ts
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- web/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.ts
- web/packages/agenta-chat/src/state/sessionMessages.ts
- web/packages/agenta-chat/src/skin/types.ts
- web/packages/agenta-chat/src/model/renderModel.ts
- web/packages/agenta-chat/src/skin/registry.ts
- web/packages/agenta-chat/src/model/toolSummary.ts
- web/packages/agenta-chat/src/model/turnViewModel.ts
- web/packages/agenta-chat/src/model/turnStatus.ts
- web/packages/agenta-chat/src/model/sessionStatus.ts
- web/packages/agenta-chat/src/assets/index.ts
- web/packages/agenta-chat/src/model/error.ts
- web/packages/agenta-chat/src/model/index.ts
| const [files, setFiles] = useState<PendingAttachment[]>(() => | ||
| sessionId ? (attachmentsBySession.get(sessionId) ?? []) : [], | ||
| ) | ||
| useEffect(() => { | ||
| if (!sessionId) return | ||
| if (files.length > 0) attachmentsBySession.set(sessionId, files) | ||
| else attachmentsBySession.delete(sessionId) | ||
| }, [files, sessionId]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Both hooks seed per-session state once but accept sessionId as a mutable prop. Each hook reads a per-session store in a useState initializer, then keys later effects on sessionId. Neither hook re-seeds nor guards when sessionId changes on a mounted instance, so the previous session's state leaks into the new session.
web/packages/agenta-chat/src/hooks/useComposerAttachments.ts#L61-L68: re-seedfilesfromattachmentsBySessionwhensessionIdchanges, before the mirror effect writes them under the new key.web/packages/agenta-chat/src/hooks/useAgentConversation.ts#L155-L160: add a development-only warning whensessionIdchanges, so a consumer that reuses the instance is forced to remount withkey={sessionId}instead of branching on the previous session'sinitialMessages.
📍 Affects 2 files
web/packages/agenta-chat/src/hooks/useComposerAttachments.ts#L61-L68(this comment)web/packages/agenta-chat/src/hooks/useAgentConversation.ts#L155-L160
a024cad to
75be3f7
Compare
6e2e450 to
854acef
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/agenta-chat/tests/unit/package.test.ts (1)
9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the parsed package manifest.
JSON.parseinfersanyforpkg. This propagates unchecked values intoall.Declare a narrow manifest type with optional dependency maps. Parse into that type before building
all.As per coding guidelines, workspace packages must “avoid
any”.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e50813c-1bbf-4ce7-82ad-6db7b396d6fa
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (76)
web/ee/package.jsonweb/oss/next.config.tsweb/oss/package.jsonweb/packages/agenta-chat/.gitignoreweb/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/attachmentRules.tsweb/packages/agenta-chat/src/assets/files.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/loadSession.tsweb/packages/agenta-chat/src/assets/rewind.tsweb/packages/agenta-chat/src/assets/toolFormat.tsweb/packages/agenta-chat/src/assets/trace.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/hooks/index.tsweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.tsweb/packages/agenta-chat/src/hooks/useApprovalDock.tsweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsweb/packages/agenta-chat/src/index.tsweb/packages/agenta-chat/src/model/actions.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/attachments.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/grouping.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/parts.tsweb/packages/agenta-chat/src/model/renderModel.tsweb/packages/agenta-chat/src/model/sessionStatus.tsweb/packages/agenta-chat/src/model/toolSummary.tsweb/packages/agenta-chat/src/model/turnStatus.tsweb/packages/agenta-chat/src/model/turnViewModel.tsweb/packages/agenta-chat/src/skin/index.tsweb/packages/agenta-chat/src/skin/registry.tsweb/packages/agenta-chat/src/skin/types.tsweb/packages/agenta-chat/src/state/expandState.tsweb/packages/agenta-chat/src/state/index.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/src/state/sessionMessages.tsweb/packages/agenta-chat/src/transport/AgentChatTransport.tsweb/packages/agenta-chat/src/transport/index.tsweb/packages/agenta-chat/tests/unit/assets/attachmentRules.test.tsweb/packages/agenta-chat/tests/unit/assets/files.test.tsweb/packages/agenta-chat/tests/unit/assets/loadSession.test.tsweb/packages/agenta-chat/tests/unit/assets/rewind.test.tsweb/packages/agenta-chat/tests/unit/assets/toolFormat.test.tsweb/packages/agenta-chat/tests/unit/assets/trace.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/fixtures/approvalTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/emptyTurns.jsonweb/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.jsonweb/packages/agenta-chat/tests/unit/fixtures/supersededGate.jsonweb/packages/agenta-chat/tests/unit/fixtures/toolTurn.jsonweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.tsweb/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.tsweb/packages/agenta-chat/tests/unit/model/approvals.test.tsweb/packages/agenta-chat/tests/unit/model/attachments.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/grouping.test.tsweb/packages/agenta-chat/tests/unit/model/parts.test.tsweb/packages/agenta-chat/tests/unit/model/renderModel.test.tsweb/packages/agenta-chat/tests/unit/model/sessionStatus.test.tsweb/packages/agenta-chat/tests/unit/model/toolSummary.test.tsweb/packages/agenta-chat/tests/unit/model/turnStatus.test.tsweb/packages/agenta-chat/tests/unit/model/turnViewModel.test.tsweb/packages/agenta-chat/tests/unit/package.test.tsweb/packages/agenta-chat/tests/unit/skin/registry.test.tsweb/packages/agenta-chat/tests/unit/state/expandState.test.tsweb/packages/agenta-chat/tests/unit/state/sessionEphemera.test.tsweb/packages/agenta-chat/tests/unit/state/sessionMessages.test.tsweb/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.tsweb/packages/agenta-chat/tsconfig.jsonweb/packages/agenta-chat/vitest.config.tsweb/turbo.json
🚧 Files skipped from review as they are similar to previous changes (73)
- web/oss/package.json
- web/packages/agenta-chat/tests/unit/model/approvals.test.ts
- web/packages/agenta-chat/vitest.config.ts
- web/packages/agenta-chat/tests/unit/fixtures/supersededGate.json
- web/packages/agenta-chat/src/transport/index.ts
- web/packages/agenta-chat/tests/unit/fixtures/toolTurn.json
- web/packages/agenta-chat/tests/unit/model/attachments.test.ts
- web/packages/agenta-chat/src/model/approvals.ts
- web/oss/next.config.ts
- web/ee/package.json
- web/packages/agenta-chat/tests/unit/fixtures/reasoningOnlyTurn.json
- web/packages/agenta-chat/tsconfig.json
- web/packages/agenta-chat/tests/unit/model/turnStatus.test.ts
- web/packages/agenta-chat/tests/unit/state/expandState.test.ts
- web/packages/agenta-chat/tests/unit/model/error.test.ts
- web/packages/agenta-chat/src/model/actions.ts
- web/packages/agenta-chat/tests/unit/assets/toolFormat.test.ts
- web/packages/agenta-chat/src/index.ts
- web/packages/agenta-chat/tests/unit/assets/trace.test.ts
- web/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.ts
- web/packages/agenta-chat/src/state/index.ts
- web/packages/agenta-chat/.gitignore
- web/packages/agenta-chat/tests/unit/model/grouping.test.ts
- web/packages/agenta-chat/tests/unit/fixtures/approvalTurn.json
- web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
- web/packages/agenta-chat/tests/unit/model/sessionStatus.test.ts
- web/packages/agenta-chat/tests/unit/model/renderModel.test.ts
- web/packages/agenta-chat/tests/unit/model/turnViewModel.test.ts
- web/packages/agenta-chat/tests/unit/skin/registry.test.ts
- web/turbo.json
- web/packages/agenta-chat/src/skin/index.ts
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- web/packages/agenta-chat/tests/unit/assets/attachmentRules.test.ts
- web/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.ts
- web/packages/agenta-chat/tests/unit/model/parts.test.ts
- web/packages/agenta-chat/src/assets/index.ts
- web/packages/agenta-chat/tests/unit/state/sessionMessages.test.ts
- web/packages/agenta-chat/src/model/index.ts
- web/packages/agenta-chat/tests/unit/model/toolSummary.test.ts
- web/packages/agenta-chat/src/model/grouping.ts
- web/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.ts
- web/packages/agenta-chat/src/assets/toolFormat.ts
- web/packages/agenta-chat/src/assets/trace.ts
- web/packages/agenta-chat/tests/unit/fixtures/emptyTurns.json
- web/packages/agenta-chat/src/skin/types.ts
- web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts
- web/packages/agenta-chat/src/model/error.ts
- web/packages/agenta-chat/src/skin/registry.ts
- web/packages/agenta-chat/src/model/renderModel.ts
- web/packages/agenta-chat/src/model/attachments.ts
- web/packages/agenta-chat/src/hooks/useComposerAttachments.ts
- web/packages/agenta-chat/src/model/turnViewModel.ts
- web/packages/agenta-chat/src/model/parts.ts
- web/packages/agenta-chat/src/assets/loadSession.ts
- web/packages/agenta-chat/tests/unit/assets/loadSession.test.ts
- web/packages/agenta-chat/src/model/toolSummary.ts
- web/packages/agenta-chat/src/assets/transcriptToMessages.ts
- web/packages/agenta-chat/tests/unit/assets/rewind.test.ts
- web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
- web/packages/agenta-chat/src/assets/attachmentRules.ts
- web/packages/agenta-chat/tests/unit/assets/files.test.ts
- web/packages/agenta-chat/src/model/turnStatus.ts
- web/packages/agenta-chat/package.json
- web/packages/agenta-chat/src/state/sessionEphemera.ts
- web/packages/agenta-chat/src/assets/files.ts
- web/packages/agenta-chat/src/hooks/index.ts
- web/packages/agenta-chat/src/model/sessionStatus.ts
- web/packages/agenta-chat/src/assets/rewind.ts
- web/packages/agenta-chat/src/state/sessionMessages.ts
- web/packages/agenta-chat/tests/unit/state/sessionEphemera.test.ts
- web/packages/agenta-chat/src/hooks/useApprovalDock.ts
- web/packages/agenta-chat/src/hooks/useAgentConversation.ts
- web/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.ts
75be3f7 to
40fd6b4
Compare
854acef to
7b120cf
Compare
Adds the empty @agenta/chat package (model placeholder only) mirroring agenta-playground's script/tsconfig/vitest shape, and wires it into the oss/ee apps (dependency + transpilePackages) and turbo.json build graph.
Copy SETTLED/isSettled, isDeferredError/isUnknownResultError, isNotHandledOutput, summarizeOutput, rowSummary, and stripFence verbatim from ToolActivity.tsx / toolFormat.ts so the headless chat package can render tool rows without pulling in the OSS ToolDisplay registry.
Copy PendingApproval/ApprovalRef/getPendingApprovals verbatim from ApprovalDock.tsx, reusing the package's own isToolPart/partToolName instead of a local duplicate.
Copy the seven status derivations (hasAnswer/hasReasoning/hasContent/noResponse/ errorText/showError/isError) verbatim from AgentMessage.tsx as a pure deriveTurnStatus(message, ctx) function, including the traceError-vs-runError gating rule (trace errors only trusted on answer-less turns; run errors always count).
Copy the RenderItem union, executedToolIdentities, isSupersededGate, and the consecutive-tool-folding loop verbatim from AgentMessage.tsx, injecting the registry-backed isClientToolPart check as a predicate so this layer stays registry-free.
… desktop chat Copy the active-turn anchoring IIFE (lastUserIndex/activeStart/reserveActive) from AgentConversation.tsx as getTurnGrouping, and the error>awaiting>running>idle session-status precedence as deriveSessionRunStatus, both as pure functions.
Copies toolFormat, trace, attachmentRules (renamed from attachments.ts), files, and rewind from web/oss/src/components/AgentChatSlice/assets into @agenta/chat's src/assets/ — all pure over `ai` types with no OSS-app imports. Canonicalizes stripFence in assets/toolFormat.ts; model/toolSummary.ts now re-exports it instead of holding a second definition. Also: add a .gitignore for the package's vitest test-results/coverage output (mirrors the sibling packages), and a one-line doc comment on isClientToolPart's registry-backed contract in renderModel.ts.
… desktop chat Copies transcriptToMessages (records -> UIMessage[] replay), loadSession (the durable-history hydration seam), and AgentChatTransport (the stream<->batch negotiating AI-SDK transport) from web/oss/src/components/AgentChatSlice/assets into @agenta/chat. All three resolve cleanly through already-allowed package deps (@agenta/entities, @agenta/playground, @agenta/shared, ai, jotai) with no OSS-app imports, so no signature adaptations were needed. New src/transport/ dir + barrel, wired into the package exports map and root src/index.ts.
Copies expandState (per-widget expand/collapse, verbatim) and sessionEphemera (composer drafts, pending attachments, fresh-session markers) from web/oss/src/components/AgentChatSlice/state into @agenta/chat's src/state/. sessionEphemera carries two declared adaptations: attachmentsBySession is typed Map<string, PendingAttachment[]> (the package's neutral staged-upload shape) instead of the desktop's upload-widget file type, and the desktop's per-session virtualized-list scroll/row-height snapshot map is omitted entirely — virtualization state is desktop-only and the package must not depend on that library.
WP3a-C5: the package's own skin registration store (empty until a skin populates it). src/skin/types.ts mirrors the real OSS clientTools, approvals, and toolDisplay registry shapes (generalized, Handler/Renderer renamed to Widget/Entry, no OSS import). src/skin/registry.ts adds registerChatSkin (later-registration-wins merge) plus resolvers that reproduce the OSS resolution order faithfully: render.kind before toolName for client tools, and the toolDisplay override-then-parsed- name-shape fallback chain (mcp__ split / gateway tools__ parsing via the existing @agenta/entities parseGatewayToolName / title-cased raw name). The OSS registries stay byte-untouched per the plan's COPY-mode banner; this store is wired to nothing until the desktop re-plumb PR. Also rewords a stray "Virtuoso" mention in expandState.ts's docstring to keep the banned-literal grep gate conservative.
Restructure useComposerAttachments' add() so setRejections runs outside the setFiles updater (React state updaters must be pure; StrictMode double-invokes them), mirroring the OSS AgentConversation original. Note the registry's kind-override as a deliberate extension over the OSS resolveToolDisplay chain.
Review of the package surfaced five defects that share a shape: a value is read once and used later, after the thing it described has moved on. - The batch transport turned a neutral `tool_result` block into a nameless `tool-` part and replayed an input chunk for it. The AI SDK keys tool parts by `toolCallId`, which that block shares with its `tool_use` sibling, so the empty name and undefined input overwrote the real call and the turn rendered an unnamed tool. Only a part that carries a name emits an input chunk now. - `loadSessionMessages` left its detached revalidation chain without a rejection handler. The chain outlives the function, so the surrounding try/catch never saw it and a failed revalidation surfaced as an unhandled rejection instead of quietly leaving the restored transcript in place. - A rewind plan truncated against the transcript snapshot taken when the plan was built. The skin confirms it after its warning dialog, so anything adopted in that window was thrown away. The index is re-resolved at confirm time, and a plan whose target has left the transcript now does nothing. - The composer validated an incoming batch against the staged count from its render closure, so a paste and a drop in the same tick both saw the same count and could push the staged set past the limit. - The approved-execution sentinel was matched by equality while every sibling user matches by prefix. The code before the colon is the contract; the prose after it is not. Each fix has a test that fails without it.
useComposerAttachments seeds its file list from the per-session store in a useState initializer, then keys the mirror effect on sessionId. Today's only caller mounts one instance per session, so the two always agree. A caller that swapped sessionId in place would not: the hook would keep the previous session's staged files, and the mirror effect would immediately write them under the new session's key, overwriting whatever that session had. The seed is now redone during render when sessionId changes, so the effect never observes a mismatched pair. The test covers the swap in both directions: the new session shows its own files, and the old session keeps its own.
40fd6b4 to
6ce2096
Compare
7b120cf to
e007054
Compare
dosubot
Bot
added
size:XXL
Railway Preview Environment
|
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
🤖 The AI agent says: @ardaerzin, the extracted @agenta/chat package in this PR is not yet behaviorally aligned with the authoritative OSS chat implementation. I found three P1 issues in durable transcript adoption and approval resume replay, plus two P2 issues in attachment replay and file-read failure handling. I left each finding inline. The replay fixes described in findings 2 through 4 appear higher in the stack in #5691; please move them into this package lane so #5682 is merge-safe on its own. The record-count adoption guard and per-file read failure remain unresolved at the current stack tip.
| // guards as the revalidate-on-open effect below — never mid-stream, only when ahead. | ||
| const adoptRefreshed = (freshMsgs: UIMessage[]) => { | ||
| if (cancelled || busyRef.current) return | ||
| if (freshMsgs.length <= messagesRef.current.length) return |
There was a problem hiding this comment.
🤖 The AI agent says: P1: Adopt server growth without comparing message counts. When the cached last turn grows in place, such as an approval resume or a tool result completed elsewhere, transcript replay still produces the same number of UI messages. This guard rejects that newer durable transcript and can leave the reopened conversation permanently partial. Carry the durable record-count watermark through loadSessionMessages and compare that watermark, as the authoritative OSS host does, instead of comparing message counts.
| const responseId = str(payload.id) | ||
| const toolCallId = str(responsePayload.toolCallId) | ||
| const part = | ||
| (toolCallId ? index.tools.get(toolCallId) : undefined) ?? |
There was a problem hiding this comment.
🤖 The AI agent says: P1: Resolve resumed approvals by interaction id first. A cold resume can reissue an approved call under a new toolCallId. Looking up that new input-only part first makes the following state check return before the original approval gate receives the response, so replay leaves the gate pending even though execution continued. Resolve by the stable interaction id first, then remap the new tool id and fold the reissued part into the original gate. This correction appears in #5691 and should move into this package lane.
| // fresh message per turn. | ||
| if (row.session_update === "done" || p.type === "done") { | ||
| if (current && traceId && !current.traceId) current.traceId = traceId | ||
| current = null |
There was a problem hiding this comment.
🤖 The AI agent says: P1: Preserve paused approval turns across done events. A durable done with stopReason: "paused" is not the final assistant-turn boundary. Later approval-resume records still belong to this same turn. Resetting current here splits the resumed execution into a second message and leaves a stale approval row beside it. Keep the draft open across paused boundaries and close it only after the resumed turn completes. This correction appears in #5691 and should move into this package lane.
|
|
||
| switch (type) { | ||
| case "message": { | ||
| draft.parts.push({type: "text", text: str(payload.text)}) |
There was a problem hiding this comment.
🤖 The AI agent says: P2: Replay attachments from durable message events. The current message branch emits only text and discards the attachment descriptors stored in the durable payload. Reopening a file-bearing session through @agenta/chat therefore loses its attachments. Construct file parts from the session id and attachment ids, matching the authoritative OSS replay path. This correction appears in #5691 and should move into this package lane.
|
|
||
| /** Convert picked `File`s into `file` parts for `sendMessage({text, files})`. */ | ||
| export const filesToParts = (files: File[]): Promise<FileUIPart[]> => | ||
| Promise.all(files.map(fileToPart)) |
There was a problem hiding this comment.
🤖 The AI agent says: P2: Preserve sends when one attachment cannot be read. If one selected file becomes unreadable before submission, Promise.all rejects the entire conversion. The send then loses the text and every readable attachment, and callers may receive an unhandled rejection. Settle files individually and return the readable parts together with the failures so one bad file does not abort the whole send.
2 participants
Context
The agent chat lives in
web/oss/src/components/AgentChatSlice, wired to antd, Lexical, and OSS app-layer imports. The mobile app in later lanes needs the same conversation logic (turn grouping, tool identity, approval extraction, transcript replay) but can use none of that UI.This lane extracts the headless half into
@agenta/chat.Changes
A new workspace package holding the parts of the chat that have no opinion about rendering: part predicates and tool identity, error parsing, tool-output summarization, approval extraction, turn status derivation, the turn render model, turn grouping and session status, the transcript replay adapter and transport, the session state stores, and the queue and model-key hooks. On top of those sit two assembled hooks (composer attachments, conversation host) and a skin registry so an app supplies its own components.
Almost none of this is new logic. Each commit copies one unit from the desktop chat, which is why the diff is large and the review is mostly a diff against the original.
Tests / notes
grep "@agenta/chat" web/oss/srcreturns 0 hits: the desktop still uses its own copy, so this PR cannot change desktop behavior. Re-plumbing OSS onto the package is deliberately out of scope; it collides with the open frontend queue and the antd migration, and is tracked as Wave 2 ofdocs/design/agenta-sessions-consolidation/plan.md.git diff --no-index.