Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Review Summary by QodoDynamic AI model discovery, vector-based knowledge search, and article link widget integration
WalkthroughsDescription**Core Features:** • Implemented dynamic AI model discovery from gateway with fallback support and timeout handling • Added vector-based knowledge search with embeddings for semantic relevance in inbox • Integrated embedding model resolution and compatibility checking across suggestions, embeddings, and knowledge modules • Added article protocol link support (article:<articleId>) in markdown rendering for widget navigation • Implemented embeddings backfill functionality for admin tools **AI Agent Improvements:** • Changed listAvailableModels from static query to dynamic action-based discovery • Enhanced AI response tracking with model and provider metadata persistence • Improved model format validation to accept provider-prefixed and raw model IDs • Added getAIGatewayProviderLabel() utility for flexible provider detection • Refactored AI settings UI with model discovery fallback and manual entry support **Knowledge & Suggestions:** • Implemented searchWithEmbeddings action for vector-based knowledge search • Added embedding model tracking in suggestion feedback with embeddingModel field • Created embedding model compatibility module with normalization and validation • Updated suggestions panel to display active embedding model **Infrastructure & Testing:** • Upgraded Convex from 1.32.0 to 1.35.1 and Next.js to 15.5.15 • Refactored test admin mutation argument handling to use JSON strings • Added comprehensive test coverage for AI model discovery, embedding models, and article links • Created Convex framework guidelines documentation for development patterns **UI/UX Enhancements:** • Added article link click handling in widget messages with data-article-id attribute • Refactored inbox thread pane layout with improved state management • Enhanced articles page with embeddings backfill button and feedback display • Added AI response metadata display (model/provider) in review panel • Improved Home component config handling with nullish coalescing **Security & Documentation:** • Added undici vulnerability allowlist entries for dev dependencies • Removed expired testAdmin v.any() exception • Updated API reference documentation with new AI functions and validation procedures • Created comprehensive openspec documentation for article link integration and vector search Diagramflowchart LR
A["AI Gateway"] -->|"discover models"| B["listAvailableModels<br/>Action"]
B -->|"cached results"| C["Settings UI"]
C -->|"selected model"| D["AI Agent"]
E["Articles"] -->|"embeddings"| F["Vector Search<br/>Action"]
F -->|"semantic results"| G["Inbox Knowledge"]
G -->|"article:ID format"| H["Widget Navigation"]
I["Embedding Models<br/>Module"] -->|"resolve & validate"| J["Suggestions"]
I -->|"resolve & validate"| K["Knowledge Retrieval"]
I -->|"resolve & validate"| L["Embeddings Generation"]
M["Markdown Parser"] -->|"detect article links"| N["data-article-id<br/>attribute"]
N -->|"click handler"| H
File Changes1. packages/convex/convex/aiAgent.ts
|
Code Review by Qodo
1. CLAUDE.md uses npx
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
|
||
| When working on Convex code, **always read `convex/_generated/ai/guidelines.md` first** for important guidelines on how to correctly use Convex APIs and patterns. The file contains rules that override what you may have learned about Convex from training data. | ||
|
|
||
| Convex agent skills for common tasks can be installed by running `npx convex ai-files install`. |
There was a problem hiding this comment.
1. claude.md uses npx 📘 Rule violation § Compliance
The new CLAUDE.md instructs running npx convex ai-files install, which violates the requirement to use pnpm for package management/CLI execution. This introduces non-standard tooling guidance for the repo.
Agent Prompt
## Issue description
`CLAUDE.md` includes a `npx` command (`npx convex ai-files install`), which violates the repo’s PNPM-only rule.
## Issue Context
Docs are included in the compliance scan; guidance must not recommend `npx`/`npm`/`yarn` when `pnpm` alternatives exist.
## Fix Focus Areas
- CLAUDE.md[1-7]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
128
to
+135
| const GET_AI_SETTINGS_REF: SuggestionQueryRef< | ||
| { workspaceId: Id<"workspaces"> }, | ||
| Doc<"aiAgentSettings"> | null | ||
| > = makeFunctionReference<"query", { workspaceId: Id<"workspaces"> }, Doc<"aiAgentSettings"> | null>( | ||
| "suggestions:getAiSettings" | ||
| ); | ||
| > = makeFunctionReference< | ||
| "query", | ||
| { workspaceId: Id<"workspaces"> }, | ||
| Doc<"aiAgentSettings"> | null | ||
| >("suggestions:getAiSettings"); |
There was a problem hiding this comment.
2. get_ai_settings_ref string ref 📘 Rule violation ⌂ Architecture
GET_AI_SETTINGS_REF is created via makeFunctionReference("suggestions:getAiSettings") even
though generated internal refs are available. This violates the required ordering for Convex
backend function references and bypasses codegen-based correctness guarantees.
Agent Prompt
## Issue description
`suggestions.ts` defines `GET_AI_SETTINGS_REF` using `makeFunctionReference("suggestions:getAiSettings")` even though a generated `internal` ref should be used.
## Issue Context
The backend reference ordering requires using generated refs first to avoid string drift and to keep references aligned with Convex codegen.
## Fix Focus Areas
- packages/convex/convex/suggestions.ts[128-135]
- packages/convex/convex/_generated/api.d.ts[392-403]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+284
to
+299
| const vectorResults = await ctx.vectorSearch("contentEmbeddings", "by_embedding", { | ||
| vector: embedding, | ||
| limit: limit * 8, | ||
| filter: (q) => q.eq("workspaceId", args.workspaceId), | ||
| }); | ||
|
|
||
| const contentTypeSet = | ||
| args.contentTypes && args.contentTypes.length > 0 ? new Set(args.contentTypes) : null; | ||
|
|
||
| const embeddingDocs = await Promise.all( | ||
| vectorResults.map((result) => | ||
| runQuery(internal.suggestions.getEmbeddingById, { id: result._id }).then((doc) => | ||
| doc ? { ...doc, _score: result._score } : null | ||
| ) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
3. Vector search query fan-out 🐞 Bug ➹ Performance
knowledge:searchWithEmbeddings performs limit * 8 vector search and then does Promise.all runQuery calls per match; at the UI default (limit=20) this can trigger ~160 internal queries per search and risks slowdowns/timeouts.
Agent Prompt
### Issue description
`knowledge:searchWithEmbeddings` does a vectorSearch with `limit * 8` and then performs one internal `runQuery` per vector result to fetch the embedding doc. With the inbox default limit (20), this becomes ~160 internal calls per search, which is likely to cause slow actions and/or timeouts.
### Issue Context
The fan-out happens even though only `limit` deduped results are ultimately returned.
### Fix Focus Areas
- packages/convex/convex/knowledge.ts[284-338]
- apps/web/src/app/inbox/hooks/useInboxConvex.ts[181-207]
### Implementation notes
- Reduce the `limit * 8` multiplier (or make it adaptive) to minimize worst-case work.
- Introduce a single internal query like `internal.suggestions.getEmbeddingsByIds({ ids: Id<"contentEmbeddings">[] })` to fetch embedding docs in batch.
- Consider an internal query that directly returns the minimal fields needed (contentType/contentId/title/snippet/updatedAt) for a list of embedding ids to avoid repeated round-trips.
- If batching isn’t feasible, at least cap concurrency and/or stop fetching once enough deduped results are found.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+74
to
+90
| const handleMessageClick = (event: React.MouseEvent<HTMLDivElement>) => { | ||
| const target = event.target; | ||
| if (!target) { | ||
| return; | ||
| } | ||
| const element = target instanceof Element ? target : (target as Text).parentElement; | ||
| if (!element) { | ||
| return; | ||
| } | ||
| const articleLink = element.closest("[data-article-id]"); | ||
| if (articleLink) { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| const articleId = articleLink.getAttribute("data-article-id"); | ||
| if (articleId) { | ||
| onSelectArticle(articleId as Id<"articles">); | ||
| } |
There was a problem hiding this comment.
4. Unvalidated article-id click 🐞 Bug ☼ Reliability
The widget treats any clicked element with [data-article-id] as an article link and casts the
attribute value to Id<"articles"> without validation; because shared markdown allows raw HTML and
now allows data-article-id, crafted message content can trigger Convex v.id("articles") argument
validation errors at runtime.
Agent Prompt
### Issue description
The widget click handler opens an article for any element matching `[data-article-id]` and casts the attribute to `Id<"articles">` without runtime validation. Because shared markdown supports raw HTML and now allows `data-article-id` through sanitization, a crafted message can inject an invalid `data-article-id` and cause Convex `v.id("articles")` argument validation errors when the widget tries to fetch the article.
### Issue Context
- `@opencom/web-shared` markdown uses `MarkdownIt({ html: true })`.
- DOMPurify allows `data-article-id`.
- The click handler matches `[data-article-id]` regardless of whether the element is a validated `article:` link.
### Fix Focus Areas
- packages/web-shared/src/markdown.ts[4-35]
- packages/web-shared/src/markdown.ts[92-139]
- apps/widget/src/components/conversationView/MessageList.tsx[74-91]
### Implementation notes
- In `enforceSafeLinksAndMedia`, proactively remove `data-article-id` from all elements/anchors by default, and only set it when the href is a validated `article:<id>` link.
- In the widget click handler, validate `articleId` before calling `onSelectArticle` (e.g., ensure it matches the same strict format you generate/accept for article ids) and ignore clicks when invalid.
- Optionally scope the selector to anchors only (e.g., `closest('a[data-article-id]')`) to reduce accidental matches.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.