fix: done tool streaming

This commit is contained in:
dal 2025-09-29 21:23:15 -06:00
parent 7fc4d44779
commit b922cf0c85
No known key found for this signature in database
GPG Key ID: 16F4B0E1E9F61122
8 changed files with 247 additions and 68 deletions

View File

@ -1,24 +0,0 @@
# AGENT.md
## Commands
- **Build**: `turbo build` or `turbo run build:dry-run` (type check only)
- **Tests**: `turbo run test:unit` (run before completing tasks), `turbo run test:integration --filter=<package>`
- **Single test**: `turbo run test:unit --filter=<package>` or run test files directly in specific packages
- **Lint**: `turbo lint`, `pnpm run check:fix <path>` (auto-fixes with Biome)
- **Pre-completion check**: `turbo run build:dry-run lint test:unit`
## Architecture
**Monorepo**: pnpm + Turborepo with `@buster/*` packages and `@buster-app/*` apps
- **Apps**: web (Next.js), server (Hono API), trigger (background jobs), electric-server, api (Rust legacy), cli (Rust)
- **Key packages**: ai (Mastra framework), database (Drizzle ORM), server-shared (API types), data-source, access-controls
- **Database**: PostgreSQL with Supabase, soft deletes only (`deleted_at`), queries in `@buster/database/src/queries/`
- **APIs**: Hono with functional handlers, type-safe with Zod schemas in `@buster/server-shared`
## Code Style
- **TypeScript**: Strict mode, no `any`, handle null/undefined explicitly
- **Imports**: Use type-only imports (`import type`), Node.js protocol (`node:fs`)
- **Formatting**: Biome - 2 spaces, single quotes, trailing commas, 100 char width
- **Functions**: Functional/composable over classes, dependency injection, small focused functions
- **Logging**: Never `console.log`, use `console.info/warn/error`
- **Naming**: `@buster/{package}` for packages, `@buster-app/{app}` for apps
- **Error handling**: Comprehensive with strategic logging, soft deletes, upserts preferred

136
AGENTS.md Normal file
View File

@ -0,0 +1,136 @@
# CLAUDE.md
This file provides core guidance to Claude/AI assistants when working with the Buster monorepo.
**Note**: Each package and app has its own CLAUDE.md with specific implementation details. This document contains only universal principles.
## Monorepo Philosophy
### Architecture Principles
1. **Packages are standalone building blocks** - Modular components with minimal cross-dependencies
2. **Apps assemble packages** - Apps piece together package code, never contain business logic directly
3. **Avoid spaghetti dependencies** - Keep clean boundaries between packages
4. **Type flow hierarchy** - Types flow: `database``server-shared``apps`
### Critical Package Boundaries
- **`@buster/database`** - Owns ALL database queries. No direct Drizzle usage elsewhere
- **`@buster/server-shared`** - API contract layer. All request/response types live here
- **`@buster/data-source`** - Isolated data source connection logic for customer databases
- **Package imports** - Packages can use each other but maintain clear, logical dependencies
## Development Principles
### Functional Programming First
- **Pure functions only** - No classes for business logic
- **Composable modules** - Build features by composing small, focused functions
- **Immutable data** - Never mutate; always create new data structures
- **Higher-order functions** - Use functions that return configured functions for dependency injection
- **No OOP** - No classes, no inheritance, no `this` keyword in business logic
### Type Safety Standards
- **Zod-first everything** - Define ALL types as Zod schemas with descriptions
- **Export inferred types** - Always use `z.infer<typeof Schema>` for TypeScript types
- **Runtime validation** - Use `.parse()` for trusted data, `.safeParse()` for user input
- **No implicit any** - Every variable, parameter, and return type must be explicitly typed
- **Constants for strings** - Use const assertions for type-safe string literals
### Testing Philosophy
- **Test-driven development** - Write tests and assertions first, then implement
- **Colocate tests** - Keep `.test.ts` (unit) and `.int.test.ts` (integration) next to implementation
- **Test naming** - If file is `user.ts`, tests are `user.test.ts` and/or `user.int.test.ts`
- **Minimize integration dependencies** - Most logic should be testable with unit tests
- **Test descriptions** - Test names should describe the assertion and situation clearly
## Development Workflow
### Command Standards
**CRITICAL**: Only use Turbo commands. Never use pnpm, npm, or vitest directly.
```bash
# Build commands
turbo build # Build entire monorepo
turbo build --filter=@buster/ai # Build specific package
# Linting
turbo lint # Lint entire monorepo
turbo lint --filter=@buster-app/web # Lint specific app
# Testing
turbo test:unit # Run all unit tests
turbo test:unit --filter=@buster/database # Test specific package
turbo test:integration --filter=@buster/ai # Integration tests for specific package
# Development
turbo dev # Start development servers
```
### Pre-Completion Checklist
Before completing any task:
1. Run `turbo build` - Ensure everything compiles
2. Run `turbo lint` - Fix all linting issues
3. Run `turbo test:unit` - All unit tests must pass
## Code Organization
### File Structure
- **Small, focused files** - Each file has a single responsibility
- **Deep nesting is OK** - Organize into logical subdirectories
- **Explicit exports** - Use named exports and comprehensive index.ts files
- **Functional patterns** - Export factory functions that return configured function sets
### Module Patterns
```typescript
// Good: Functional approach with Zod
import { z } from 'zod';
const UserParamsSchema = z.object({
userId: z.string().describe('Unique user identifier'),
orgId: z.string().describe('Organization identifier')
});
type UserParams = z.infer<typeof UserParamsSchema>;
export function validateUser(params: UserParams) {
const validated = UserParamsSchema.parse(params);
// Implementation
}
// Bad: Class-based approach
class UserService { // Never do this
validateUser() { }
}
```
## Cross-Cutting Concerns
### Environment Variables
- Centralized at root level in `.env` file
- Turbo passes variables via `globalEnv` configuration
- Individual packages validate their required variables
### Database Operations
- ALL queries go through `@buster/database` package
- Never use Drizzle directly outside the database package
- Soft deletes only (use `deleted_at` field)
- Prefer upserts over updates
### API Development
- Request/response types in `@buster/server-shared`
- Import database types through server-shared for consistency
- Validate with Zod at API boundaries
- Use type imports: `import type { User } from '@buster/database'`
## Legacy Code Migration
- **Rust code** (`apps/api`) is legacy and being migrated to TypeScript
- Focus new development on TypeScript patterns
- Follow patterns in `apps/server` for new API development
## Agent Workflows
- Use `planner` agent for spec, plan, ticket, research development workflows.
## Important Reminders
- Do only what has been asked; nothing more, nothing less
- Never create files unless absolutely necessary
- Always prefer editing existing files over creating new ones
- Never proactively create documentation unless explicitly requested
- Check package/app-specific CLAUDE.md files for implementation details

