Skip to content

fix(agentic): added clarification storage for HITL clarifications#1226

Open
junaid-shirur wants to merge 2 commits intomainfrom
feat/add_clarification_storage
Open

fix(agentic): added clarification storage for HITL clarifications#1226
junaid-shirur wants to merge 2 commits intomainfrom
feat/add_clarification_storage

Conversation

@junaid-shirur
Copy link
Collaborator

@junaid-shirur junaid-shirur commented Nov 24, 2025

Description

Testing

Additional Notes

Summary by CodeRabbit

  • New Features

    • Request user clarification during agent runs with in-memory clarification support.
  • Bug Fixes

    • Clarification selections now send the chosen option value (not an internal id); labels display raw option text.
    • Tool icons now fall back to a default icon when no specific mapping exists.
  • Refactor

    • Removed deprecated tool definitions and streamlined agent/clarification plumbing for consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Refactors clarification handling and JAF context typing across frontend and backend: clarification options changed from objects to strings and selectedId → selectedOption; JAFAdapterCtx renamed to JAFCtx throughout; in-memory clarification storage and provideClarification-based resumption added; several deprecated tool enum members removed.

Changes

Cohort / File(s) Summary
Frontend — Clarification option shape
frontend/src/components/EnhancedReasoning.tsx, frontend/src/hooks/useChatStream.ts
ClarificationOption type removed; ClarificationRequest.options now string[]; UI renders raw option values and passes stringified index as selected option id; event handling uses selectedOption / selectedOption parsing.
JAF types & providers
server/api/chat/agents.ts, server/api/chat/jaf-adapter.ts, server/api/chat/jaf-stream.ts
Renamed public generic/context type JAFAdapterCtxJAFCtx and propagated to JAFAgent, JAFRunConfig, JAFRunState, makeXyneJAFProvider, runStream signatures, and baseCtx variables.
Clarification storage & resumption
server/api/chat/agents.ts, server/api/chat/jaf-stream.ts
Added in-memory clarification storage to JAF run configuration; switched resumption flow to call provideClarification with selectedOption/customInput to resume runs; logs/events updated to use selectedOption.
Tool enum & imports cleanup
server/shared/types.ts, server/api/chat/mapper.ts
Removed multiple deprecated XyneTools members and added requestUserClarification; cleaned up unused imports from vespa-ts.
Miscellaneous formatting/flow tweaks
frontend/src/components/EnhancedReasoning.tsx, server/api/chat/*
Minor conditional/formatting adjustments and small logging/attribute formatting changes; no semantic change beyond above items.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Frontend
    participant JafStream
    participant JAF as JAF Runtime
    participant ClarStore as ClarificationStorage

    Client->>Frontend: User triggers reasoning
    Frontend->>JafStream: start runStream (with clarificationStorage)
    JafStream->>JAF: runStream(...)
    JAF-->>JafStream: interruption (needs clarification)
    JafStream->>Frontend: emit clarification request (options: string[])
    Frontend->>Client: show options (raw strings)

    rect rgb(200,230,200)
      Client->>Frontend: select option (index or custom)
      Frontend->>JafStream: send selectedOption (string) / selectedIdValue
      JafStream->>ClarStore: store clarification result
      JafStream->>JAF: provideClarification(selectedOption)
    end

    JAF->>JafStream: resumedRunState
    JafStream->>JAF: continue streaming
    JafStream->>Frontend: stream resumed results
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Potential focus areas:

  • Verify all occurrences of JAFAdapterCtx were renamed to JAFCtx and type generics align.
  • Confirm clarity of the ClarificationRequest.options type change across front/back boundaries and event payload naming (selectedOption).
  • Ensure removed XyneTools members are not referenced elsewhere.
  • Validate provideClarification call uses correct selected value (customInput vs selectedOption).

Possibly related PRs

  • #1205 — Closely related: touches HITL clarification flow and the same files (EnhancedReasoning, useChatStream, jaf-stream); overlaps in selectedId → selectedOption and JAFCtx changes.
  • #1219 — Related: modifies clarification and reasoning paths including enum member requestUserClarification and payload naming.
  • #616 — Related: changes to XyneTools enum and tool-handling code that overlap with removed tool members here.

Suggested reviewers

  • zereraz
  • shivamashtikar
  • kalpadhwaryu
  • devesh-juspay

Poem

🐰🌿 I hopped through code with nimble paws,
Strings replaced objects without a pause,
JAFAdapterCtx renamed to JAFCtx bright,
Clarifications stored and resumed in flight,
Deprecated tools buried — onward, we soar!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective of the changeset: adding clarification storage support for HITL (human-in-the-loop) workflows, which is a central change across multiple files including agents.ts and jaf-stream.ts.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add_clarification_storage

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @junaid-shirur, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the agentic system's Human-in-the-Loop (HITL) clarification capabilities by standardizing its storage and simplifying its frontend interaction. It also includes significant refactoring of the JAF context and stream resumption logic, alongside a comprehensive cleanup of unused tool definitions and imports, contributing to a more robust and maintainable codebase.

Highlights

  • Standardized HITL Clarification Storage: Implemented a standardized in-memory storage mechanism for Human-in-the-Loop (HITL) clarifications using createInMemoryClarificationStorage from the @xynehq/jaf library, ensuring consistent handling of user clarifications across the agentic system.
  • Simplified Clarification Option Handling: Refactored the frontend to simplify how clarification options are presented and selected. The ClarificationRequest interface now uses a string[] for options instead of ClarificationOption[], streamlining the data structure and rendering logic in EnhancedReasoning.tsx.
  • Refactored JAF Context and Stream Resumption: Renamed the JAF context type from JAFAdapterCtx to JAFCtx for clarity and consistency. The logic for resuming the JAF stream after a user clarification has been significantly simplified by utilizing the new provideClarification utility, removing complex manual state manipulation.
  • Codebase Cleanup and Deprecation: Performed a substantial cleanup by removing deprecated and unused tool definitions from the XyneTools enum and associated imports in both frontend and backend files. This includes various old search, Slack, and user info tools, reducing technical debt and improving code maintainability.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant and beneficial refactoring of the Human-in-the-Loop (HITL) clarification logic. By incorporating an in-memory clarification storage and utilizing the provideClarification helper from the @xynehq/jaf library, the server-side code for resuming agent execution is now much simpler and more maintainable. The changes also include simplifying the data model for clarification options on the frontend and cleaning up deprecated tool definitions, which improves overall code health. My review includes one suggestion to align with React best practices for list keys to prevent potential UI bugs.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
frontend/src/components/EnhancedReasoning.tsx (2)

410-421: Iteration rendering behavior change (no substeps) and icon selection

Two small behavior notes here:

  • Returning early when !hasSubsteps on an iteration means attempts with zero substeps won’t render at all. That’s probably fine (no-op iterations), but worth keeping in mind if upstream ever emits empty iterations you’d still want to expose (e.g., “Attempt 1 started but aborted”).
  • Passing toolName into getAppIcon and using planning/first-step → Brain, then app icon, then tool icon is a clear priority order and matches the new XyneTools/App mappings.

No correctness issues, just documenting the intentional UX change around empty iterations.

Also applies to: 518-520


1112-1123: Clarification UI now assumes string options; consider slightly more stable keys

Using clarificationRequest.options: string[] and passing idx as selectedOptionId with the raw option string as the label matches the updated HITL model and the hook’s provideClarification signature. The only minor nit is key={idx}; if options were ever re-ordered or repeated, you might prefer something like key={`${idx}-${option}`} for a bit more stability, but it’s not functionally required here.

server/shared/types.ts (1)

701-710: XyneTools: new requestUserClarification entry aligns with frontend usage

The trimmed XyneTools enum with the new requestUserClarification = "request_user_clarification" member matches how EnhancedReasoning maps tool names to icons (including the Brain icon for this one). Assuming usages of the removed members have been cleaned up elsewhere, this looks consistent.

server/api/chat/agents.ts (1)

919-921: Eliminate redundant date computation.

dateForAIForAgentSteps duplicates the dateForAI computed at line 676 with the same userTimezone. Reuse the existing dateForAI variable to avoid unnecessary computation.

Apply this diff:

-        const dateForAIForAgentSteps = getDateForAI({
-          userTimeZone: userTimezone,
-        })
-
         const agentSteps = new AgentSteps({
           stream,
           thinking: thinkingRef,
-          dateForAI: dateForAIForAgentSteps,
+          dateForAI: dateForAI,
           actualModelId: actualModelId || null,
         })
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc65df7 and eff2e13.

📒 Files selected for processing (7)
  • frontend/src/components/EnhancedReasoning.tsx (6 hunks)
  • frontend/src/hooks/useChatStream.ts (2 hunks)
  • server/api/chat/agents.ts (12 hunks)
  • server/api/chat/jaf-adapter.ts (2 hunks)
  • server/api/chat/jaf-stream.ts (10 hunks)
  • server/api/chat/mapper.ts (1 hunks)
  • server/shared/types.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-11T08:44:08.412Z
Learnt from: MayankBansal2004
Repo: xynehq/xyne PR: 719
File: server/api/chat/chat.ts:4023-4029
Timestamp: 2025-08-11T08:44:08.412Z
Learning: In server/api/chat/chat.ts and similar files, MayankBansal2004 prefers not to add explicit type definitions for metadata.usage structures when the receiving array is already typed and default values ensure type safety through the || 0 pattern.

Applied to files:

  • frontend/src/hooks/useChatStream.ts
  • server/api/chat/jaf-adapter.ts
  • server/api/chat/mapper.ts
  • server/shared/types.ts
📚 Learning: 2025-07-28T12:07:13.546Z
Learnt from: vipul-maheshwari
Repo: xynehq/xyne PR: 686
File: server/utils/chainUtils.ts:65-69
Timestamp: 2025-07-28T12:07:13.546Z
Learning: In the chainUtils.ts file, the correct type for message parameters is `SelectMessage[]` rather than `Message[]` when working with conversation message arrays.

Applied to files:

  • frontend/src/hooks/useChatStream.ts
📚 Learning: 2025-11-06T09:49:42.273Z
Learnt from: Asrani-Aman
Repo: xynehq/xyne PR: 1163
File: server/api/chat/agents.ts:3941-3991
Timestamp: 2025-11-06T09:49:42.273Z
Learning: In server/api/chat/agents.ts dual RAG path and similar agent message flows, when `agentForDb.appIntegrations` is parsed, the outer function (agents.ts) only needs to extract high-level routing info like `agentAppEnums`. The detailed per-app selections (`selectedItems`) are intentionally extracted inside the inner RAG functions (`generateAnswerFromDualRag`, `UnderstandMessageAndAnswer` in chat.ts) by parsing the `agentPrompt` parameter. This is the established architectural pattern: outer function handles routing, inner function handles detailed selection parsing via `parseAppSelections(agentPromptData.appIntegrations)` and passes `selectedItem` to `searchVespaAgent` for per-app item scoping.

Applied to files:

  • server/api/chat/mapper.ts
  • server/api/chat/agents.ts
📚 Learning: 2025-05-28T10:47:41.020Z
Learnt from: naSim087
Repo: xynehq/xyne PR: 484
File: server/integrations/google/sync.ts:222-222
Timestamp: 2025-05-28T10:47:41.020Z
Learning: The functions `handleGoogleDriveChange` and `getDriveChanges` in `server/integrations/google/sync.ts` are intentionally exported for future changes, even though they are not currently being imported by other modules.

Applied to files:

  • server/api/chat/mapper.ts
📚 Learning: 2025-06-16T11:56:22.752Z
Learnt from: oindrila-b
Repo: xynehq/xyne PR: 545
File: server/integrations/google/index.ts:137-145
Timestamp: 2025-06-16T11:56:22.752Z
Learning: In server/integrations/google/index.ts, both Logger (base logger) and loggerWithChild (factory for email-scoped child loggers) are intentionally maintained to handle different logging scenarios - one for when email context is not available and one for when it is available.

Applied to files:

  • server/api/chat/mapper.ts
📚 Learning: 2025-05-28T10:55:46.701Z
Learnt from: naSim087
Repo: xynehq/xyne PR: 484
File: server/integrations/google/gmail-worker.ts:293-294
Timestamp: 2025-05-28T10:55:46.701Z
Learning: There are two separate `parseMail` functions in the codebase: one in `server/integrations/google/gmail-worker.ts` with signature `(email, gmail, client, userEmail)` returning `{ mailData, insertedPdfCount, exist }`, and another in `server/integrations/google/gmail/index.ts` with signature `(email, gmail, userEmail, client, tracker?)` returning `{ mailData, exist }`. Each file calls its own local version correctly.

Applied to files:

  • server/api/chat/mapper.ts
🧬 Code graph analysis (1)
server/api/chat/jaf-adapter.ts (1)
frontend/src/components/workflow/Types.ts (1)
  • Tool (58-63)
🔇 Additional comments (14)
server/api/chat/mapper.ts (1)

33-33: LGTM! Clean import optimization.

The import has been appropriately streamlined to include only Apps, which is the only type from this module used in the file (consistently as Apps.Github throughout). This reduces the import surface and improves code clarity.

frontend/src/hooks/useChatStream.ts (1)

33-38: Clarification request/response wiring looks consistent with backend changes

ClarificationRequest.options as string[] and the ClarificationProvided handler parsing { clarificationId, selectedOption } align with the new JAF-side payload (options: string[] and selectedOption), and state flags (waitingForClarification, clarificationRequest, isStreaming) are cleared in a strictly idempotent way from both the SSE handler and provideClarification. I don’t see correctness issues here.

Also applies to: 652-659

frontend/src/components/EnhancedReasoning.tsx (1)

206-228: Reasoning step enhancement and tool name propagation look sound

The added isToolStep && step.toolName branch that hoists the tool name to the iteration and backfills existing substeps without app/toolName gives you more consistent icons and metadata without changing the external step shape. Normalizing consolidated summary steps to set both stepSummary and aiGeneratedSummary from whichever is present also keeps downstream display logic simple. I don’t see any functional regressions in this parsing path.

Also applies to: 266-272

server/api/chat/jaf-adapter.ts (1)

11-21: JAFCtx and Tool generics update are consistent and non-breaking

Defining JAFCtx here and switching ToolSchemaParameters and buildMCPJAFTools to Tool<unknown, JAFCtx> keeps the MCP/JAF adapter aligned with the rest of the JAFCtx migration. Since execute doesn’t inspect context, this is a pure typing/interop change with no runtime impact.

Also applies to: 67-71

server/shared/types.ts (1)

118-126: UserMetadata re-export tweak is benign

Adding the trailing comma on UserMetadata in the schema export is syntactic only; the re-exported type surface is unchanged.

server/api/chat/jaf-stream.ts (3)

37-45: JAFCtx migration and runStream generics are coherent

Switching JafStreamer to JAFRunState<JAFCtx>, JAFRunConfig<JAFCtx>, baseCtx: JAFCtx, and runStream<JAFCtx, string> lines up cleanly with the new JAFCtx definition in jaf-adapter.ts and keeps the JAF context typing uniform across adapter, agents, and stream. Since the context isn’t manipulated here beyond being passed through to tools/fallback, this is a safe type-level refactor.

Also applies to: 50-56, 99-104, 138-154


176-213: Improved telemetry and tool result messaging around tool calls

The added span attributes (tool_calls_count, per-call tool_selection span, image_citations_count) and the new toolStatus derivation in tool_call_end are all robust:

  • toolStatus is clamped to "completed" | "failed" | "in_progress", with any unexpected string or non-string defaulting to "completed", which keeps it compatible with the status type on reasoning steps.
  • contextsLength read from toolContextsMap (defaulting to 0) is safely folded into resultSummary and itemsFound.

These are good observability improvements without behavior risk.

Also applies to: 230-247, 316-323


400-421: Verify JAF API contract for clarification third parameter

The review comment's concern appears valid but cannot be definitively resolved without access to the @xynehq/jaf package documentation. Here's what the codebase shows:

  • selectedIdValue at line 636–637 is computed as selectedOption.customInput || selectedOption.selectedOption
  • However, the selectedOption object also contains an selectedOptionId field (per the type definition at server/api/chat/jaf-stream.ts:563–567)
  • The variable name suggests an ID is expected, but the value passed is text (the option label or custom input), not the ID

Public documentation for @xynehq/jaf.provideClarification is not available in search results, and no test files in the repository demonstrate the intended usage pattern.

You should verify: Does provideClarification's third parameter expect the selected option's ID (from selectedOptionId), or the value/text (from selectedOption or customInput)? If it expects the ID, line 636–637 should be corrected to pass selectedOption.selectedOptionId. This applies to both code locations mentioned in the review (lines 400–421 and 606–647).

server/api/chat/agents.ts (6)

155-155: LGTM: Import additions for clarification storage and context type.

The new imports support the HITL clarification storage functionality and the JAFCtx type refactor.

Also applies to: 162-162


1381-1412: LGTM: Tool execution context properly updated for new JAF structure.

The onAfterToolExecution callback correctly handles the updated context structure with state: JAFRunState<JAFCtx> and safely accesses the status with appropriate type handling.


1501-1501: LGTM: Span variable declaration for JAF streaming.

The turnSpan variable is properly declared and passed to JafStreamer for span management during JAF execution.


1299-1299: JAFCtx type refactor verified as complete across all related modules.

The type rename from JAFAdapterCtx to JAFCtx is consistent throughout the file and properly propagated across jaf-adapter.ts and jaf-stream.ts with no remaining legacy references.


327-333: No action required—all call sites have been properly updated.

The logAndStreamReasoning method signature now includes a second userQuery parameter, and verification confirms all 14 call sites across server/api/chat/agents.ts and server/api/chat/jaf-stream.ts have been updated to pass the required second argument.


1369-1380: No issues found. The in‑memory providers in @xynehq/jaf are intended for development/testing and are cleaned up by normal GC; there's no required external cleanup API to call. The clarificationStorage created at line 1371 requires no explicit disposal, and the current implementation is correct.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
server/api/chat/agents.ts (1)

1349-1356: Simplify the agenticModelId logic for maintainability.

The nested ternary operators make this logic difficult to read and verify. Consider refactoring to use explicit if-else statements or extracting the logic into a helper function.

Apply this diff to improve readability:

-        const agenticModelId: Models =
-          actualModelId === defaultBestModelAgenticMode ||
-          actualModelId === defaultBestModel
-            ? (actualModelId as Models)
-            : defaultBestModelAgenticMode &&
-                defaultBestModelAgenticMode !== ("" as Models)
-              ? (defaultBestModelAgenticMode as Models)
-              : defaultBestModel
+        // Use actualModelId if it's already one of the agentic models
+        let agenticModelId: Models
+        if (actualModelId === defaultBestModelAgenticMode || 
+            actualModelId === defaultBestModel) {
+          agenticModelId = actualModelId as Models
+        } else if (defaultBestModelAgenticMode && 
+                   defaultBestModelAgenticMode !== ("" as Models)) {
+          agenticModelId = defaultBestModelAgenticMode as Models
+        } else {
+          agenticModelId = defaultBestModel
+        }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eff2e13 and 7f9ff80.

📒 Files selected for processing (1)
  • server/api/chat/agents.ts (13 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T09:49:42.273Z
Learnt from: Asrani-Aman
Repo: xynehq/xyne PR: 1163
File: server/api/chat/agents.ts:3941-3991
Timestamp: 2025-11-06T09:49:42.273Z
Learning: In server/api/chat/agents.ts dual RAG path and similar agent message flows, when `agentForDb.appIntegrations` is parsed, the outer function (agents.ts) only needs to extract high-level routing info like `agentAppEnums`. The detailed per-app selections (`selectedItems`) are intentionally extracted inside the inner RAG functions (`generateAnswerFromDualRag`, `UnderstandMessageAndAnswer` in chat.ts) by parsing the `agentPrompt` parameter. This is the established architectural pattern: outer function handles routing, inner function handles detailed selection parsing via `parseAppSelections(agentPromptData.appIntegrations)` and passes `selectedItem` to `searchVespaAgent` for per-app item scoping.

Applied to files:

  • server/api/chat/agents.ts
🧬 Code graph analysis (1)
server/api/chat/agents.ts (7)
server/utils/index.ts (1)
  • getDateForAI (3-29)
server/api/chat/agentSteps.ts (1)
  • AgentSteps (32-354)
server/api/chat/jaf-adapter.ts (1)
  • JAFCtx (11-16)
server/api/chat/jaf-provider.ts (1)
  • makeXyneJAFProvider (36-236)
server/ai/agentPrompts.ts (1)
  • hitlClarificationDescription (2316-2340)
server/api/chat/types.ts (1)
  • MinimalAgentFragment (95-101)
server/shared/types.ts (1)
  • Span (57-57)
🪛 GitHub Actions: TypeScript Build Check
server/api/chat/agents.ts

[error] 155-155: Module '"@xynehq/jaf"' has no exported member 'createInMemoryClarificationStorage'.


[error] 1391-1391: Object literal may only specify known properties, and 'clarificationStorage' does not exist in type 'RunConfig'.

🔇 Additional comments (4)
server/api/chat/agents.ts (4)

1303-1303: LGTM: JAFCtx type refactor is consistent.

The refactor from JAFAdapterCtx to JAFCtx is applied consistently throughout the file, and the type is correctly imported from @/api/chat/jaf-adapter. The type definition exists and matches the usage pattern.

Also applies to: 1357-1357, 1364-1364, 1366-1366, 1370-1370, 1398-1398


1410-1423: LGTM: Formatting improvements maintain logic.

The reformatting of the attribute setting code improves readability while preserving the original logic. The confidence calculation and attribute assignments remain functionally identical.


155-155: The JAF library exports createInMemoryProvider, not createInMemoryClarificationStorage. The code at line 155 is importing the wrong function name.

Correct the import statement: Replace createInMemoryClarificationStorage with createInMemoryProvider

Line 155 should import:

createInMemoryProvider,

Line 1382 should call:

const clarificationStorage = createInMemoryProvider()
⛔ Skipped due to learnings
Learnt from: Asrani-Aman
Repo: xynehq/xyne PR: 1163
File: server/api/chat/agents.ts:3941-3991
Timestamp: 2025-11-06T09:49:42.273Z
Learning: In server/api/chat/agents.ts dual RAG path and similar agent message flows, when `agentForDb.appIntegrations` is parsed, the outer function (agents.ts) only needs to extract high-level routing info like `agentAppEnums`. The detailed per-app selections (`selectedItems`) are intentionally extracted inside the inner RAG functions (`generateAnswerFromDualRag`, `UnderstandMessageAndAnswer` in chat.ts) by parsing the `agentPrompt` parameter. This is the established architectural pattern: outer function handles routing, inner function handles detailed selection parsing via `parseAppSelections(agentPromptData.appIntegrations)` and passes `selectedItem` to `searchVespaAgent` for per-app item scoping.

328-334: Signature verification confirms all calls are correct. The logAndStreamReasoning method signature is properly defined as (reasoningStep: AgentReasoningStep, userQuery: string): Promise<void>, and all calls throughout the file (lines 328-334, 442-448, 934-940, 1111-1117, 1255-1270, 1280-1287) pass the appropriate object shapes matching the AgentReasoningStep union type, with message correctly provided as the second parameter.

Comment on lines +1381 to +1391
// Create clarification storage for HITL
const clarificationStorage = createInMemoryClarificationStorage()

const runCfg: JAFRunConfig<JAFCtx> = {
agentRegistry,
modelProvider,
maxTurns: 10,
modelOverride: agenticModelId,
allowClarificationRequests: true,
clarificationDescription: hitlClarificationDescription,
clarificationStorage,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: clarificationStorage property not recognized in RunConfig type.

The pipeline indicates that clarificationStorage does not exist in RunConfig<JAFCtx>. This suggests either:

  1. The @xynehq/jaf library version doesn't support this property yet
  2. The property name or type is incorrect
  3. The library needs to be updated

This blocks compilation.

Verify the JAF library version and RunConfig type definition:


🏁 Script executed:

#!/bin/bash
# Check installed JAF version
npm list @xynehq/jaf

# Search for RunConfig type definition in node_modules
fd -t f "index.d.ts" node_modules/@xynehq/jaf -x cat {} | grep -A 20 "interface RunConfig" || echo "Type definition not found in node_modules"

# Check if clarificationStorage is mentioned anywhere in JAF types
fd -t f -e ts -e d.ts . node_modules/@xynehq/jaf -x rg "clarificationStorage|ClarificationStorage" {}

Length of output: 297


🏁 Script executed:

#!/bin/bash
# Check package.json for JAF dependency
grep -A 2 -B 2 "@xynehq/jaf" package.json || echo "JAF not found in package.json"

# Look at imports in agents.ts
head -50 server/api/chat/agents.ts | grep -E "import|from" | head -20

# Search for RunConfig definition in the codebase
rg -n "interface RunConfig|type RunConfig" --type ts --type tsx

# Look for createInMemoryClarificationStorage import/definition
rg -n "createInMemoryClarificationStorage" --type ts --type tsx -B2 -A2

# Search for clarificationStorage references
rg -n "clarificationStorage" server/api/chat/agents.ts -B2 -A2

Length of output: 1202


🏁 Script executed:

#!/bin/bash
# Search for imports around createInMemoryClarificationStorage
rg -n "createInMemoryClarificationStorage|ClarificationStorage" --type ts -A2 -B2

# Look for JAFRunConfig type and imports
rg -n "JAFRunConfig|from.*jaf" --type ts -A1 -B1

# Get more context around line 1380-1391 in agents.ts
sed -n '1370,1400p' server/api/chat/agents.ts

# Check all imports at top of agents.ts
sed -n '1,100p' server/api/chat/agents.ts | rg "import"

Length of output: 8480


🌐 Web query:

@xynehq/jaf clarificationStorage JAFRunConfig property

💡 Result:

I can't find any reference to a RunConfig property named clarificationStorage in the public JAF docs or repo snapshots. Do you want me to:

  • search the xynehq/jaf GitHub repo (or a specific branch/file) for "clarificationStorage", or
  • look for a similarly named property (e.g., clarification, clarificationStore) in the docs/code?

Tell me which you'd prefer and I'll run the search and cite the results.


🌐 Web query:

xynehq/jaf GitHub RunConfig interface type definition

💡 Result:

The RunConfig type (TypeScript) in xynehq/jaf is:

export type RunConfig = {
readonly agentRegistry: ReadonlyMap<string, Agent<Ctx, any>>;
readonly modelProvider: ModelProvider;
readonly maxTurns?: number;
readonly modelOverride?: string;
readonly initialInputGuardrails?: readonly Guardrail[];
readonly finalOutputGuardrails?: readonly Guardrail[];
readonly onEvent?: (event: TraceEvent) => void;
readonly memory?: MemoryConfig;
readonly conversationId?: string;
}. [1][2]

Sources:

  • JAF Core Concepts — RunConfig type definition. [1]
  • JAF Server API docs (ServerConfig referencing RunConfig). [2]

Critical: clarificationStorage and related properties are not supported in the current JAFRunConfig type.

The RunConfig type from @xynehq/jaf does not include clarificationStorage, allowClarificationRequests, clarificationDescription, or onAfterToolExecution properties. The code at lines 1385-1391 attempts to add these unsupported properties, which will cause a TypeScript compilation error.

Either use a different API for HITL configuration, downgrade to a JAF version that supports these properties, or remove these properties if they're not required.

🧰 Tools
🪛 GitHub Actions: TypeScript Build Check

[error] 1391-1391: Object literal may only specify known properties, and 'clarificationStorage' does not exist in type 'RunConfig'.

🤖 Prompt for AI Agents
In server/api/chat/agents.ts around lines 1381-1391, the JAFRunConfig type from
@xynehq/jaf does not include clarificationStorage, allowClarificationRequests,
clarificationDescription, or onAfterToolExecution, so adding them causes a
TypeScript error; fix it by either (A) removing those unsupported properties
from runCfg and moving HITL-specific setup into the correct HITL API call or
separate config object accepted by the library, or (B) if you intentionally need
to keep them, define and use a new interface that extends JAFRunConfig with
these HITL fields (or cast the config to unknown as any) and ensure the runtime
consumer expects that extended shape, or (C) update/downgrade @xynehq/jaf to a
version that exposes these properties and adjust imports accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant