feat: agent context features and chat improvements#454
Conversation
Allow each builtin agent to declare which context features it needs (e.g., integrations) so the beforeContext hook can skip unnecessary queries for agents that don't use them.
…nation Merge duplicate beforeGenerateHook implementations into shared lib/agent_chat module and remove the redundant onError hook. Add a tools summary section to the context window with token estimation for system prompt and tools. Fix thread listing to paginate through all pages instead of only the first 100, and sort threads client-side by creation time. Switch customer delete from collection API to direct mutation.
Add AssistantMessageInfo component to test chat panel so users can inspect message metadata. Replace reset icon with Trash2 and implement proper chat reset by unmounting/remounting the panel. Style h3 headings in context window content for better readability.
📝 WalkthroughWalkthroughThis PR refactors the agent chat and custom agents systems while improving UI components. Key changes include: (1) introducing a contextFeatures system that enables selective feature loading in agent hooks, (2) removing onError hooks from the agent response pipeline, (3) adding a toolsSummary to the context window display without sending it to the LLM, (4) reorganizing internal action hooks and their references across agent variants, (5) converting customer deletion from collection-based to Convex mutation-based operations, (6) enhancing the test chat panel with message metadata display via a new AssistantMessageInfo component, (7) fixing thread pagination to fetch all pages, and (8) updating message dialog styling with CSS adjustments. Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/convex/agents/chat/internal_actions.ts (1)
132-132: 🧹 Nitpick | 🔵 TrivialRedundant nullish coalescing operator.
integrationsListis already typed asRecord<string, unknown>[] | undefined, so?? undefinedis a no-op.♻️ Suggested simplification
return { integrationsInfo, - integrationsList: integrationsList ?? undefined, + integrationsList, };
🤖 Fix all issues with AI agents
In `@services/platform/app/features/chat/hooks/queries.ts`:
- Around line 22-28: Tests are calling the useThreads hook directly instead of
via a React hook wrapper; update collection-hooks.test.ts to import renderHook
from '@testing-library/react' and replace direct calls like
useThreads(mockCollection as never) with const { result } = renderHook(() =>
useThreads(mockCollection as never)); then use result.current to access threads
in assertions (update all occurrences and any destructuring that assumed a
direct return).
In `@services/platform/app/features/custom-agents/components/test-chat-panel.tsx`:
- Around line 525-532: The handler handleClearChat currently calls onReset
unconditionally after attempting deleteChatThread, so onReset should be invoked
only when the deletion succeeds; update handleClearChat to await or chain the
deleteChatThread call (using async/await or .then) and move the onReset?.()
invocation into the success branch, while logging/handling errors in the catch
branch and not calling onReset there; reference the handleClearChat function and
the deleteChatThread and onReset symbols when making the change.
- Around line 114-142: The IconButton in AssistantMessageInfo currently uses a
hardcoded aria-label "Message info"; replace this with a localized string by
using the project's translation hook/function (e.g., call useTranslation()/t or
useIntl().formatMessage) inside AssistantMessageInfo and pass the translated
label to the IconButton aria-label prop; ensure you import the appropriate i18n
hook at the top of the component file and use a clear key like
"assistant.messageInfo" when calling the translation function.
In `@services/platform/convex/threads/list_threads.ts`:
- Around line 32-61: Add a defensive iteration cap to the pagination loop in
list_threads.ts to prevent an infinite loop if result.isDone is never true:
track an iteration counter inside the for(;;) loop (near the
paginationOpts/result handling) and break with a logged warning or thrown error
once a reasonable max (e.g., 1000) is reached; reference the pagination
variables cursor, paginationOpts, result.isDone, result.continueCursor and the
target query components.agent.threads.listThreadsByUserId when making the
change.
| const sorted = useMemo( | ||
| () => data?.slice().sort((a, b) => b._creationTime - a._creationTime), | ||
| [data], | ||
| ); | ||
|
|
||
| return { | ||
| threads: data, | ||
| threads: sorted, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the test file structure and how useThreads is called
echo "=== Locating test file ==="
find . -name "collection-hooks.test.ts" -o -name "*useThreads*test*" 2>/dev/null | head -20
echo -e "\n=== Checking for test file at mentioned path ==="
if [ -f "services/platform/app/features/chat/hooks/__tests__/collection-hooks.test.ts" ]; then
echo "File found, examining content..."
wc -l "services/platform/app/features/chat/hooks/__tests__/collection-hooks.test.ts"
else
echo "File not found at exact path, searching..."
find . -path "*/chat/hooks/*test*" -type f 2>/dev/null
fiRepository: tale-project/tale
Length of output: 1467
🏁 Script executed:
#!/bin/bash
# Search for useThreads usage in test files
echo "=== Searching for useThreads in test files ==="
rg -n "useThreads" --glob "**/*.test.ts" --glob "**/*.test.tsx" -A 5 -B 2Repository: tale-project/tale
Length of output: 2975
🏁 Script executed:
#!/bin/bash
# Check if renderHook is imported/used anywhere in tests
echo "=== Checking for renderHook usage ==="
rg "renderHook|@testing-library/react" --glob "**/*.test.ts" --glob "**/*.test.tsx" -nRepository: tale-project/tale
Length of output: 1676
🏁 Script executed:
#!/bin/bash
# Read the complete test file
echo "=== Complete test file content ==="
cat -n services/platform/app/features/chat/hooks/__tests__/collection-hooks.test.tsRepository: tale-project/tale
Length of output: 1656
🏁 Script executed:
#!/bin/bash
# Read the queries.ts file to see useThreads implementation
echo "=== Checking queries.ts file size ==="
wc -l services/platform/app/features/chat/hooks/queries.ts
echo -e "\n=== Full queries.ts content ==="
cat -n services/platform/app/features/chat/hooks/queries.tsRepository: tale-project/tale
Length of output: 12628
🏁 Script executed:
#!/bin/bash
# Check for test run output or CI configuration
echo "=== Looking for vitest configuration ==="
find . -name "vitest.config.*" -o -name "vite.config.*" 2>/dev/null | head -5
echo -e "\n=== Checking if tests directory has any setup files ==="
ls -la services/platform/app/features/chat/hooks/__tests__/ 2>/dev/null || echo "Dir not accessible in this context"Repository: tale-project/tale
Length of output: 485
🏁 Script executed:
#!/bin/bash
# Check vitest configuration
echo "=== vitest.config.mjs content ==="
cat services/platform/vitest.config.mjs
echo -e "\n=== vite.config.ts content ==="
cat services/platform/vite.config.tsRepository: tale-project/tale
Length of output: 4584
🏁 Script executed:
#!/bin/bash
# Check if there are any CI/test runner configurations
echo "=== Checking for CI configuration ==="
find . -name "*.yml" -o -name "*.yaml" 2>/dev/null | grep -i -E "(ci|github|gitlab|test)" | head -10
echo -e "\n=== Checking if tests are run elsewhere ==="
grep -r "collection-hooks.test" . --include="*.yml" --include="*.yaml" --include="*.json" 2>/dev/null | head -5
echo -e "\n=== Checking package.json test scripts ==="
if [ -f "services/platform/package.json" ]; then
jq '.scripts' services/platform/package.json 2>/dev/null | grep -i test
fiRepository: tale-project/tale
Length of output: 1163
🏁 Script executed:
#!/bin/bash
# Check the UI test configuration which may have different exclusions
echo "=== vitest.ui.config.mjs content ==="
cat services/platform/vitest.ui.config.mjs 2>/dev/null || echo "File not found"
echo -e "\n=== Check test.yml CI workflow ==="
cat .github/workflows/test.yml 2>/dev/null | head -100Repository: tale-project/tale
Length of output: 2635
🏁 Script executed:
#!/bin/bash
# Check the setup-ui.ts file to see what React context is provided
echo "=== Checking test setup file ==="
if [ -f "services/platform/test/setup-ui.ts" ]; then
cat services/platform/test/setup-ui.ts
else
echo "File not found at services/platform/test/setup-ui.ts"
find . -name "setup-ui.ts" -type f 2>/dev/null
fiRepository: tale-project/tale
Length of output: 2764
Update tests to use renderHook for hook testing.
The sorting implementation is correct, but the tests call useThreads directly without the proper React hook wrapper. Update services/platform/app/features/chat/hooks/__tests__/collection-hooks.test.ts to wrap hook calls with renderHook from @testing-library/react:
import { renderHook } from '@testing-library/react';
// Change direct calls like:
// const result = useThreads(mockCollection as never);
// To:
// const { result } = renderHook(() => useThreads(mockCollection as never));🧰 Tools
🪛 GitHub Check: Test
[failure] 22-22: app/features/chat/hooks/tests/collection-hooks.test.ts > useThreads > returns data even while loading
TypeError: Cannot read properties of null (reading 'useMemo')
❯ process.env.NODE_ENV.exports.useMemo ../../node_modules/react/cjs/react.development.js:1251:33
❯ Module.useThreads app/features/chat/hooks/queries.ts:22:18
❯ app/features/chat/hooks/tests/collection-hooks.test.ts:41:20
[failure] 22-22: app/features/chat/hooks/tests/collection-hooks.test.ts > useThreads > returns data when loaded
TypeError: Cannot read properties of null (reading 'useMemo')
❯ process.env.NODE_ENV.exports.useMemo ../../node_modules/react/cjs/react.development.js:1251:33
❯ Module.useThreads app/features/chat/hooks/queries.ts:22:18
❯ app/features/chat/hooks/tests/collection-hooks.test.ts:29:20
🤖 Prompt for AI Agents
In `@services/platform/app/features/chat/hooks/queries.ts` around lines 22 - 28,
Tests are calling the useThreads hook directly instead of via a React hook
wrapper; update collection-hooks.test.ts to import renderHook from
'@testing-library/react' and replace direct calls like useThreads(mockCollection
as never) with const { result } = renderHook(() => useThreads(mockCollection as
never)); then use result.current to access threads in assertions (update all
occurrences and any destructuring that assumed a direct return).
| function AssistantMessageInfo({ | ||
| messageId, | ||
| timestamp, | ||
| }: { | ||
| messageId: string; | ||
| timestamp: Date; | ||
| }) { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const { metadata } = useMessageMetadata(messageId); | ||
|
|
||
| return ( | ||
| <div className="flex items-center"> | ||
| <IconButton | ||
| icon={Info} | ||
| aria-label="Message info" | ||
| onClick={() => setIsOpen(true)} | ||
| iconSize={3} | ||
| className="text-muted-foreground hover:text-foreground p-1" | ||
| /> | ||
| <MessageInfoDialog | ||
| isOpen={isOpen} | ||
| onOpenChange={setIsOpen} | ||
| messageId={messageId} | ||
| timestamp={timestamp} | ||
| metadata={metadata} | ||
| /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Internationalize the aria-label for accessibility.
The hardcoded "Message info" string should use the translation function for proper i18n support.
🌐 Proposed fix
function AssistantMessageInfo({
messageId,
timestamp,
}: {
messageId: string;
timestamp: Date;
}) {
const [isOpen, setIsOpen] = useState(false);
const { metadata } = useMessageMetadata(messageId);
+ const { t: tCommon } = useT('common');
return (
<div className="flex items-center">
<IconButton
icon={Info}
- aria-label="Message info"
+ aria-label={tCommon('actions.showInfo')}
onClick={() => setIsOpen(true)}
iconSize={3}
className="text-muted-foreground hover:text-foreground p-1"
/>🤖 Prompt for AI Agents
In `@services/platform/app/features/custom-agents/components/test-chat-panel.tsx`
around lines 114 - 142, The IconButton in AssistantMessageInfo currently uses a
hardcoded aria-label "Message info"; replace this with a localized string by
using the project's translation hook/function (e.g., call useTranslation()/t or
useIntl().formatMessage) inside AssistantMessageInfo and pass the translated
label to the IconButton aria-label prop; ensure you import the appropriate i18n
hook at the top of the component file and use a clear key like
"assistant.messageInfo" when calling the translation function.
| const handleClearChat = useCallback(() => { | ||
| if (threadId) { | ||
| void deleteChatThread({ threadId }).catch((error) => { | ||
| console.error('Error deleting test chat thread:', error); | ||
| }); | ||
| } | ||
| }, [threadId, deleteChatThread]); | ||
| onReset?.(); | ||
| }, [threadId, deleteChatThread, onReset]); |
There was a problem hiding this comment.
onReset is called even if thread deletion fails.
The onReset?.() callback fires unconditionally after the delete attempt, regardless of whether deleteChatThread succeeded or failed. If the deletion fails, this will still remount the component, potentially causing a confusing UX where the user expects a fresh state but the old thread may persist.
Consider calling onReset only on successful deletion:
🛠️ Proposed fix
const handleClearChat = useCallback(() => {
if (threadId) {
- void deleteChatThread({ threadId }).catch((error) => {
- console.error('Error deleting test chat thread:', error);
- });
+ void deleteChatThread({ threadId })
+ .then(() => {
+ onReset?.();
+ })
+ .catch((error) => {
+ console.error('Error deleting test chat thread:', error);
+ });
+ } else {
+ onReset?.();
}
- onReset?.();
}, [threadId, deleteChatThread, onReset]);📝 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 handleClearChat = useCallback(() => { | |
| if (threadId) { | |
| void deleteChatThread({ threadId }).catch((error) => { | |
| console.error('Error deleting test chat thread:', error); | |
| }); | |
| } | |
| }, [threadId, deleteChatThread]); | |
| onReset?.(); | |
| }, [threadId, deleteChatThread, onReset]); | |
| const handleClearChat = useCallback(() => { | |
| if (threadId) { | |
| void deleteChatThread({ threadId }) | |
| .then(() => { | |
| onReset?.(); | |
| }) | |
| .catch((error) => { | |
| console.error('Error deleting test chat thread:', error); | |
| }); | |
| } else { | |
| onReset?.(); | |
| } | |
| }, [threadId, deleteChatThread, onReset]); |
🤖 Prompt for AI Agents
In `@services/platform/app/features/custom-agents/components/test-chat-panel.tsx`
around lines 525 - 532, The handler handleClearChat currently calls onReset
unconditionally after attempting deleteChatThread, so onReset should be invoked
only when the deletion succeeds; update handleClearChat to await or chain the
deleteChatThread call (using async/await or .then) and move the onReset?.()
invocation into the success branch, while logging/handling errors in the catch
branch and not calling onReset there; reference the handleClearChat function and
the deleteChatThread and onReset symbols when making the change.
| for (;;) { | ||
| // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- Convex pagination cursor type | ||
| const paginationOpts = { cursor, numItems: 100 } as { | ||
| cursor: string | null; | ||
| numItems: number; | ||
| }; | ||
| const result = await ctx.runQuery( | ||
| components.agent.threads.listThreadsByUserId, | ||
| { | ||
| userId: args.userId, | ||
| order: 'desc', | ||
| paginationOpts, | ||
| }, | ||
| ); | ||
|
|
||
| for (const thread of result.page) { | ||
| if (thread.status !== 'active') continue; | ||
| if (!isGeneralThread(thread.summary)) continue; | ||
|
|
||
| threads.push({ | ||
| _id: thread._id, | ||
| _creationTime: thread._creationTime, | ||
| title: thread.title, | ||
| status: thread.status, | ||
| userId: thread.userId, | ||
| }); | ||
| } | ||
|
|
||
| if (result.isDone) break; | ||
| cursor = result.continueCursor; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Pagination implementation looks correct.
The loop correctly paginates through all pages, filtering for active general threads. The comment on lines 29-31 clearly explains the rationale for full pagination.
One minor consideration: there's no upper bound on iterations. If result.isDone were never true due to an API bug, this would loop indefinitely. For defensive coding, consider adding a safety limit:
🛡️ Optional: Add iteration safety limit
export async function listThreads(
ctx: QueryCtx,
args: Pick<ListThreadsArgs, 'userId'>,
): Promise<Thread[]> {
const threads: Thread[] = [];
let cursor: string | null = null;
+ const MAX_PAGES = 100; // Safety limit: 10,000 threads max
+ let pageCount = 0;
// Paginate through all pages to collect every general thread.
// Sub-threads consume pagination slots but are filtered out, so a
// single page of 100 can miss general threads for active users.
for (;;) {
+ if (++pageCount > MAX_PAGES) {
+ console.warn('listThreads: reached max page limit');
+ break;
+ }
// oxlint-disable-next-line typescript/no-unsafe-type-assertion -- Convex pagination cursor type🤖 Prompt for AI Agents
In `@services/platform/convex/threads/list_threads.ts` around lines 32 - 61, Add a
defensive iteration cap to the pagination loop in list_threads.ts to prevent an
infinite loop if result.isDone is never true: track an iteration counter inside
the for(;;) loop (near the paginationOpts/result handling) and break with a
logged warning or thrown error once a reasonable max (e.g., 1000) is reached;
reference the pagination variables cursor, paginationOpts, result.isDone,
result.continueCursor and the target query
components.agent.threads.listThreadsByUserId when making the change.
…tations for custom agents Simplify useUpdateCustomAgent and useUpdateCustomAgentMetadata hooks by removing the Collection dependency and using useConvexMutation directly. This removes the need for useCustomAgentCollection across all agent config tabs (general, instructions, tools, knowledge).
1 participant
Summary
contextFeaturesto agent config for selective context loading, allowing each builtin agent to declare which context features it needs so unnecessary queries are skippedbeforeGenerateHookimplementations into sharedlib/agent_chatmodule and remove redundantonErrorhookTest plan
contextFeaturesconfigs only load relevant context🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Improvements