feat(agent): add debug logging / enhance chat history handling in com…#15
feat(agent): add debug logging / enhance chat history handling in com…#15
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRestructured composition chain error handling into a single outer try/catch, added chat history formatting to the composition prompt inputs, and added a debug console.log for the formatted chat history in the worker runner. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent
participant Composition as CompositionChain
participant LLM as OpenAI LLM
participant Gemini as GeminiFallback
Agent->>Composition: invoke with AgentState (includes context.history)
Composition->>Composition: format chatHistory from state.context?.history
Composition->>LLM: call primary LLM invocation with prompt (includes chatHistory)
alt LLM rate/quota error
LLM-->>Composition: error (rate/quota)
Composition->>Gemini: retry with Gemini model
Gemini-->>Composition: response
else success
LLM-->>Composition: response
end
Composition-->>Agent: return composed safe-fallback or LLM response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/worker/src/agent/runner.ts (2)
80-81: Remove debug logging before merging or replace with structured logger.Debug
console.logstatements with emojis and mixed-language text should not be committed to production code. The comment acknowledges this is for debugging.Consider either:
- Removing before merge
- Using a proper logger with configurable log levels (e.g.,
logger.debug())♻️ Option 1: Remove debug log
- // Debug log to verify history formatting - can be removed in production - console.log('🔥 [DEBUG] HISTORY DARI DATABASE:', JSON.stringify(formattedHistory, null, 2)); - const { processMessage } = await import('@wa-chat/llm');♻️ Option 2: Use structured logging
- // Debug log to verify history formatting - can be removed in production - console.log('🔥 [DEBUG] HISTORY DARI DATABASE:', JSON.stringify(formattedHistory, null, 2)); + // Use DEBUG level so this only appears when explicitly enabled + logger.debug('Formatted chat history for agent pipeline', { + conversationId: input.conversationId, + historyLength: formattedHistory.length + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/worker/src/agent/runner.ts` around lines 80 - 81, Remove the development console.log in runner.ts that prints formattedHistory (console.log('🔥 [DEBUG] HISTORY DARI DATABASE:', JSON.stringify(formattedHistory, null, 2))); either delete the line or replace it with a structured logger call (e.g., logger.debug) that respects log levels; locate the statement referencing formattedHistory in the agent/runner.ts file and switch to the project's logger utility or remove the debug output prior to merging so no informal console output remains in production.
75-78: Unsafe type cast is misleading but doesn't cause runtime failures as claimed.The
as unknown as BaseMessage[]cast creates plain objects{role, content}that don't implement theBaseMessageclass. However, composition.ts (line 73) re-casts this data back to{ role?: string; content?: string }[]and only accesses theroleandcontentproperties—it never callsBaseMessagemethods. The code works despite the unsafe cast.That said, the cast is deceptive and bypasses type checking unnecessarily. Using
HumanMessageandAIMessageconstructors would be clearer and more maintainable:🔧 Suggested improvement
+import { HumanMessage, AIMessage } from '@langchain/core/messages';- const formattedHistory = rawHistory.map((message) => ({ - role: message.direction === 'inbound' ? 'user' : 'assistant', - content: message.body, - })) as unknown as BaseMessage[]; + const formattedHistory: BaseMessage[] = rawHistory.map((message) => + message.direction === 'inbound' + ? new HumanMessage(message.body) + : new AIMessage(message.body) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/worker/src/agent/runner.ts` around lines 75 - 78, The current unsafe cast creating plain objects as BaseMessage[] is misleading; replace the mapping that creates formattedHistory from rawHistory so it constructs proper message instances using the HumanMessage and AIMessage constructors (e.g., new HumanMessage(message.body) or new AIMessage(message.body)) based on message.direction, remove the "as unknown as BaseMessage[]" cast, and ensure formattedHistory remains typed as BaseMessage[]; update imports to include HumanMessage and AIMessage and keep downstream usage (e.g., composition.ts) unchanged.
🤖 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/llm/src/langchain/chains/composition.ts`:
- Around line 121-125: Replace the grammatically incorrect user-facing fallback
message in the safeFallback object (variable safeFallback in composition.ts)
with a correct, clear sentence; update the content string from "System have some
trouble." to a proper phrasing such as "The system is experiencing an issue." or
"The system is having trouble." so end users see a professional message while
keeping the other fields (confidence, escalate_flag) unchanged.
- Around line 73-81: The chat history mapping uses a non-existent msg.role which
causes every message to be labeled "Assistant"; update the logic in the
composition chain where history and formattedChatHistory are computed to read
BaseMessage.type instead of role (map 'human' -> 'User', 'ai' -> 'Assistant',
'system' -> 'System' and default to a safe label), ensure the history variable
is typed as BaseMessage[] (or the appropriate type from
`@langchain/core/messages`) so TypeScript catches misuse, and defensively handle
non-string content values as the existing code does.
---
Nitpick comments:
In `@apps/worker/src/agent/runner.ts`:
- Around line 80-81: Remove the development console.log in runner.ts that prints
formattedHistory (console.log('🔥 [DEBUG] HISTORY DARI DATABASE:',
JSON.stringify(formattedHistory, null, 2))); either delete the line or replace
it with a structured logger call (e.g., logger.debug) that respects log levels;
locate the statement referencing formattedHistory in the agent/runner.ts file
and switch to the project's logger utility or remove the debug output prior to
merging so no informal console output remains in production.
- Around line 75-78: The current unsafe cast creating plain objects as
BaseMessage[] is misleading; replace the mapping that creates formattedHistory
from rawHistory so it constructs proper message instances using the HumanMessage
and AIMessage constructors (e.g., new HumanMessage(message.body) or new
AIMessage(message.body)) based on message.direction, remove the "as unknown as
BaseMessage[]" cast, and ensure formattedHistory remains typed as BaseMessage[];
update imports to include HumanMessage and AIMessage and keep downstream usage
(e.g., composition.ts) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d163476-4030-4825-a545-a1f5f09f7e54
📒 Files selected for processing (2)
apps/worker/src/agent/runner.tspackages/llm/src/langchain/chains/composition.ts
| const safeFallback = { | ||
| content: 'System have some trouble.', | ||
| confidence: 0, | ||
| escalate_flag: true, | ||
| }; |
There was a problem hiding this comment.
Fix grammatical error in user-facing fallback message.
The message "System have some trouble." is grammatically incorrect and will be shown to end users.
✏️ Proposed fix
const safeFallback = {
- content: 'System have some trouble.',
+ content: 'The system is experiencing some trouble. Please try again later.',
confidence: 0,
escalate_flag: true,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const safeFallback = { | |
| content: 'System have some trouble.', | |
| confidence: 0, | |
| escalate_flag: true, | |
| }; | |
| const safeFallback = { | |
| content: 'The system is experiencing some trouble. Please try again later.', | |
| confidence: 0, | |
| escalate_flag: true, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/llm/src/langchain/chains/composition.ts` around lines 121 - 125,
Replace the grammatically incorrect user-facing fallback message in the
safeFallback object (variable safeFallback in composition.ts) with a correct,
clear sentence; update the content string from "System have some trouble." to a
proper phrasing such as "The system is experiencing an issue." or "The system is
having trouble." so end users see a professional message while keeping the other
fields (confidence, escalate_flag) unchanged.
…d improve formatting
…position chain
Summary by CodeRabbit
Refactor
Chores