[superlog] Resolve websiteId in annotation tools to fix domain-name mismatch#474
[superlog] Resolve websiteId in annotation tools to fix domain-name mismatch#474superlog-app[bot] wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Greptile SummaryThis PR fixes annotation tools (
Confidence Score: 3/5The core fix is directionally correct but the new ctx.websiteDomain branch in resolveToolWebsite uses the wrong ID precedence, which can silently route an annotation to the wrong website in multi-website contexts. The ctx.websiteDomain resolution path returns ctx.defaultWebsiteId ?? ctx.websiteId, but ctx.websiteDomain is the domain of ctx.websiteId, not ctx.defaultWebsiteId. In any session where those two IDs differ, domain-based resolution returns the default website's ID while labelling it with the current website's domain — an ID/domain mismatch that silently creates the annotation on the wrong property. packages/ai/src/ai/tools/utils/context.ts — the ctx.websiteDomain fallback branch; the new regression tests do not exercise a case where defaultWebsiteId and websiteId differ simultaneously, so this bug is not caught by the test suite. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["LLM passes websiteId e.g. 'databuddy.cc'"] --> B[resolveToolWebsite]
B --> C{Exact ID match in accessible or ctx.websiteId?}
C -- Yes --> D[Return internal ID + domain]
C -- No --> E{Domain match in accessible list?}
E -- Yes --> F[Return matched website ID]
E -- No --> G{Matches ctx.websiteDomain?}
G -- Yes --> H["Return ctx.defaultWebsiteId ?? ctx.websiteId ⚠️ Should prefer ctx.websiteId first"]
G -- No --> I[Throw: not in workspace]
D --> J[RPC call with correct internal ID]
F --> J
H --> J
Reviews (1): Last reviewed commit: "[superlog] Resolve websiteId in annotati..." | Re-trigger Greptile |
| if (ctx.websiteDomain === inputWebsiteId) { | ||
| const fallbackId = ctx.defaultWebsiteId ?? ctx.websiteId; | ||
| if (fallbackId) { | ||
| return { websiteId: fallbackId, domain: ctx.websiteDomain }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Wrong ID priority when resolving by
ctx.websiteDomain. ctx.websiteDomain is specifically the domain of ctx.websiteId, not of ctx.defaultWebsiteId. If a user's current context is website A (domain "databuddy.cc") but their default website is website B, passing "databuddy.cc" will incorrectly resolve to B while labelling it with A's domain — a silent ID/domain mismatch. Prefer ctx.websiteId over ctx.defaultWebsiteId in this branch.
| if (ctx.websiteDomain === inputWebsiteId) { | |
| const fallbackId = ctx.defaultWebsiteId ?? ctx.websiteId; | |
| if (fallbackId) { | |
| return { websiteId: fallbackId, domain: ctx.websiteDomain }; | |
| } | |
| } | |
| if (ctx.websiteDomain === inputWebsiteId) { | |
| const fallbackId = ctx.websiteId ?? ctx.defaultWebsiteId; | |
| if (fallbackId) { | |
| return { websiteId: fallbackId, domain: ctx.websiteDomain }; | |
| } | |
| } |
| const context = getAppContext(options); | ||
| const resolved = resolveToolWebsite(context, websiteId); | ||
| try { |
There was a problem hiding this comment.
Resolution error escapes the catch handler in
createAnnotationTool. resolveToolWebsite is called before the try block (unlike listAnnotationsTool where it is inside), so if resolution throws (e.g. domain not in workspace), the error propagates without being logged via logger.error. Moving the call into the try block would make error-handling consistent with listAnnotationsTool.
Summary
The insights agent's
create_annotation(andlist_annotations) tool was passing the LLM's rawwebsiteIdinput directly to the annotations RPC. The LLM consistently passes the website domain name (e.g.,"databuddy.cc") instead of the internal website ID (e.g.,"OXmNQsViBT-FOS_wZCTHc"), because it sees domain names appear to work aswebsiteIdin SQL queries — theexecute_sqltool silently overrides the model's value server-side, but no such override existed in the annotation tools. The result is aNOT_FOUNDerror on every annotation creation attempt.The root cause is two-part: (1) the annotation tools did not call
resolveToolWebsiteto obtain the real internal ID from context, unlikeexecute-sql-query.ts; and (2)resolveToolWebsiteonly matched by ID and would throw rather than fall back if the model happened to pass a domain name.This PR adds
resolveToolWebsite(context, websiteId)calls tocreateAnnotationToolandlistAnnotationsTool, and extendsresolveToolWebsiteto also match a domain name againstctx.websiteDomainandaccessibleWebsites[*].domain, returning the corresponding internal ID. All existingresolveToolWebsitetests continue to pass, and three new regression tests cover the domain-name matching paths.An alternative approach would be to strip the
websiteIdinput from the annotation tool schema entirely and always usectx.websiteId— that would be simpler for the insights-agent-only case but would break multi-website chat contexts where the user explicitly picks a website. The current fix preserves that flexibility while fixing the domain-name confusion.Incident on Superlog
Was this PR helpful? Leave feedback — goes straight to the Superlog team.
Summary by cubic
Fixes annotation tools so domain names like "databuddy.cc" resolve to the correct internal website ID. This removes NOT_FOUND errors when creating or listing annotations and keeps multi-website selection working.
websiteId.ctx.websiteDomainoraccessibleWebsites[*].domain, with clear errors if not found.Written for commit e3c93fc. Summary will update on new commits.