View File

@ -1,5 +1,6 @@
import {
type UpdateMessageEntriesParams,
getAssetLatestVersion,
updateChat,
updateMessage,
updateMessageEntries,
@ -28,6 +29,9 @@ export function createDoneToolDelta(context: DoneToolContext, doneToolState: Don
return async function doneToolDelta(
options: { inputTextDelta: string } & ToolCallOptions
): Promise<void> {
if (doneToolState.isFinalizing) {
return;
}
// Accumulate the delta to the args
doneToolState.args = (doneToolState.args || '') + options.inputTextDelta;
@ -54,31 +58,68 @@ export function createDoneToolDelta(context: DoneToolContext, doneToolState: Don
versionNumber: number;
};
function isAssetToReturn(value: unknown): value is AssetToReturn {
if (!value || typeof value !== 'object') return false;
const obj = value as Record<string, unknown>;
const idOk = typeof obj.assetId === 'string';
const nameOk = typeof obj.assetName === 'string';
const typeVal = obj.assetType;
const typeOk =
typeof typeVal === 'string' &&
ResponseMessageFileTypeSchema.options.includes(typeVal as ResponseMessageFileType);
const versionOk = typeof obj.versionNumber === 'number' && obj.versionNumber > 0;
return idOk && nameOk && typeOk && versionOk;
}
let assetsToInsert: AssetToReturn[] = [];
if (Array.isArray(rawAssets)) {
assetsToInsert = rawAssets.filter(isAssetToReturn);
} else if (typeof rawAssets === 'string') {
try {
const parsed: unknown = JSON.parse(rawAssets);
if (Array.isArray(parsed)) {
assetsToInsert = parsed.filter(isAssetToReturn);
}
} catch {
// ignore malformed JSON until more delta arrives
const rawAssetItems: unknown[] = (() => {
if (Array.isArray(rawAssets)) {
return rawAssets;
}
if (typeof rawAssets === 'string') {
try {
const parsed: unknown = JSON.parse(rawAssets);
if (Array.isArray(parsed)) {
return parsed;
}
} catch {
// ignore malformed JSON until more delta arrives
}
}
return [];
})();
const assetsToInsert: AssetToReturn[] = [];
for (const candidate of rawAssetItems) {
if (!candidate || typeof candidate !== 'object') {
continue;
}
const data = candidate as Record<string, unknown>;
const assetId = typeof data.assetId === 'string' ? data.assetId : undefined;
const assetName = typeof data.assetName === 'string' ? data.assetName : undefined;
const rawType = data.assetType;
const normalizedType =
typeof rawType === 'string' &&
ResponseMessageFileTypeSchema.options.includes(rawType as ResponseMessageFileType)
? (rawType as ResponseMessageFileType)
: undefined;
if (!assetId || !assetName || !normalizedType) {
continue;
}
let versionNumber: number | undefined;
if (typeof data.versionNumber === 'number') {
versionNumber = data.versionNumber;
} else if (typeof (data as { version_number?: unknown }).version_number === 'number') {
versionNumber = (data as { version_number: number }).version_number;
}
if (versionNumber === undefined || Number.isNaN(versionNumber) || versionNumber <= 0) {
try {
versionNumber = await getAssetLatestVersion({
assetId,
assetType: normalizedType,
});
} catch (error) {
console.error('[done-tool] Failed to fetch asset version, defaulting to 1:', error);
versionNumber = 1;
}
}
assetsToInsert.push({
assetId,
assetName,
assetType: normalizedType,
versionNumber,
});
}
// Insert any newly completed asset items as response messages (dedupe via state)
@ -129,7 +170,11 @@ export function createDoneToolDelta(context: DoneToolContext, doneToolState: Don
if (newAssets.length > 0) {
doneToolState.addedAssets = [
...(doneToolState.addedAssets || []),
...newAssets.map((a) => ({ assetId: a.assetId, assetType: a.assetType, versionNumber: a.versionNumber })),
...newAssets.map((a) => ({
assetId: a.assetId,
assetType: a.assetType,
versionNumber: a.versionNumber,
})),
];
}
}

View File

@ -75,6 +75,7 @@ export function createDoneToolExecute(context: DoneToolContext, state: DoneToolS
throw new Error('Tool call ID is required');
}
state.isFinalizing = true;
// CRITICAL: Wait for ALL pending updates from delta/finish to complete FIRST
// This ensures execute's update is always the last one in the queue
await waitForPendingUpdates(context.messageId);

View File

@ -24,6 +24,7 @@ export function createDoneToolStart(context: DoneToolContext, doneToolState: Don
doneToolState.finalResponse = undefined;
doneToolState.addedAssetIds = [];
doneToolState.addedAssets = [];
doneToolState.isFinalizing = false;
// Selection logic moved to delta; skip extracting files here
if (options.messages) {

View File

@ -603,6 +603,29 @@ describe('Done Tool Streaming Tests', () => {
expect(state.args).toBe('{"finalResponse": ""}');
expect(state.finalResponse).toBeUndefined();
});
test('should ignore deltas after execute begins', async () => {
vi.clearAllMocks();
const state: DoneToolState = {
toolCallId: 'test-entry',
args: '',
finalResponse: 'Complete response',
isFinalizing: true,
};
const deltaHandler = createDoneToolDelta(mockContext, state);
await deltaHandler({
inputTextDelta: '{"finalResponse": "Stale"}',
toolCallId: 'tool-call-123',
messages: [],
});
const queries = await import('@buster/database/queries');
expect(queries.updateMessageEntries).not.toHaveBeenCalled();
expect(state.args).toBe('');
expect(state.finalResponse).toBe('Complete response');
});
});
describe('createDoneToolFinish', () => {

View File

@ -15,7 +15,11 @@ export const DoneToolInputSchema = z.object({
assetId: z.string().uuid(),
assetName: z.string(),
assetType: AssetTypeSchema,
versionNumber: z.number().int().positive().describe('The version number of the asset to return'),
versionNumber: z
.number()
.int()
.positive()
.describe('The version number of the asset to return'),
})
)
.describe(
@ -67,6 +71,10 @@ const DoneToolStateSchema = z.object({
)
.optional()
.describe('Assets that have been added with their types and version numbers for chat update'),
isFinalizing: z
.boolean()
.optional()
.describe('Indicates the execute phase has started so further deltas should be ignored'),
});
export type DoneToolInput = z.infer<typeof DoneToolInputSchema>;
@ -81,6 +89,7 @@ export function createDoneTool(context: DoneToolContext) {
finalResponse: undefined,
addedAssetIds: [],
addedAssets: [],
isFinalizing: false,
};
const execute = createDoneToolExecute(context, state);

View File

@ -1,24 +1,15 @@
{
"$schema": "https://turborepo.com/schema.json",
"extends": [
"//"
],
"extends": ["//"],
"tasks": {
"build": {
"dependsOn": [
"^build"
],
"outputs": [
"dist/**"
]
"dependsOn": ["^build"],
"outputs": ["dist/**"]
},
"db:init": {
"cache": false,
"persistent": false,
"dependsOn": [
"db:seed",
"@buster-app/supabase#start"
]
"dependsOn": ["db:seed", "@buster-app/supabase#start"]
},
"db:migrate": {
"cache": false,
@ -27,10 +18,7 @@
"db:seed": {
"cache": false,
"persistent": false,
"dependsOn": [
"db:migrate",
"@buster-app/supabase#start"
]
"dependsOn": ["db:migrate", "@buster-app/supabase#start"]
},
"db:dump": {
"cache": false,
@ -53,4 +41,4 @@
"persistent": true
}
}
}
}