feat: LangGraph Protocol Implementation with Streaming Chat#383
feat: LangGraph Protocol Implementation with Streaming Chat#383serefyarar merged 113 commits intoindexnetwork:devfrom
Conversation
…dual descriptions
- Remove database dependency from OpportunityEvaluator - Require sourceProfileContext to be passed explicitly - Update OpportunityService to fetch profile data before calling evaluator - Update tests to reflect dependency injection changes
- Add Semantic Governance fields to schema: semantic_entropy, referential_anchor, intentMode, etc. - Implement SemanticVerifier for calculating felicity conditions and constraint density - Update IntentReconciler to utilize semantic metrics for conflict resolution - Add comprehensive documentation on Linguistic Architectures and Semantic Governance
- Add OpGraphState, OpportunityGraph, and OpportunityGraphSpec - Implement opportunity generation and scoring logic within the graph - Minor formatting updates to profile and hyde generators
- Implement ProfileController with Drizzle adapter and new ParallelScraperAdapter - Update ProfileGraph to correctly use userId for filtering - Integrate new Parallel searchUser API for better objective-based scraping - Add integration tests for ProfileController
…ecific methods - Update Database interface to use getProfile, saveProfile, saveHydeProfile, getUser - Implement specific methods in DrizzleDatabaseAdapter - Update ProfileGraph and OpportunityGraph to use new Database API - Refactor Intent Agents (Inferrer, Verifier, Reconciler) to use ReactAgent and remove manual SystemMessage handling - Remove explicit Database/Embedder dependency injection from IntentGraph and Agents - Update all related tests to match new architecture
Introduce AuthGuard for validating JWT tokens via Privy. - Validates Authorization header and extracts access token - Verifies token with Privy client - Fetches user from database and validates account status - Returns typed AuthenticatedUser object for use in controllers
Implement new Bun.serve-based HTTP server for v2 API: - Route matching with global /v2 prefix - Controller and route discovery via RouteRegistry - Guard execution chain with result passing - Proper error handling and HTTP status mapping - Runs on port 3003
Migrate ProfileController to decorator-based routing: - Add @controller('/profiles') decorator - Add @post('/sync') and @UseGuards(AuthGuard) to sync method - Update sync signature to accept Request and AuthenticatedUser - Return Response.json instead of raw result
Add npm scripts for running the v2 server: - dev:v2: Development mode with watch for main.ts - start:v2: Production start for main.ts
Clean up unused import that was moved to auth guard
Add empty chat graph module for future implementation
Add comprehensive intent-related types for database operations: - ActiveIntent, CreateIntentData, UpdateIntentData for CRUD - CreatedIntent, IntentRecord, SimilarIntent for results - ArchiveResult for soft-delete operations - SimilarIntentSearchOptions for vector search Extend Database interface with intent operations: - getActiveIntents for state population - createIntent, updateIntent, archiveIntent for actions - getIntent, getIntentWithOwnership for queries - getUserIndexIds, associateIntentWithIndexes for associations - findSimilarIntents for vector similarity search Add narrowed database interfaces (Interface Segregation): - ProfileGraphDatabase for profile operations - ChatGraphDatabase for chat context loading - ChatGraphCompositeDatabase for subgraph orchestration - OpportunityGraphDatabase for opportunity evaluation - IntentExecutorDatabase for action execution - IntentGraphDatabase for graph operations
…odes Enhance IntentGraphState with new fields: - Add userId as required input for database operations - Add executionResults to track action outcomes - Change activeIntents from input to graph-populated field - Add ExecutionResult interface for action tracking Update IntentGraphFactory to accept IntentGraphDatabase: - Add prepNode to fetch active intents before reconciliation - Add executorNode to persist reconciler actions to database - Wire new nodes into graph flow: START -> prep -> inference -> verification -> reconciler -> executor -> END Update tests to work with new graph structure
Add ChatGraph as the main orchestration layer for conversations: - loadContextNode: fetches user profile and active intents - routerNode: analyzes message and determines routing target - Subgraph wrapper nodes for intent, profile, and opportunity - generateResponseNode: synthesizes final response from results - Conditional routing based on message intent Add RouterAgent for intelligent message routing: - Routes to intent_subgraph for goal/desire expressions - Routes to profile_subgraph for profile queries - Routes to opportunity_subgraph for discovery requests - Routes to respond for general conversation - Routes to clarify for ambiguous messages Add ResponseGeneratorAgent: - Synthesizes natural, conversational responses - Contextualizes responses based on subgraph results - Handles error cases with graceful fallbacks Add ChatGraphState with comprehensive state annotations: - Message history with HumanMessage/AIMessage support - User context (profile, active intents) - Routing decision tracking - Subgraph results aggregation
Add ChatController with ChatDatabaseAdapter: - Implements ChatGraphCompositeDatabase for full subgraph support - chat() method for message processing via ChatGraph - Integrates with IndexEmbedder and ParallelScraper - Supports all intent lifecycle operations Add IntentController with IntentDatabaseAdapter: - Implements IntentGraphDatabase for intent operations - process() method for intent graph invocation - CRUD operations: create, update, archive - Active intent retrieval for user context Add OpportunityController with OpportunityDatabaseAdapter: - Implements OpportunityGraphDatabase for profile lookup - discover() method for opportunity matching - Vector search integration for semantic profile matching - User filtering to exclude self-matches Add controller.template.md for consistent implementation patterns
…unity controllers Add ChatController tests: - Test chat() with various conversation scenarios - Validate routing decision handling - Verify subgraph invocation and response generation Add IntentController tests: - Test intent processing via graph invocation - Validate CRUD operations (create, update, archive) - Test active intent retrieval Add OpportunityController tests: - Test opportunity discovery flow - Validate vector search integration - Verify profile matching and scoring
Update ProfileGraphFactory: - Accept ProfileGraphDatabase instead of full Database interface - Maintains same functionality with reduced coupling Update ProfileController: - DrizzleDatabaseAdapter implements ProfileGraphDatabase - Type-safe interface for profile operations only Update ProfileController tests: - Add AuthenticatedUser mock for sync method calls - Align test signatures with controller method changes
…e interface Update OpportunityGraph: - Accept OpportunityGraphDatabase instead of full Database interface - Only requires getProfile method for opportunity evaluation - Improved interface segregation for minimal dependency
Update IntentReconcilerAgent tests: - Remove mock Database and Embedder dependencies - Agent no longer requires these in constructor Update SemanticVerifierAgent tests: - Remove mock Database and Embedder dependencies - Simplified agent instantiation Update OpportunityEvaluator tests: - Remove mock Runnable agent dependency - Agent now manages its own LLM instance internally
Add imports and instantiate new controllers: - ChatController for conversation handling - IntentController for intent processing - OpportunityController for opportunity discovery Controllers are registered with RouteRegistry for API exposure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@frontend/src/contexts/AIChatContext.tsx`:
- Around line 104-163: The SSE parsing loop (using decoder, buffer and
reader.read()) can leave a trailing '\r' from CRLF and cause JSON.parse to fail;
before checking startsWith('data: ') and slicing, normalize each line by
removing a trailing carriage return (e.g. strip a trailing '\r' or use a regex
like /\r$/) or trim it, then proceed to parse the JSON and update messages
(cases referencing assistantMessageId, setMessages, ThinkingStep, etc.); apply
this normalization right inside the for (const line of lines) block before any
startsWith or JSON.parse calls.
In `@protocol/src/controllers/upload.controller.ts`:
- Around line 189-193: The pagination parsing for url.searchParams currently
trusts parseInt and can produce NaN for malformed inputs; update the logic
around url, page, limit, and skip in upload.controller.ts to validate parsed
values using Number.isNaN (or isFinite) and fall back to safe defaults (page =
1, limit = 100) and enforce bounds (page >= 1, 1 <= limit <= 100) before
computing skip; ensure any non-numeric or out-of-range values are normalized so
skip = (page - 1) * limit never becomes NaN or negative.
In `@protocol/src/lib/log.ts`:
- Around line 69-83: The function wrapWithContext currently declares an unused
parameter line; remove that parameter from wrapWithContext's signature and its
internal references, then update all call sites (notably where createLogger
invokes wrapWithContext) to stop passing the extra line argument so the calls
match the new signature; ensure you only keep the parameters: context and source
in wrapWithContext and adjust createLogger to concatenate the wrapped
prefix/suffix around the line externally as before.
🧹 Nitpick comments (6)
protocol/src/lib/log.ts (2)
24-31: Consider validating parsed RGB values.If
hexcontains non-hex characters,parseIntreturnsNaN, producing a malformed ANSI sequence. While the current hardcodedCONTEXT_STYLESvalues are safe, this could cause issues if the function is reused elsewhere.🛡️ Optional defensive fix
function hexToAnsi(hex: string): string { const n = hex.replace(/^#/, ''); if (n.length !== 6) return ''; const r = parseInt(n.slice(0, 2), 16); const g = parseInt(n.slice(2, 4), 16); const b = parseInt(n.slice(4, 6), 16); + if (Number.isNaN(r) || Number.isNaN(g) || Number.isNaN(b)) return ''; return `\x1b[38;2;${r};${g};${b}m`; }
126-130: Consider defining a proper type for the context-bound logger.The type assertion works but could be replaced with a dedicated type for better maintainability.
♻️ Optional type improvement
+type ContextLogger = LoggerWithSource & { from: (source: string) => LoggerWithSource }; -function addFrom<T extends LogContext>(context: T): LoggerWithSource & { from: (source: string) => LoggerWithSource } { - const logger = createLogger(context) as LoggerWithSource & { from: (source: string) => LoggerWithSource }; +function addFrom<T extends LogContext>(context: T): ContextLogger { + const logger = createLogger(context) as ContextLogger; logger.from = (source: string) => createLogger(context, source); return logger; }protocol/src/controllers/upload.controller.ts (2)
131-133: Prefer async disk I/O on the request path.
Sync writes block the event loop; switch tofs.promises.writeFile.♻️ Proposed refactor
- try { - fs.writeFileSync(filePath, buffer); - } catch (err) { + try { + await fs.promises.writeFile(filePath, buffer); + } catch (err) {
121-149: Align ID typing with Id<'tableName'> for file/user IDs.
Consider typingfileId(and user IDs where applicable) asId<'files'>(andId<'users'>) for consistency and safety.As per coding guidelines: Use
Id<'tableName'>type from_generated/dataModelfor document IDs.protocol/src/controllers/chat.controller.ts (2)
53-107: Use Id<> types for document identifiers.Several adapter methods accept raw string IDs (e.g.,
userId,intentId). Switching toId<'users'>/Id<'intents'>improves compile-time safety and aligns with project conventions. Add theIdimport from_generated/dataModeland apply across these signatures.♻️ Example change
- async getProfile(userId: string): Promise<ProfileDocument | null> { + async getProfile(userId: Id<'users'>): Promise<ProfileDocument | null> { const result = await db.select() .from(schema.userProfiles)- async getUser(userId: string): Promise<User | null> { + async getUser(userId: Id<'users'>): Promise<User | null> { const result = await db.select() .from(schema.users)As per coding guidelines: Use Id<'tableName'> type from _generated/dataModel for document IDs.
196-212: Use Drizzle-inferred type for update payload.
Record<string, any>bypasses type safety for columns. Prefer a schema-derived type likePartial<typeof schema.intents.$inferInsert>to keep updates aligned with the table definition.♻️ Suggested refactor
- const updateData: Record<string, any> = { + const updateData: Partial<typeof schema.intents.$inferInsert> = { updatedAt: new Date(), };As per coding guidelines: Prefer type inference from Drizzle schema over manual type definitions.
| const decoder = new TextDecoder(); | ||
| let buffer = ''; | ||
|
|
||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
|
|
||
| buffer += decoder.decode(value, { stream: true }); | ||
| const lines = buffer.split('\n'); | ||
| buffer = lines.pop() || ''; | ||
|
|
||
| for (const line of lines) { | ||
| if (line.startsWith('data: ')) { | ||
| try { | ||
| const event = JSON.parse(line.slice(6)); | ||
|
|
||
| switch (event.type) { | ||
| case 'thinking': | ||
| setMessages(prev => prev.map(msg => { | ||
| if (msg.id === assistantMessageId) { | ||
| const newThinkingStep: ThinkingStep = { | ||
| content: event.content, | ||
| step: event.step, | ||
| timestamp: new Date(event.timestamp), | ||
| }; | ||
| return { | ||
| ...msg, | ||
| thinking: [...(msg.thinking || []), newThinkingStep], | ||
| }; | ||
| } | ||
| return msg; | ||
| })); | ||
| break; | ||
| case 'token': | ||
| setMessages(prev => prev.map(msg => | ||
| msg.id === assistantMessageId | ||
| ? { ...msg, content: msg.content + event.content } | ||
| : msg | ||
| )); | ||
| break; | ||
| case 'done': | ||
| setMessages(prev => prev.map(msg => | ||
| msg.id === assistantMessageId | ||
| ? { ...msg, content: event.response || msg.content, isStreaming: false } | ||
| : msg | ||
| )); | ||
| break; | ||
| case 'error': | ||
| setMessages(prev => prev.map(msg => | ||
| msg.id === assistantMessageId | ||
| ? { ...msg, content: `Error: ${event.message}`, isStreaming: false } | ||
| : msg | ||
| )); | ||
| break; | ||
| } | ||
| } catch (e) { | ||
| console.error('Failed to parse SSE event:', e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle CRLF line endings in SSE parsing.
SSE commonly uses \r\n; splitting on \n can leave a trailing \r and break JSON.parse.
🔧 Proposed fix
- const lines = buffer.split('\n');
+ const lines = buffer.split(/\r?\n/);
...
- const event = JSON.parse(line.slice(6));
+ const event = JSON.parse(line.slice(6).trim());🤖 Prompt for AI Agents
In `@frontend/src/contexts/AIChatContext.tsx` around lines 104 - 163, The SSE
parsing loop (using decoder, buffer and reader.read()) can leave a trailing '\r'
from CRLF and cause JSON.parse to fail; before checking startsWith('data: ') and
slicing, normalize each line by removing a trailing carriage return (e.g. strip
a trailing '\r' or use a regex like /\r$/) or trim it, then proceed to parse the
JSON and update messages (cases referencing assistantMessageId, setMessages,
ThinkingStep, etc.); apply this normalization right inside the for (const line
of lines) block before any startsWith or JSON.parse calls.
| const url = new URL(req.url); | ||
| const page = Math.max(1, parseInt(url.searchParams.get('page') || '1', 10)); | ||
| const limit = Math.min(100, Math.max(1, parseInt(url.searchParams.get('limit') || '100', 10))); | ||
| const skip = (page - 1) * limit; | ||
|
|
There was a problem hiding this comment.
Guard against NaN page/limit values.
Invalid query params can yield NaN and break pagination logic.
🛠️ Proposed fix
- const page = Math.max(1, parseInt(url.searchParams.get('page') || '1', 10));
- const limit = Math.min(100, Math.max(1, parseInt(url.searchParams.get('limit') || '100', 10)));
+ const pageRaw = parseInt(url.searchParams.get('page') || '1', 10);
+ const limitRaw = parseInt(url.searchParams.get('limit') || '100', 10);
+ const page = Number.isFinite(pageRaw) ? Math.max(1, pageRaw) : 1;
+ const limit = Number.isFinite(limitRaw) ? Math.min(100, Math.max(1, limitRaw)) : 100;🤖 Prompt for AI Agents
In `@protocol/src/controllers/upload.controller.ts` around lines 189 - 193, The
pagination parsing for url.searchParams currently trusts parseInt and can
produce NaN for malformed inputs; update the logic around url, page, limit, and
skip in upload.controller.ts to validate parsed values using Number.isNaN (or
isFinite) and fall back to safe defaults (page = 1, limit = 100) and enforce
bounds (page >= 1, 1 <= limit <= 100) before computing skip; ensure any
non-numeric or out-of-range values are normalized so skip = (page - 1) * limit
never becomes NaN or negative.
| /** Wrap line with emoji + source + optional color. Format: "emoji source: message" (source required for consistency). */ | ||
| function wrapWithContext( | ||
| context: LogContext | undefined, | ||
| source: string | undefined, | ||
| line: string | ||
| ): { start: string; end: string } { | ||
| if (!context || !CONTEXT_STYLES[context]) | ||
| return { start: source ? `${source}: ` : '', end: '' }; | ||
| const { emoji, color } = CONTEXT_STYLES[context]; | ||
| const colorOn = useColor() && color; | ||
| const ansi = colorOn ? hexToAnsi(color) : ''; | ||
| const reset = colorOn ? RESET : ''; | ||
| const prefix = source ? `${emoji} ${source}: ` : `${emoji} `; | ||
| return { start: ansi ? `${ansi}${prefix}` : prefix, end: reset }; | ||
| } |
There was a problem hiding this comment.
Remove unused line parameter.
The line parameter is declared but never referenced in the function body. The actual line content is wrapped externally in createLogger. This unused parameter should be removed to avoid confusion.
🧹 Proposed fix
-/** Wrap line with emoji + source + optional color. Format: "emoji source: message" (source required for consistency). */
+/** Build prefix/suffix for context-styled log output. Format: "emoji source: message". */
function wrapWithContext(
context: LogContext | undefined,
- source: string | undefined,
- line: string
+ source: string | undefined
): { start: string; end: string } {And update the call sites in createLogger:
- const { start, end } = wrapWithContext(context, source, line);
+ const { start, end } = wrapWithContext(context, source);🤖 Prompt for AI Agents
In `@protocol/src/lib/log.ts` around lines 69 - 83, The function wrapWithContext
currently declares an unused parameter line; remove that parameter from
wrapWithContext's signature and its internal references, then update all call
sites (notably where createLogger invokes wrapWithContext) to stop passing the
extra line argument so the calls match the new signature; ensure you only keep
the parameters: context and source in wrapWithContext and adjust createLogger to
concatenate the wrapped prefix/suffix around the line externally as before.
- Add CONCEPT EXTRACTION rules to ExplicitIntentInferrer prompt - Strip project/company/product names - describe work and tech instead - Remove source references and file names from intent descriptions - Add intent-enrichment-approaches.md documenting problem and solutions
- Add adapters/: database.adapter.ts (Intent, Chat, Profile, Opportunity, Index adapters with local types, no lib/protocol deps), scraper.adapter.ts (ScraperAdapter, imports only from lib/parallel), queue.adapter.ts - Remove adapter implementations from controllers; import from adapters - Rename DrizzleDatabaseAdapter → ProfileDatabaseAdapter; drop vendor names from adapter layer (no Parallel/Drizzle in class names or comments) - Move lib/db.ts → lib/drizzle/drizzle.ts; update all imports across controllers, services, routes, lib, agents, cli, tests, plans, templates
…pter - Protocol queue.interface.ts re-exports queue types from adapters/queue.adapter - Keeps protocol decoupled from BullMQ; adapter owns the queue contract
- Move schema from src/lib/schema.ts to src/schemas/database.schema.ts - Update drizzle.config.ts to use ./src/schemas/database.schema.ts - All imports already reference schemas/database.schema (from prior commit)
- Add index.graph.state.ts with state for prep/evaluate/execute - Add index.graph.ts (IndexGraphFactory) with prep, evaluate (IntentIndexer), execute nodes - Extend Database interface with IndexGraphDatabase (getIntentForIndexing, getIndexMemberContext, isIntentAssignedToIndex, assignIntentToIndex, unassignIntentFromIndex) - Add INDEX-MANAGEMENT-AGENTIC-ARCHITECTURE.md documenting agentic index management
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@protocol/src/adapters/queue.adapter.ts`:
- Around line 169-177: The addJob method currently uses a non-null assertion on
job.id (in the async addJob function calling queue.add), which can crash if
BullMQ returns undefined; update addJob to defensively handle a missing id by
checking if job.id is undefined after await queue.add(name, data, {...base,
...extra}) and then either throw a clear Error (e.g., "Failed to create job:
missing id") or return a controlled failure result, ensuring callers receive a
predictable response instead of relying on job.id!; refer to addJob, queue.add,
job.id and getOptions to locate and implement the fix.
In `@protocol/src/adapters/scraper.adapter.ts`:
- Around line 34-41: The method extractUrlContent in scraper.adapter.ts
currently returns the result of the underlying extractUrlContent call directly
and will propagate exceptions, violating its Promise<string|null> contract; wrap
the call in a try/catch in the async method (async extractUrlContent(url:
string): Promise<string | null>) so that any thrown error is caught, optionally
log the error, and return null on failure instead of letting the exception
bubble up to callers (keep the same signature and call the underlying
extractUrlContent within the try block).
In `@protocol/src/controllers/chat.controller.ts`:
- Around line 292-300: The SSE response headers returned by the "return new
Response(...)" block need to include the nginx-compatible header to disable
buffering; update the headers object in the Response creation (the same block
that sets 'Content-Type', 'Cache-Control', 'Connection', and 'X-Session-Id' and
uses sessionId) to also include 'X-Accel-Buffering': 'no' so SSE events aren't
buffered by nginx/reverse proxies.
In `@protocol/src/controllers/controller.template.md`:
- Around line 220-227: The markdown table spacing for the decorators section is
not in the repo’s configured “compact” table style; update the pipe padding so
columns are compact (no extra spaces between pipes and cell text) for the table
rows containing `@Controller(path)`, `@Get(path)`, `@Post(path)`, `@Put(path)`,
`@Delete(path)`, and `@UseGuards(...guards)`, or simply run the repository
formatter/markdownlint auto-fix to apply the MD060-compliant compact table
style; apply the same fix to the other decorator tables in this template.
🧹 Nitpick comments (7)
protocol/src/agents/core/intent_indexer/index.ts (1)
1-2: Avoid direct DB access in agents (even if deprecated).Move persistence into a service/adapter and keep this agent layer pure to align with the agent guidelines. As per coding guidelines: Keep agents pure with no direct database access; let services handle persistence.
protocol/src/agents/context_brokers/connector.ts (1)
1-2: Avoid direct DB access in broker connector.Consider moving persistence into a service/adapter and keep this agent-layer module pure. As per coding guidelines: Keep agents pure with no direct database access; let services handle persistence.
protocol/src/controllers/profile.controller.spec.ts (1)
1-12: Move this integration test underprotocol/tests/.
This spec is an integration test and should live inprotocol/tests/(orprotocol/src/lib/*/tests/for unit tests), with imports adjusted accordingly.Based on learnings: Test files should be located in
protocol/tests/for integration/E2E tests orprotocol/src/lib/*/tests/for unit tests.protocol/src/controllers/upload.controller.spec.ts (1)
78-83: Consider using consistentprivyIdin mock user.The
getMockUserfunction generates a newprivyIdon each call usingDate.now(), but this doesn't match the actual test user'sprivyIdcreated inbeforeAll. While this doesn't affect current tests (since the controller likely only usesid), it could cause subtle issues if the controller logic ever validatesprivyIdconsistency.♻️ Suggested fix
+ let testUserPrivyId: string; + beforeAll(async () => { // ... existing code ... + testUserPrivyId = `privy:upload:${Date.now()}`; const [user] = await db.insert(schema.users).values({ email: testEmail, name: "Test Upload User", - privyId: `privy:upload:${Date.now()}`, + privyId: testUserPrivyId, // ... }).returning(); // ... }); const getMockUser = (): AuthenticatedUser => ({ id: testUserId, - privyId: `privy:upload:${Date.now()}`, + privyId: testUserPrivyId, email: testEmail, name: "Test Upload User", });protocol/src/controllers/opportunity.controller.ts (2)
36-41: Fragile filter handling withanycasts.The filter handling uses unsafe type assertions that could break silently if the filter structure changes. Consider defining a proper type for the expected filter shape.
♻️ Suggested improvement
+// Define expected filter shape +interface ProfileSearchFilter { + userId?: { ne: string }; +} async function searchProfiles<T>( vector: number[], collection: string, - options?: VectorStoreOption<T> + options?: VectorStoreOption<ProfileSearchFilter> ): Promise<VectorSearchResult<T>[]> { // ... const limit = options?.limit || 10; - const filter = options?.filter; + const filter = options?.filter as ProfileSearchFilter | undefined; // Build conditions const conditions = [isNotNull(schema.userProfiles.embedding)]; if (filter) { - // Handle userId exclusion filter (ne = not equal) - if (filter.userId && typeof filter.userId === 'object' && (filter.userId as any).ne) { - conditions.push(ne(schema.userProfiles.userId, (filter.userId as any).ne)); + if (filter.userId?.ne) { + conditions.push(ne(schema.userProfiles.userId, filter.userId.ne)); } }
54-57: Type safety lost in result mapping.The
anycast on line 54 discards type information. Consider using a more specific type or Drizzle's inferred select type.♻️ Suggested improvement
- return resultsWithDistance.map((r: any) => ({ - item: r.item as unknown as T, + return resultsWithDistance.map((r) => ({ + item: r.item as unknown as T, score: 1 - r.distance // Convert distance to similarity score }));The
rparameter should already be typed by Drizzle from the select statement, so you can remove the explicitanyannotation.protocol/src/adapters/database.adapter.ts (1)
77-169: Significant code duplication between adapters.
IntentDatabaseAdapterandChatDatabaseAdaptershare nearly identical implementations of:
getActiveIntentscreateIntentupdateIntentarchiveIntentSimilarly,
ChatDatabaseAdapterandProfileDatabaseAdapterduplicate:
getProfilesaveProfilesaveHydeProfilegetUserConsider extracting a shared base class or composable mixins to reduce duplication and ensure consistent behavior across adapters.
♻️ Architectural suggestion
// Shared implementation for intent operations class IntentOperationsMixin { async getActiveIntents(userId: string): Promise<ActiveIntentRow[]> { /* ... */ } async createIntent(data: CreateIntentInput): Promise<CreatedIntentRow> { /* ... */ } async updateIntent(intentId: string, userId: string, data: UpdateIntentInput): Promise<CreatedIntentRow | null> { /* ... */ } async archiveIntent(intentId: string, userId: string): Promise<ArchiveResultShape> { /* ... */ } } // Shared implementation for profile operations class ProfileOperationsMixin { async getProfile(userId: string): Promise<ProfileRow | null> { /* ... */ } async saveProfile(userId: string, profile: ProfileRow): Promise<void> { /* ... */ } async saveHydeProfile(userId: string, description: string, embedding: number[]): Promise<void> { /* ... */ } async getUser(userId: string): Promise<User | null> { /* ... */ } } // Compose as needed export class ChatDatabaseAdapter { private intents = new IntentOperationsMixin(); private profiles = new ProfileOperationsMixin(); getActiveIntents = this.intents.getActiveIntents.bind(this.intents); // ... delegate other methods }Also applies to: 178-315
| async addJob(name, data, priority = 0) { | ||
| const base: JobsOptions = { | ||
| priority: priority > 0 ? priority : undefined, | ||
| }; | ||
| const extra = getOptions?.(name, data, priority); | ||
| const job = await queue.add(name as never, data as never, { ...base, ...extra }); | ||
| return { id: job.id! }; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Non-null assertion on job.id may cause runtime error.
The job.id! assertion assumes BullMQ always returns an ID, but this could be undefined in certain edge cases (e.g., queue errors). Consider adding defensive handling.
🛠️ Suggested fix
const job = await queue.add(name as never, data as never, { ...base, ...extra });
- return { id: job.id! };
+ if (!job.id) {
+ throw new Error(`Failed to add job "${name}" - no job ID returned`);
+ }
+ return { id: job.id };📝 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.
| async addJob(name, data, priority = 0) { | |
| const base: JobsOptions = { | |
| priority: priority > 0 ? priority : undefined, | |
| }; | |
| const extra = getOptions?.(name, data, priority); | |
| const job = await queue.add(name as never, data as never, { ...base, ...extra }); | |
| return { id: job.id! }; | |
| }, | |
| }; | |
| async addJob(name, data, priority = 0) { | |
| const base: JobsOptions = { | |
| priority: priority > 0 ? priority : undefined, | |
| }; | |
| const extra = getOptions?.(name, data, priority); | |
| const job = await queue.add(name as never, data as never, { ...base, ...extra }); | |
| if (!job.id) { | |
| throw new Error(`Failed to add job "${name}" - no job ID returned`); | |
| } | |
| return { id: job.id }; | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In `@protocol/src/adapters/queue.adapter.ts` around lines 169 - 177, The addJob
method currently uses a non-null assertion on job.id (in the async addJob
function calling queue.add), which can crash if BullMQ returns undefined; update
addJob to defensively handle a missing id by checking if job.id is undefined
after await queue.add(name, data, {...base, ...extra}) and then either throw a
clear Error (e.g., "Failed to create job: missing id") or return a controlled
failure result, ensuring callers receive a predictable response instead of
relying on job.id!; refer to addJob, queue.add, job.id and getOptions to locate
and implement the fix.
| /** | ||
| * Extracts content from a URL. | ||
| * @param url - The URL to extract content from | ||
| * @returns The extracted content as a string, or null if extraction failed | ||
| */ | ||
| async extractUrlContent(url: string): Promise<string | null> { | ||
| return extractUrlContent(url); | ||
| } |
There was a problem hiding this comment.
Handle URL extraction failures to honor the null-return contract.
extractUrlContent() will reject on exceptions, but the doc promises null on failure. Wrap it in try/catch to avoid bubbling errors into graphs.
🛠️ Proposed fix
async extractUrlContent(url: string): Promise<string | null> {
- return extractUrlContent(url);
+ try {
+ return await extractUrlContent(url);
+ } catch (error: unknown) {
+ console.error('ScraperAdapter extractUrlContent error:', error);
+ return null;
+ }
}🤖 Prompt for AI Agents
In `@protocol/src/adapters/scraper.adapter.ts` around lines 34 - 41, The method
extractUrlContent in scraper.adapter.ts currently returns the result of the
underlying extractUrlContent call directly and will propagate exceptions,
violating its Promise<string|null> contract; wrap the call in a try/catch in the
async method (async extractUrlContent(url: string): Promise<string | null>) so
that any thrown error is caught, optionally log the error, and return null on
failure instead of letting the exception bubble up to callers (keep the same
signature and call the underlying extractUrlContent within the try block).
| return new Response(stream, { | ||
| headers: { | ||
| 'Content-Type': 'text/event-stream', | ||
| 'Cache-Control': 'no-cache', | ||
| 'Connection': 'keep-alive', | ||
| 'X-Session-Id': sessionId, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
HTTP headers Server-Sent Events SSE reverse proxy buffering nginx best practices
💡 Result:
SSE response headers (from your upstream app)
Send SSE with these headers so browsers/proxies treat it as a live stream:
Content-Type: text/event-stream(developer.mozilla.org)Cache-Control: no-cache(so intermediaries don’t cache) (developer.mozilla.org)X-Accel-Buffering: no(tells nginx to disable response buffering for that response) (developer.mozilla.org)- (If cross-origin) set the appropriate CORS headers; SSE follows CORS rules (w3.org)
nginx reverse-proxy best practices for SSE (buffering, timeouts, and headers)
Key points:
- Disable proxy response buffering for the SSE location:
proxy_buffering off;(nginx.org) - Disable proxy caching for the SSE location (
proxy_cache off;) (common practice to avoid “stuck”/delayed events) (stackoverflow.com) - Increase timeouts if your stream can be idle for long periods (otherwise nginx may close the connection) (stackoverflow.com)
- Use HTTP/1.1 to the upstream (recommended for keepalive-style behavior) via
proxy_http_version 1.1;(nginx.org) - If you rely on the upstream’s
X-Accel-Buffering: no: be aware nginx can hideX-Accel-*headers by default; you may need to explicitly pass it onward in some multi-proxy setups (nginx-in-front-of-nginx) (nginx.org) - Ensure you are not ignoring
X-Accel-Bufferingviaproxy_ignore_headers(nginx.org)
Minimal nginx location example
location /events/ {
proxy_pass http://upstream_app;
# SSE: stream immediately (don’t buffer)
proxy_buffering off;
proxy_cache off;
# Keep the upstream connection open
proxy_http_version 1.1;
proxy_set_header Connection "";
# If events can be infrequent, raise timeouts
proxy_read_timeout 1h;
send_timeout 1h;
# If you depend on upstream’s X-Accel-Buffering header and have multiple proxies:
# proxy_pass_header X-Accel-Buffering;
}Why this works: proxy_buffering off makes nginx forward data synchronously as it arrives, and nginx also supports toggling buffering via the upstream X-Accel-Buffering: no response header. (nginx.org)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events?utm_source=openai
- 3: https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events?utm_source=openai
- 4: https://www.w3.org/TR/2009/WD-eventsource-20090423/?utm_source=openai
- 5: https://nginx.org/r/proxy_ssl_conf_command?utm_source=openai
- 6: https://stackoverflow.com/questions/27898622/server-sent-events-stopped-work-after-enabling-ssl-on-proxy?utm_source=openai
- 7: https://stackoverflow.com/questions/27898622/server-sent-events-stopped-work-after-enabling-ssl-on-proxy?utm_source=openai
- 8: https://nginx.org/r/proxy_ssl_conf_command?utm_source=openai
- 9: https://nginx.org/r/proxy_ssl_conf_command?utm_source=openai
- 10: https://nginx.org/r/proxy_ssl_conf_command?utm_source=openai
- 11: https://nginx.org/r/proxy_ssl_conf_command?utm_source=openai
Add X-Accel-Buffering: no header for nginx SSE compatibility.
The current headers are missing X-Accel-Buffering: no, which is recommended by MDN and nginx documentation to prevent response buffering when deploying behind nginx or other reverse proxies that support this header. Without it, SSE events may be delayed. Add this header to the response:
'X-Accel-Buffering': 'no'
🤖 Prompt for AI Agents
In `@protocol/src/controllers/chat.controller.ts` around lines 292 - 300, The SSE
response headers returned by the "return new Response(...)" block need to
include the nginx-compatible header to disable buffering; update the headers
object in the Response creation (the same block that sets 'Content-Type',
'Cache-Control', 'Connection', and 'X-Session-Id' and uses sessionId) to also
include 'X-Accel-Buffering': 'no' so SSE events aren't buffered by nginx/reverse
proxies.
| | Decorator | Purpose | Example | | ||
| |-----------|---------|---------| | ||
| | `@Controller(path)` | Class decorator defining base route path | `@Controller('/profiles')` | | ||
| | `@Get(path)` | GET endpoint | `@Get('/:id')` | | ||
| | `@Post(path)` | POST endpoint | `@Post('/sync')` | | ||
| | `@Put(path)` | PUT endpoint | `@Put('/:id')` | | ||
| | `@Delete(path)` | DELETE endpoint | `@Delete('/:id')` | | ||
| | `@UseGuards(...guards)` | Apply authentication/validation guards | `@UseGuards(AuthGuard)` | |
There was a problem hiding this comment.
Align table pipe spacing with markdownlint MD060.
Static analysis flags table column spacing; adjust the pipe padding (or run the repo’s formatter) to match the configured “compact” table style.
Also applies to: 368-373, 542-547
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 221-221: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 221-221: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 221-221: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 221-221: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 221-221: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 221-221: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@protocol/src/controllers/controller.template.md` around lines 220 - 227, The
markdown table spacing for the decorators section is not in the repo’s
configured “compact” table style; update the pipe padding so columns are compact
(no extra spaces between pipes and cell text) for the table rows containing
`@Controller(path)`, `@Get(path)`, `@Post(path)`, `@Put(path)`, `@Delete(path)`,
and `@UseGuards(...guards)`, or simply run the repository formatter/markdownlint
auto-fix to apply the MD060-compliant compact table style; apply the same fix to
the other decorator tables in this template.
- Add intent.indexer.ts following agent template (config, system prompt, Zod schema, invoke/evaluate, asTool) - Add intent.indexer.types.ts with IntentIndexerOutputSchema and IntentIndexerOutput - Add intent.indexer.spec.ts with 3 tests (structured output, null member prompt, evaluate alias) - Update index graph and state to import from protocol agent (../../agents/index/)
- Add new properties "adapters" and "index" to the workspace configuration for improved project structure and clarity.
Replaces the old chat and inbox page logic with redirects to the main page, centralizes AI chat UI in a new ChatContent component, and updates the root page to use ChatContent for authenticated users. Refactors conversations and user chat pages for improved layout and removes the deprecated InboxContent component. Also introduces ContentContainer for consistent layout and updates onboarding, index, and invitation pages to use it.
Removed unnecessary borders and padding from chat and sidebar components for a cleaner look. Adjusted input and card paddings for better alignment. Moved FeedbackWidget to bottom-right. Improved loading and error state layouts in user profile page. Added 'Discover' heading to ChatContent discovery view.
Deleted the /conversations and /chat pages and removed related navigation logic. Updated all references and redirects to use the home page (/) instead of the removed routes. Cleaned up Sidebar and ClientWrapper to reflect these changes, and removed the LeftNav component. Adjusted protected route logic in AuthContext to remove references to /inbox.
Deleted the intent and inbox pages that redirected users to the home page. These routes are no longer needed as their functionality has been replaced or deprecated.
Integrate Index Graph into Chat Graph to automatically assign newly created intents to user's auto-assign indexes. Extends the composite database interface with Index Graph methods and tracks indexing results in the subgraph output.
…g to - Add IndexMembership type and getIndexMemberships() to database interface - Implement getIndexMemberships in ChatDatabaseAdapter (join index_members + indexes) - Add index_query route to chat router (read-only, with safety rule) - Add index_query node and routing in chat graph (fast path to response) - Add index result formatting in response generator for membership list
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@protocol/src/adapters/database.adapter.ts`:
- Around line 188-244: The class duplicates intent logic and inconsistently
handles DB errors: extract shared intent query logic into a helper (e.g.,
getActiveIntentsQuery or a protected method used by getActiveIntents,
createIntent, updateIntent, archiveIntent) to remove copy/paste, wrap all DB
accessors/mutators (getProfile, getUser, saveProfile, saveHydeProfile) in
try/catch like getActiveIntents to log errors and return safe defaults
(null/[]/void), and remove the unsafe cast in getProfile (replace "as unknown as
ProfileRow" with a safe check/mapping or use a single-row select/first helper
that returns a typed ProfileRow|null).
- Around line 104-107: The catch block in IntentDatabaseAdapter.getActiveIntents
currently swallows errors and returns an empty array; change it to surface
failures and use the structured logger: import the logger from ../lib/log.ts,
replace console.error(...) with logger.error(...) including the caught error,
and then either re-throw the error from getActiveIntents so callers can handle
it or return a discriminated result type (e.g., { ok: true, data: Intent[] } | {
ok: false, error: Error }) so callers can tell “no intents” vs “DB error”;
ensure the error object is preserved when re-throwing or placed into the
discriminated error variant.
🧹 Nitpick comments (4)
protocol/src/adapters/database.adapter.ts (4)
11-66: Consider inferring types from Drizzle schema instead of manual definitions.Manual type definitions can drift from the actual database schema. Drizzle provides inference utilities:
// Example: infer select types from schema type IntentSelect = typeof schema.intents.$inferSelect; type ActiveIntentRow = Pick<IntentSelect, 'id' | 'payload' | 'summary' | 'createdAt'>;Also, the
embeddingfield typenumber[] | number[][](line 65) is unusual and requires defensive handling insaveProfile. Consider normalizing this at the boundary where data enters the adapter rather than handling both shapes internally.Based on coding guidelines: "Prefer type inference from Drizzle schema over manual type definitions."
361-406: DRY violation: Entire class duplicates ChatDatabaseAdapter methods.
ProfileDatabaseAdaptercontains identical implementations ofgetProfile,saveProfile,saveHydeProfile, andgetUserfromChatDatabaseAdapter. This violates DRY and creates maintenance burden.Consider composition or shared utilities:
// Option 1: Composition export class ProfileDatabaseAdapter { private readonly shared = new SharedProfileOperations(); getProfile(userId: string) { return this.shared.getProfile(userId); } // ... } // Option 2: Mixin or shared module import { getProfile, saveProfile } from './shared/profile.operations';
415-423: Same duplication pattern continues.
getProfileis duplicated for the fourth time across adapters. The previous refactoring suggestions apply here as well.
433-446: Add explicit return type annotation.Methods returning inferred types without annotation reduce type safety and IDE support. Define the return type explicitly:
interface IntentForIndexing { id: string; payload: string; userId: string; sourceType: SourceType | null; sourceId: string | null; } async getIntentForIndexing(intentId: string): Promise<IntentForIndexing | null> {Same applies to
getIndexMemberContext(line 448).As per coding guidelines: "Use strict TypeScript mode for all code" implies avoiding implicit
anyreturns.
| } catch (error: unknown) { | ||
| console.error('IntentDatabaseAdapter.getActiveIntents error:', error); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Silent failure masks database errors.
Returning an empty array on database errors makes it impossible for callers to distinguish between "user has no intents" and "database query failed." This could lead to incorrect application behavior (e.g., showing "no intents" when the database is down).
Consider either:
- Re-throwing the error to let callers handle it
- Returning a discriminated result type that distinguishes empty results from errors
Additionally, consider using the structured logger from ../lib/log.ts instead of console.error for consistent observability.
🤖 Prompt for AI Agents
In `@protocol/src/adapters/database.adapter.ts` around lines 104 - 107, The catch
block in IntentDatabaseAdapter.getActiveIntents currently swallows errors and
returns an empty array; change it to surface failures and use the structured
logger: import the logger from ../lib/log.ts, replace console.error(...) with
logger.error(...) including the caught error, and then either re-throw the error
from getActiveIntents so callers can handle it or return a discriminated result
type (e.g., { ok: true, data: Intent[] } | { ok: false, error: Error }) so
callers can tell “no intents” vs “DB error”; ensure the error object is
preserved when re-throwing or placed into the discriminated error variant.
| export class ChatDatabaseAdapter { | ||
| async getProfile(userId: string): Promise<ProfileRow | null> { | ||
| const result = await db.select() | ||
| .from(schema.userProfiles) | ||
| .where(eq(schema.userProfiles.userId, userId)) | ||
| .limit(1); | ||
| return (result[0] as unknown as ProfileRow) ?? null; | ||
| } | ||
|
|
||
| async getActiveIntents(userId: string): Promise<ActiveIntentRow[]> { | ||
| try { | ||
| const result = await db.select({ | ||
| id: schema.intents.id, | ||
| payload: schema.intents.payload, | ||
| summary: schema.intents.summary, | ||
| createdAt: schema.intents.createdAt, | ||
| }) | ||
| .from(schema.intents) | ||
| .where( | ||
| and( | ||
| eq(schema.intents.userId, userId), | ||
| isNull(schema.intents.archivedAt) | ||
| ) | ||
| ); | ||
| return result; | ||
| } catch (error: unknown) { | ||
| console.error('ChatDatabaseAdapter.getActiveIntents error:', error); | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| async getUser(userId: string): Promise<User | null> { | ||
| const result = await db.select() | ||
| .from(schema.users) | ||
| .where(eq(schema.users.id, userId)) | ||
| .limit(1); | ||
| return result[0] ?? null; | ||
| } | ||
|
|
||
| async saveProfile(userId: string, profile: ProfileRow): Promise<void> { | ||
| const data = { | ||
| userId, | ||
| identity: profile.identity, | ||
| narrative: profile.narrative, | ||
| attributes: profile.attributes, | ||
| embedding: Array.isArray(profile.embedding[0]) | ||
| ? (profile.embedding as number[][])[0] | ||
| : (profile.embedding as number[]), | ||
| updatedAt: new Date(), | ||
| }; | ||
| await db.insert(schema.userProfiles) | ||
| .values(data) | ||
| .onConflictDoUpdate({ | ||
| target: schema.userProfiles.userId, | ||
| set: data, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication and inconsistent error handling.
- Duplication: Methods
getActiveIntents,createIntent,updateIntent, andarchiveIntentare identical copies fromIntentDatabaseAdapter. Consider extracting shared logic:
// Shared helper or base class
async function getActiveIntentsQuery(userId: string): Promise<ActiveIntentRow[]> { ... }-
Missing error handling:
getProfile,getUser,saveProfile, andsaveHydeProfilelack try-catch blocks while other methods in the same class have them. This inconsistency means mutations likesaveProfilewill throw unhandled errors. -
Unsafe cast (line 194):
as unknown as ProfileRowbypasses type checking. If the schema changes, this will fail silently at runtime.
♻️ Proposed fix for error handling in saveProfile
async saveProfile(userId: string, profile: ProfileRow): Promise<void> {
+ try {
const data = {
userId,
identity: profile.identity,
...
};
await db.insert(schema.userProfiles)
.values(data)
.onConflictDoUpdate({
target: schema.userProfiles.userId,
set: data,
});
+ } catch (error: unknown) {
+ console.error('ChatDatabaseAdapter.saveProfile error:', error);
+ throw error;
+ }
}🤖 Prompt for AI Agents
In `@protocol/src/adapters/database.adapter.ts` around lines 188 - 244, The class
duplicates intent logic and inconsistently handles DB errors: extract shared
intent query logic into a helper (e.g., getActiveIntentsQuery or a protected
method used by getActiveIntents, createIntent, updateIntent, archiveIntent) to
remove copy/paste, wrap all DB accessors/mutators (getProfile, getUser,
saveProfile, saveHydeProfile) in try/catch like getActiveIntents to log errors
and return safe defaults (null/[]/void), and remove the unsafe cast in
getProfile (replace "as unknown as ProfileRow" with a safe check/mapping or use
a single-row select/first helper that returns a typed ProfileRow|null).
- Database: getOwnedIndexes, isIndexOwner, getIndexMembersForOwner, getIndexIntentsForOwner, updateIndexSettings (owner-only checks) - Router: index_write target and prompts for index settings - Chat graph: indexQueryNode uses owned indexes only, explicit isIndexOwner check before fetching member/intent data; indexWriteNode matches only owned indexes; comments for programmatic restriction - Routes: GET /indexes/:id/intents filters by ownership (owners see all) - Generator: SubgraphResults index types and formatSubgraphResults - Plan: index-ownership-chat-implementation.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@plans/index-ownership-chat-implementation.md`:
- Around line 47-53: The markdown tables (starting at the "**Roles:**" table
containing `owner`, `admin`, and `member`) violate markdownlint rules: add a
blank line above and below each table and normalize pipe spacing so there is a
single space after and before each pipe (e.g., "| Role | Permissions Array |
Capabilities |"), ensure header separator rows use the same compact-pipe spacing
("|------|------------------|--------------|"), and apply the same fixes to the
other tables that include the same role names (`owner`, `admin`, `member`)
elsewhere in the file.
- Around line 102-125: The markdown contains unlabeled fenced code blocks (the
ASCII diagram and other code fences) which trigger MD040; update each
triple-backtick fence (e.g., the diagram block starting with ``` and the other
blocks around lines referenced in the review) to include a language identifier
like ```text (or ```ascii) so the fences are labeled; search for occurrences of
``` in this document and replace with ```text for plain diagrams or the
appropriate language tag for code snippets (for example the block that contains
the Router → load_context → index_query diagram).
In `@protocol/src/adapters/database.adapter.ts`:
- Around line 20-28: The CreateIntentInput interface currently allows creation
of intents without provenance because sourceType and sourceId are optional;
update the interface declaration (CreateIntentInput) to make sourceType and
sourceId non-optional (remove the ? and avoid | null) and then audit and update
any callers of CreateIntentInput (constructors, factory functions, tests, and
places where intents are created or validated) to supply or derive valid
sourceType and sourceId values so compilation passes and intent creation always
includes provenance.
🧹 Nitpick comments (4)
plans/index-ownership-chat-implementation.md (1)
63-71: Minor clarification: mark the security issue as a must-fix blocker for this plan.
This is a plan doc, but the current wording could still be interpreted as optional. Consider explicitly labeling the endpoint exposure as a release blocker to prevent it from being deprioritized.protocol/src/adapters/database.adapter.ts (3)
438-481: Consider aggregating counts to avoid N+1 queries.Per-index count queries scale linearly with owned indexes; a grouped aggregate can fetch counts in one pass.
498-540: Potential N+1 counts per member.Intent counts are fetched per member; consider a grouped aggregate by
userIdto reduce query load.
11-76: Prefer Drizzle-inferred row shapes to avoid schema duplication.The manual interfaces (
ActiveIntentRow,CreateIntentInput, etc.) duplicate schema definitions and can drift; using Drizzle's type inference keeps row types in sync with your schema.In drizzle-orm v0.36.4, the recommended approach is to infer types directly from table definitions using
$inferSelectand$inferInsert:♻️ Suggested refactor pattern
-import { eq, and, isNull, sql, count, desc } from 'drizzle-orm'; +import { eq, and, isNull, sql, count, desc } from 'drizzle-orm'; -// Local types used by adapters (shapes only; protocol layer defines the contracts) -interface ActiveIntentRow { - id: string; - payload: string; - summary: string | null; - createdAt: Date; -} +// Infer types directly from schema +type ActiveIntentRow = Pick<typeof schema.intents.$inferSelect, 'id' | 'payload' | 'summary' | 'createdAt'>; -interface CreateIntentInput { ... } +type CreateIntentInput = typeof schema.intents.$inferInsert & { + userId: string; + sourceType?: 'file' | 'integration' | 'link' | 'discovery_form' | 'enrichment' | null; +};
| **Roles:** | ||
| | Role | Permissions Array | Capabilities | | ||
| |------|------------------|--------------| | ||
| | `owner` | `['owner']` | Full control: settings, members, view all intents | | ||
| | `admin` | `['admin', 'member']` | Manage members (add/remove), cannot promote to owner | | ||
| | `member` | `['member']` | View own intents, manage own index membership | | ||
|
|
There was a problem hiding this comment.
Fix table spacing and blank-line rules to satisfy markdownlint.
Tables are missing required blank lines and compact-pipe spacing, which will keep lint failing.
🧹 Suggested formatting fixes
-**Roles:**
-| Role | Permissions Array | Capabilities |
-|------|------------------|--------------|
-| `owner` | `['owner']` | Full control: settings, members, view all intents |
-| `admin` | `['admin', 'member']` | Manage members (add/remove), cannot promote to owner |
-| `member` | `['member']` | View own intents, manage own index membership |
+**Roles:**
+
+| Role | Permissions Array | Capabilities |
+| ---- | ----------------- | ------------ |
+| `owner` | `['owner']` | Full control: settings, members, view all intents |
+| `admin` | `['admin', 'member']` | Manage members (add/remove), cannot promote to owner |
+| `member` | `['member']` | View own intents, manage own index membership |Also applies to: 56-59, 63-70, 91-96, 670-677, 938-945
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 48-48: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 49-49: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 49-49: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 49-49: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 49-49: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 49-49: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 49-49: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@plans/index-ownership-chat-implementation.md` around lines 47 - 53, The
markdown tables (starting at the "**Roles:**" table containing `owner`, `admin`,
and `member`) violate markdownlint rules: add a blank line above and below each
table and normalize pipe spacing so there is a single space after and before
each pipe (e.g., "| Role | Permissions Array | Capabilities |"), ensure header
separator rows use the same compact-pipe spacing
("|------|------------------|--------------|"), and apply the same fixes to the
other tables that include the same role names (`owner`, `admin`, `member`)
elsewhere in the file.
|
|
||
| ``` | ||
| User: "Show me the members of my AI Founders index" | ||
| │ | ||
| ▼ | ||
| ┌───────────┐ | ||
| │ Router │ ──▶ Detects: index ownership query | ||
| └─────┬─────┘ Target: index_query | ||
| │ extractedContext: "AI Founders" | ||
| ▼ | ||
| ┌───────────────┐ | ||
| │ load_context │ | ||
| └───────┬───────┘ | ||
| ▼ | ||
| ┌───────────────┐ | ||
| │ index_query │ ──▶ Check ownership | ||
| │ (enhanced) │ If owner: return full member list + intents | ||
| └───────┬───────┘ If not owner: return limited view | ||
| │ | ||
| ▼ | ||
| ┌───────────────────┐ | ||
| │ generate_response │ | ||
| └───────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
MD040 is triggered by unlabeled code fences.
🔧 Example fix
-```
+```text
User: "Show me the members of my AI Founders index"
│
▼
┌───────────┐Also applies to: 880-916, 920-932
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@plans/index-ownership-chat-implementation.md` around lines 102 - 125, The
markdown contains unlabeled fenced code blocks (the ASCII diagram and other code
fences) which trigger MD040; update each triple-backtick fence (e.g., the
diagram block starting with ``` and the other blocks around lines referenced in
the review) to include a language identifier like ```text (or ```ascii) so the
fences are labeled; search for occurrences of ``` in this document and replace
with ```text for plain diagrams or the appropriate language tag for code
snippets (for example the block that contains the Router → load_context →
index_query diagram).
| interface CreateIntentInput { | ||
| userId: string; | ||
| payload: string; | ||
| summary?: string | null; | ||
| embedding?: number[]; | ||
| isIncognito?: boolean; | ||
| sourceType?: SourceType | null; | ||
| sourceId?: string | null; | ||
| } |
There was a problem hiding this comment.
Require sourceType/sourceId for intent creation.
These fields are optional, which allows intents to be created without provenance.
✅ Enforce required origin fields
interface CreateIntentInput {
userId: string;
payload: string;
summary?: string | null;
embedding?: number[];
isIncognito?: boolean;
- sourceType?: SourceType | null;
- sourceId?: string | null;
+ sourceType: SourceType;
+ sourceId: string;
}As per coding guidelines "Intents must track their origin via polymorphic sourceType and sourceId fields".
🤖 Prompt for AI Agents
In `@protocol/src/adapters/database.adapter.ts` around lines 20 - 28, The
CreateIntentInput interface currently allows creation of intents without
provenance because sourceType and sourceId are optional; update the interface
declaration (CreateIntentInput) to make sourceType and sourceId non-optional
(remove the ? and avoid | null) and then audit and update any callers of
CreateIntentInput (constructors, factory functions, tests, and places where
intents are created or validated) to supply or derive valid sourceType and
sourceId values so compilation passes and intent creation always includes
provenance.
This update introduces chat deletion functionality in both the sidebar and chat view, displays unread message counts, and improves session/chat selection highlighting. It also adds a message CTA button to user profiles, refines input placeholder styling, and exposes an updateUser method in AuthContext for profile updates.
- Replace chat router/generator agents with graphs/chat agent, tools, nodes - Add chat.agent.ts, chat.tools.ts, streaming/, nodes/ under graphs/chat - Add operation.interface and extend database interface/adapter - Update chat controller and graph state; align intent/index/profile graphs - Remove agents/chat generator and router; update explicit inferrer, hyde, profile generator - Add README and REFACTORING_SUMMARY for chat graph
- chat.graph.state: add SubgraphResults, RoutingDecision, state fields (routingDecision, subgraphResults, userProfile) for legacy nodes - chat.graph.spec: type createIntent mock as CreateIntentData; cast MemorySaver checkpointer in streaming graph test - chat.tools.spec: fix expect overload using equality + toBe(true) - response.nodes: define local ResponseGeneratorAgent interface, optional getSuggestedActions, typed map callback - remove orchestration.nodes.spec (references removed modules)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@protocol/src/controllers/chat.controller.spec.ts`:
- Around line 220-222: The test relies on adapter.getActiveIntents returning at
least one non-archived intent but a prior test archives the only intent; to fix,
make this test create its own fresh intent before calling
adapter.getActiveIntents (or use a beforeEach to create a test intent), then use
that new intent's id when calling adapter.assignIntentToIndex; update references
to adapter.getActiveIntents and adapter.assignIntentToIndex in the test to use
the newly created intent (or add a beforeEach that calls the intent creation
helper) so the test no longer depends on the archived intent from the other
test.
🧹 Nitpick comments (6)
protocol/src/controllers/chat.controller.spec.ts (2)
1-11: Consider using Vitest and relocating this integration test file.Based on learnings, test files should be located in
protocol/tests/for integration/E2E tests and should use the Vitest framework. This file usesbun:testand is located inprotocol/src/controllers/.If there's a project-specific decision to use bun:test and co-locate controller tests, this can be ignored. Otherwise, consider:
- Moving to
protocol/tests/integration/chat.controller.spec.ts- Switching imports from
bun:testtovitest
55-55: Remove debug console.log statements from tests.Multiple
console.logstatements are left in the test file for debugging purposes. Consider removing them to keep test output clean.Also applies to: 75-75, 306-306, 309-309, 330-330, 333-333, 355-355, 358-358
protocol/src/controllers/chat.controller.ts (3)
56-91: Consider adding input length validation for message content.The endpoint validates that the message is non-empty but doesn't limit the maximum length. Very large messages could cause issues with LLM context windows or memory usage.
🛡️ Suggested fix
if (!messageContent.trim()) { return Response.json( { error: 'Message content is required' }, { status: 400 } ); } + const MAX_MESSAGE_LENGTH = 32000; // Adjust based on LLM context limits + if (messageContent.length > MAX_MESSAGE_LENGTH) { + return Response.json( + { error: `Message exceeds maximum length of ${MAX_MESSAGE_LENGTH} characters` }, + { status: 400 } + ); + } + // 2. Create graph and invoke with state
112-119: Add error handling for file content loading failures.If
loadFileContentthrows for a corrupted or inaccessible file, the entire request will fail. Consider catching errors per-file to allow partial success.🛠️ Suggested fix
for (const row of rows) { const ext = path.extname(row.name); const filePath = path.join(targetDir, row.id + ext); - const result = await loadFileContent(filePath); - if (result.content?.trim()) { - parts.push(`=== ${row.name} ===\n${result.content.substring(0, 10000)}`); + try { + const result = await loadFileContent(filePath); + if (result.content?.trim()) { + parts.push(`=== ${row.name} ===\n${result.content.substring(0, 10000)}`); + } + } catch (error) { + logger.warn('Failed to load file content', { fileId: row.id, fileName: row.name, error: error instanceof Error ? error.message : String(error) }); } }
202-287: Consider adding a stream timeout to prevent resource exhaustion.Long-running SSE streams without activity could hold connections indefinitely. Consider implementing a timeout that sends a heartbeat or closes inactive streams.
protocol/src/adapters/database.adapter.ts (1)
260-264: Partial name matching may cause ambiguous results.The
includes()check on line 262 could match multiple indexes with similar names. For example, searching "AI" would match both "AI Projects" and "Fair AI Ethics". Consider prioritizing exact matches or returning an error when multiple matches exist.🛠️ Suggested improvement
const needle = indexNameOrId.trim().toLowerCase(); - const match = memberships.find( - (m) => (m.indexTitle ?? '').toLowerCase() === needle || (m.indexTitle ?? '').toLowerCase().includes(needle) - ); + // Prioritize exact match, then fall back to partial match + const exactMatch = memberships.find( + (m) => (m.indexTitle ?? '').toLowerCase() === needle + ); + const partialMatches = memberships.filter( + (m) => (m.indexTitle ?? '').toLowerCase().includes(needle) + ); + const match = exactMatch ?? (partialMatches.length === 1 ? partialMatches[0] : null); indexId = match?.indexId ?? null;
| const activeIntents = await adapter.getActiveIntents(testUserId); | ||
| expect(activeIntents.length).toBeGreaterThan(0); | ||
| await adapter.assignIntentToIndex(activeIntents[0].id, testIndexId); |
There was a problem hiding this comment.
Test may fail due to state dependency on archived intent.
This test expects activeIntents.length > 0, but the preceding archiveIntent test (lines 177-191) archives the only intent created. Since getActiveIntents excludes archived intents, this assertion will likely fail.
Either:
- Create a new intent at the start of this test
- Reorder tests so
archiveIntentruns after index-related tests - Use
beforeEachto ensure test isolation
🛠️ Suggested fix
test("getIntentsInIndexForMember should return intents when queried by index name", async () => {
const [index] = await db.insert(schema.indexes).values({
title: "Open Mock Network",
prompt: "Test index for chat adapter",
}).returning({ id: schema.indexes.id });
if (!index) throw new Error("Failed to create index");
testIndexId = index.id;
await db.insert(schema.indexMembers).values({
indexId: testIndexId,
userId: testUserId,
permissions: [],
autoAssign: false,
});
+ // Create a fresh intent for this test since previous intent was archived
+ await adapter.createIntent({
+ userId: testUserId,
+ payload: "Looking for collaborators on a machine learning project",
+ summary: "ML collaboration",
+ sourceType: 'discovery_form' as const,
+ });
+
const activeIntents = await adapter.getActiveIntents(testUserId);
expect(activeIntents.length).toBeGreaterThan(0);🤖 Prompt for AI Agents
In `@protocol/src/controllers/chat.controller.spec.ts` around lines 220 - 222, The
test relies on adapter.getActiveIntents returning at least one non-archived
intent but a prior test archives the only intent; to fix, make this test create
its own fresh intent before calling adapter.getActiveIntents (or use a
beforeEach to create a test intent), then use that new intent's id when calling
adapter.assignIntentToIndex; update references to adapter.getActiveIntents and
adapter.assignIntentToIndex in the test to use the newly created intent (or add
a beforeEach that calls the intent creation helper) so the test no longer
depends on the archived intent from the other test.
… hide member emails - Add list_index_members: owner-only tool to list members with name, avatar, permissions, intent count, joined date (no email for privacy) - Add list_index_intents: owner-only tool to list all intents in owned index with payload, summary, userName, createdAt - Update agent system prompt with new tool descriptions - Add tests for both tools (owner success, name resolution, non-owner error)
Introduces a new dynamic route at /d/[id] for discovery sessions and updates ChatContent, Sidebar, and ClientWrapper to use this route instead of query parameters for session IDs. Refactors session ID extraction and URL updates to use the pathname, improving navigation and session management.
Enabled the autoFocus attribute on input fields in ChatContent and ChatView components to improve user experience by focusing the input automatically when the component mounts.
Deleted all admin index-related pages under /admin/[indexId] including approvals, directory, opportunities, main, and settings. Added new IndexOwnerModal and IndexSelectorModal components. Updated Sidebar and several modal components to reflect these changes, likely as part of a refactor or redesign of the admin interface.
2 participants
Overview
This PR introduces a comprehensive LangGraph-based protocol architecture with a streaming AI chat system, enabling real-time conversational AI interactions with the platform.
Major Features
🤖 LangGraph Protocol Architecture
💬 Streaming Chat System
🎨 Frontend Components
🚀 V2 Server Architecture
📊 Database Enhancements
📚 Documentation
Technical Improvements
Dependencies
Breaking Changes
None - this is additive functionality building on existing infrastructure
Testing
Total Commits: 44
Lines Changed: Extensive additions across backend and frontend
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.