fix: improve embed location intent guidance and highlighting - #18
Merged
Merged
Conversation
Improve demo embed chat handling for location-intent prompts, enrich page context, and harden UI highlight targeting so create-account and sign-up queries produce actionable guidance instead of generic fallback text. Made-with: Cursor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request adds client-side UI highlighting and intent detection for navigation-style questions across the embedded runbook assistant. Changes include element scoring and visual overlay logic in the client, enhanced retrieval with multiple embedding queries, extended system prompts for onboarding/location inference, demo mode support in API responses, and improved error handling with fallback instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Browser
participant Embed as runbook-embed.js
participant Component as EmbeddedRunbookAssistant
participant API as /embed/chat Route
participant Retrieval as Retrieval & Embedding
participant LLM as Claude API
Client->>Embed: Load page + ask navigation question
Embed->>Embed: Detect intent (where/find/click/open)
Embed->>Component: Send question with page context
Component->>API: POST /embed/chat
API->>Retrieval: Retrieve docs (multiple queries + dedup)
Retrieval->>LLM: Send context + extended system prompt
LLM->>API: Return response + mode flag
API->>Component: Return ChatResponse {answer, sources, mode}
Component->>Component: Check for missing-index signal<br/>(mode=fallback or text patterns or empty sources)
alt Missing Index Detected
Component->>Component: Prepend assistant message<br/>with index guidance
end
Component->>Embed: Trigger maybeHighlight(question, response)
Embed->>Embed: Extract target label from response
Embed->>Embed: Score page elements against label
Embed->>Embed: Scroll to best-match element
alt Match Found
Embed->>Client: Show animated pulse overlay + tooltip
else No Match Found
Embed->>Component: Emit assistant message<br/>requesting clarification
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/components/EmbeddedRunbookAssistant.tsx (1)
113-128:⚠️ Potential issue | 🟡 MinorSame
mode === "fallback"conflation as the route handler.This shares the issue flagged on
src/app/api/embed/chat/route.tslines 234-249: a transient AI failure now triggers the “Codebase not indexed yet. Connect your GitHub repo in Studio…” banner even when the index is healthy. The fix is best applied at the root cause (distinct mode for AI errors vs. unindexed) so this branch can stay narrow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EmbeddedRunbookAssistant.tsx` around lines 113 - 128, The banner is incorrectly shown when data.mode === "fallback" (AI transient failure); remove that check from the missingIndexSignal and instead only treat the response as "unindexed" when there's an explicit unindexed indicator or sourceCount === 0 (or when data.mode === "unindexed" if you introduce that server-side), i.e., change the missingIndexSignal computation to NOT include data.mode === "fallback" and rely on sourceCount === 0 or a distinct data.mode/value that represents an actually unindexed repo before pushing the "Codebase not indexed yet..." assistant message via setMessages.
🧹 Nitpick comments (6)
src/lib/embedRetrieval.ts (2)
57-57:match_count: TOP_K * 8is large when multiplied across 3 queries.With
TOP_K = 6, each call asks for up to 48 rows; for flow-intent questions that's 144 rows fetched, marshalled, and post-filtered just to keep the top 6. Consider trimming the per-query fan-out (e.g.,TOP_K * 4) now that the multi-query merge is expanding recall on its own.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/embedRetrieval.ts` at line 57, Reduce the per-query fan-out by changing the match_count expression from TOP_K * 8 to a smaller multiplier (e.g., TOP_K * 4) in src/lib/embedRetrieval.ts; update the occurrence where match_count is set (the line currently using "match_count: TOP_K * 8") so the code asks for fewer rows per query while leaving the TOP_K constant and the multi-query merge logic unchanged. Ensure any tests or callers that assume the larger fan-out still pass or adjust assertions accordingly.
47-71: Run embedding queries in parallel to avoid latency tripling.For flow-intent questions,
buildEmbedQueriesreturns 3 strings. The current loop awaits eachgenerateEmbedding(...)and eachsupabaseAdmin.rpc("match_documents", …)in sequence, so a single chat request now incurs roughly 3× the embedding+RPC round trips it used to. On a hot path served from a request thread this is a meaningful slowdown.Running the queries concurrently keeps the same merge semantics with one round-trip’s worth of latency:
♻️ Suggested parallelization
- for (const query of queries) { - const embedding = await generateEmbedding(query); - if (!embedding) continue; - - const { data: documents, error } = await supabaseAdmin.rpc("match_documents", { - query_embedding: `[${embedding.join(",")}]`, - match_threshold: DEFAULT_MATCH_THRESHOLD, - match_count: TOP_K * 8 - }); - - if (error) { - console.error("Embed vector search error:", error); - continue; - } - - const batch = (documents || []) as MatchedDocument[]; - for (const doc of batch) { - if (!matchesEmbedScope(doc.content, projectId)) continue; - const prev = merged.get(doc.id); - if (!prev || doc.similarity > prev.similarity) merged.set(doc.id, doc); - } - } + const results = await Promise.all( + queries.map(async (query) => { + const embedding = await generateEmbedding(query); + if (!embedding) return [] as MatchedDocument[]; + const { data, error } = await supabaseAdmin.rpc("match_documents", { + query_embedding: `[${embedding.join(",")}]`, + match_threshold: DEFAULT_MATCH_THRESHOLD, + match_count: TOP_K * 8 + }); + if (error) { + console.error("Embed vector search error:", error); + return [] as MatchedDocument[]; + } + return (data || []) as MatchedDocument[]; + }) + ); + + for (const batch of results) { + for (const doc of batch) { + if (!matchesEmbedScope(doc.content, projectId)) continue; + const prev = merged.get(doc.id); + if (!prev || doc.similarity > prev.similarity) merged.set(doc.id, doc); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/embedRetrieval.ts` around lines 47 - 71, The loop is serially awaiting generateEmbedding and supabaseAdmin.rpc for each query; refactor embed retrieval to run per-query work in parallel by mapping buildEmbedQueries(question) to an array of promises that concurrently call generateEmbedding(query) then, if embedding exists, call supabaseAdmin.rpc("match_documents", ...) and return the batch (or an error marker), then await Promise.all on those promises, iterate the resolved results and apply the same filtering/merge logic (use matchesEmbedScope(doc.content, projectId) and the merged Map to keep the highest similarity per doc id). Preserve existing error handling by logging per-promise RPC errors and skip those results, and ensure the final merged Map semantics remain identical to the original sequential code.test-embed.html (1)
76-83: Optional: use real<label>elements for inputs.The fields use
<p>Email</p>/<p>Password</p>plusaria-label, which works for screen readers but loses<label for="…">/<input id="…">association (clicking the visible text won’t focus the input). Since this harness exists to exercise the embed’s element-targeting heuristics (which includelabelin its selector), a real<label>would also give the highlighter a richer surface to score against.- <p>Email</p> - <input aria-label="Email address" placeholder="name@company.com" /> - <p style="margin-top: 12px">Password</p> - <input aria-label="Password" placeholder="Password" type="password" /> + <label for="email-field">Email</label> + <input id="email-field" aria-label="Email address" placeholder="name@company.com" /> + <label for="password-field" style="margin-top: 12px">Password</label> + <input id="password-field" aria-label="Password" placeholder="Password" type="password" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-embed.html` around lines 76 - 83, Replace the decorative <p> elements with real <label> elements and associate them to the inputs by adding unique id attributes to each input (e.g., id="email-input" and id="password-input") and using <label for="email-input">Email</label> and <label for="password-input">Password</label>; keep the existing aria-label and placeholder attributes and the existing buttons (ids "log-in-btn" and "reset-password-btn") unchanged so the embed/highlighter can match labels as well as aria attributes.public/runbook-embed.js (1)
213-344: Highlight engine is duplicated verbatim with the React component.
ensurePageHighlightStyles,clearPageHighlight,tokenize,compact,extractIntentPhrase,scoreCandidate/findBestElement,showPageOverlay,maybeHighlight, andisLocationIntentare reproduced almost line-for-line insrc/components/EmbeddedRunbookAssistant.tsx(lines 375-532). They share the same stylesheet idrb-page-highlight-styleand the samerb-highlight-targetclass, so if both ever load on the same page they will race on cleanup and potentially clobber each other's overlay/timeout state.Recommend extracting the shared logic into a single module (e.g.,
src/lib/embedHighlight.ts) and consuming it from both the IIFE and the React component, or — at minimum — keeping the regexes/scoring constants in one canonical place. This will also stop the intent regex from drifting between the three files where it currently appears (embedDemoKnowledge.ts,runbook-embed.js,EmbeddedRunbookAssistant.tsx).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/runbook-embed.js` around lines 213 - 344, The highlight engine is duplicated across files causing races and drift; extract the shared functions (ensurePageHighlightStyles, clearPageHighlight, tokenize, compact, extractIntentPhrase, scoreCandidate, findBestElement, showPageOverlay, maybeHighlight, and isLocationIntent) into a single module (e.g., src/lib/embedHighlight.ts) that exports these functions and any scoring/regex constants, update EmbeddedRunbookAssistant.tsx and public/runbook-embed.js to import/use that module (and also update embedDemoKnowledge.ts to use the same isLocationIntent/regex), and ensure the single stylesheet id ("rb-page-highlight-style"), class names ("rb-highlight-target","rb-highlight-overlay") and shared overlay/timeout state are managed from that module so both consumers call the same cleanup and avoid clobbering each other.src/app/api/embed/chat/route.ts (1)
178-179: Onboarding instruction overlaps with the system prompt.The new sentence at line 178 largely restates
EMBED_CHAT_SYSTEM_PROMPTlines 4-9 insrc/lib/prompts.ts. Keeping it in both places risks the two instructions drifting out of sync. Consider relying solely on the system prompt for onboarding behavior and keeping the user-prompt suffix focused on theRUNBOOK_STEPS_JSON:contract on line 179.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/embed/chat/route.ts` around lines 178 - 179, Remove the duplicated onboarding sentence from the user prompt in the chat route and rely on the existing EMBED_CHAT_SYSTEM_PROMPT for onboarding behavior; update the user-prompt suffix to only enforce the RUNBOOK_STEPS_JSON contract (i.e., append the JSON steps line exactly as specified) and ensure the route’s prompt construction uses EMBED_CHAT_SYSTEM_PROMPT as the single source of truth for onboarding instructions to avoid drift between prompts.src/components/EmbeddedRunbookAssistant.tsx (1)
418-507:findBestTargetandfindBestTargetWithIntentoverlap heavily; consolidate.The two helpers iterate the same node set with the same selector, the same
tokensloop, and the same threshold. The only differences are the intent-phrase bonus and the “create/sign up/register/get started/start/continue/next” keyword bonus.maybeHighlightElementForQuestionalready callsfindBestTargetWithIntentfirst and falls back tofindBestTarget, butfindBestTargetWithIntentis a strict superset offindBestTarget's scoring (the intent/keyword bonuses can only add), so the fallback can never win. You can dropfindBestTargetand pass an emptyintentCompact/skip the keyword bonus when not desired.Also note this drifts from
public/runbook-embed.js'sfindBestElement, which always applies the intent bonus — so identical inputs may pick different targets between the React preview and the embedded script.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EmbeddedRunbookAssistant.tsx` around lines 418 - 507, The two functions findBestTarget and findBestTargetWithIntent are redundant because the latter is a strict superset; remove findBestTarget and consolidate into a single function (keep the name findBestTargetWithIntent or rename to findBestTarget) that accepts an optional intentCompact (string) and an options flag (e.g., { applyKeywordBonus: boolean }) so callers can pass an empty intentCompact or set applyKeywordBonus=false to mimic the old behavior; update maybeHighlightElementForQuestion to call the unified function accordingly, keep the same node selector, token loop, scoring thresholds, and scoring bonuses, and ensure the unified implementation mirrors public/runbook-embed.js's findBestElement behavior for parity (always apply intent bonus there or make behavior configurable) so identical inputs produce the same target across both contexts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/runbook-embed.js`:
- Around line 352-361: The current capture of document.body.innerText into the
bodyText variable exposes potential PII; change this to opt-in and safer
extraction: only collect body text when the embedding script has a
data-include-body-text attribute (check document.currentScript or
querySelector('script[data-include-body-text]')), otherwise leave bodyText empty
and return only title/meta/heading landmarks; when opt-in, sanitize by removing
values of form controls and text from <input> and <textarea> elements before
building bodyText (strip or replace their values), limit length as before, and
ensure the return that joins [u, t, meta, bodyText] uses the sanitized/opt-in
bodyText variable so default behavior never sends arbitrary page content.
In `@src/app/api/embed/chat/route.ts`:
- Around line 234-249: The catch block that handles errors from
generateFromGemini is returning mode: "fallback" which conflates transient AI
failures with an unindexed codebase; change the response in that catch (the
NextResponse.json that's returning answer, sources: baseSources, mode, steps) to
use mode: "error" (or another distinct value) for AI/API failures, keep mode:
"fallback" reserved for the unindexed/missing-index path, and update client-side
checks (the logic that inspects mode to show the “connect your repo / re-index”
banner) to only treat mode === "fallback" as the missing-index case while
ignoring mode === "error".
- Around line 28-34: The originAllowed logic currently enforces strict equality
between allowed.hostname and actual.hostname which blocks valid subdomains;
update the check in the try block (the code that creates `allowed` and `actual`
from `projectSite` and `origin`) to allow either exact hostname match OR a
subdomain match — e.g. return true if `actual.hostname === allowed.hostname ||
actual.hostname.endsWith('.' + allowed.hostname)` — keeping the existing URL
parsing and try/catch structure intact so malformed URLs still return false.
In `@src/components/EmbeddedRunbookAssistant.tsx`:
- Line 60: The highlight cleanup stored in highlightCleanupRef is never invoked
on component unmount, so add a useEffect(() => () => {
highlightCleanupRef.current?.(); }) to EmbeddedRunbookAssistant to call
highlightCleanupRef.current() during cleanup; ensure you reference the existing
highlightCleanupRef and that maybeHighlightElementForQuestion’s assigned cleanup
is invoked so the rb-highlight-target class and overlay are removed when the
component unmounts (also apply same cleanup pattern where highlightCleanupRef is
used around maybeHighlightElementForQuestion).
- Around line 138-150: The fallback assistant bubble from
maybeHighlightElementForQuestion is being appended too aggressively even when
the model already provided a target or when step data exists; change the logic
that currently sets the fallback message in the block handling
maybeHighlightElementForQuestion so you only append that extra assistant message
when there are no data.steps AND the latest assistant/model answer does not
already include a target label (i.e., inspect the last assistant message text
from messages or the model response for phrases/labels), otherwise skip adding
the extra message and just set highlightCleanupRef.current = () => undefined;
additionally consider gating this only for location intents resolved by
findBestTargetWithIntent/findBestTarget so non-location answers never trigger
the fallback.
In `@src/lib/embedDemoKnowledge.ts`:
- Around line 48-63: The response for location intents in the isLocationIntent
branch currently hard-codes "account creation flow"; change the answer to use
the extracted target (from extractLocationTarget(message) assigned to target) or
a neutral phrase so it reads correctly for any intent—update the returned object
in the isLocationIntent block (where target, product, and excerptFromContent are
used) to replace the account-specific sentence with something like a template
referencing ${target} or a generic "that action" wording and adjust any step
text if it references account creation.
- Around line 20-24: The isLocationIntent function's regex is too broad (it
includes "how do i" and lacks word boundaries), causing demo-specific prompts to
be misclassified; tighten it by removing the bare "how do i" alternative, add
word boundaries (\b) around single-word triggers like
find/register/create/account/signup/login/log in, and restrict phrase triggers
to genuinely UI-locator forms such as "where", "where is", "where can I",
"locate", "click", "open", "go to", or "get started"; update the regex inside
isLocationIntent to use these bounded/phrased terms so existing
GitHub/local-setup/first-week branches are no longer short-circuited.
---
Duplicate comments:
In `@src/components/EmbeddedRunbookAssistant.tsx`:
- Around line 113-128: The banner is incorrectly shown when data.mode ===
"fallback" (AI transient failure); remove that check from the missingIndexSignal
and instead only treat the response as "unindexed" when there's an explicit
unindexed indicator or sourceCount === 0 (or when data.mode === "unindexed" if
you introduce that server-side), i.e., change the missingIndexSignal computation
to NOT include data.mode === "fallback" and rely on sourceCount === 0 or a
distinct data.mode/value that represents an actually unindexed repo before
pushing the "Codebase not indexed yet..." assistant message via setMessages.
---
Nitpick comments:
In `@public/runbook-embed.js`:
- Around line 213-344: The highlight engine is duplicated across files causing
races and drift; extract the shared functions (ensurePageHighlightStyles,
clearPageHighlight, tokenize, compact, extractIntentPhrase, scoreCandidate,
findBestElement, showPageOverlay, maybeHighlight, and isLocationIntent) into a
single module (e.g., src/lib/embedHighlight.ts) that exports these functions and
any scoring/regex constants, update EmbeddedRunbookAssistant.tsx and
public/runbook-embed.js to import/use that module (and also update
embedDemoKnowledge.ts to use the same isLocationIntent/regex), and ensure the
single stylesheet id ("rb-page-highlight-style"), class names
("rb-highlight-target","rb-highlight-overlay") and shared overlay/timeout state
are managed from that module so both consumers call the same cleanup and avoid
clobbering each other.
In `@src/app/api/embed/chat/route.ts`:
- Around line 178-179: Remove the duplicated onboarding sentence from the user
prompt in the chat route and rely on the existing EMBED_CHAT_SYSTEM_PROMPT for
onboarding behavior; update the user-prompt suffix to only enforce the
RUNBOOK_STEPS_JSON contract (i.e., append the JSON steps line exactly as
specified) and ensure the route’s prompt construction uses
EMBED_CHAT_SYSTEM_PROMPT as the single source of truth for onboarding
instructions to avoid drift between prompts.
In `@src/components/EmbeddedRunbookAssistant.tsx`:
- Around line 418-507: The two functions findBestTarget and
findBestTargetWithIntent are redundant because the latter is a strict superset;
remove findBestTarget and consolidate into a single function (keep the name
findBestTargetWithIntent or rename to findBestTarget) that accepts an optional
intentCompact (string) and an options flag (e.g., { applyKeywordBonus: boolean
}) so callers can pass an empty intentCompact or set applyKeywordBonus=false to
mimic the old behavior; update maybeHighlightElementForQuestion to call the
unified function accordingly, keep the same node selector, token loop, scoring
thresholds, and scoring bonuses, and ensure the unified implementation mirrors
public/runbook-embed.js's findBestElement behavior for parity (always apply
intent bonus there or make behavior configurable) so identical inputs produce
the same target across both contexts.
In `@src/lib/embedRetrieval.ts`:
- Line 57: Reduce the per-query fan-out by changing the match_count expression
from TOP_K * 8 to a smaller multiplier (e.g., TOP_K * 4) in
src/lib/embedRetrieval.ts; update the occurrence where match_count is set (the
line currently using "match_count: TOP_K * 8") so the code asks for fewer rows
per query while leaving the TOP_K constant and the multi-query merge logic
unchanged. Ensure any tests or callers that assume the larger fan-out still pass
or adjust assertions accordingly.
- Around line 47-71: The loop is serially awaiting generateEmbedding and
supabaseAdmin.rpc for each query; refactor embed retrieval to run per-query work
in parallel by mapping buildEmbedQueries(question) to an array of promises that
concurrently call generateEmbedding(query) then, if embedding exists, call
supabaseAdmin.rpc("match_documents", ...) and return the batch (or an error
marker), then await Promise.all on those promises, iterate the resolved results
and apply the same filtering/merge logic (use matchesEmbedScope(doc.content,
projectId) and the merged Map to keep the highest similarity per doc id).
Preserve existing error handling by logging per-promise RPC errors and skip
those results, and ensure the final merged Map semantics remain identical to the
original sequential code.
In `@test-embed.html`:
- Around line 76-83: Replace the decorative <p> elements with real <label>
elements and associate them to the inputs by adding unique id attributes to each
input (e.g., id="email-input" and id="password-input") and using <label
for="email-input">Email</label> and <label
for="password-input">Password</label>; keep the existing aria-label and
placeholder attributes and the existing buttons (ids "log-in-btn" and
"reset-password-btn") unchanged so the embed/highlighter can match labels as
well as aria attributes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14171f7e-8828-453f-8e44-486e5fb7ae41
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
public/runbook-embed.jssrc/app/api/embed/chat/route.tssrc/components/EmbeddedRunbookAssistant.tsxsrc/lib/embedDemoKnowledge.tssrc/lib/embedNorthstarChat.tssrc/lib/embedRetrieval.tssrc/lib/prompts.tstest-embed.html
Harden embed privacy and error semantics, tighten location-intent handling, improve highlight cleanup behavior, parallelize embed retrieval queries, and update the test harness accessibility labels. Made-with: Cursor
|
Deployment failed with the following error: Learn More: https://vercel.com/domenic-federicos-projects?upgradeToPro=build-rate-limit |
This was referenced Apr 26, 2026
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
2 participants
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.
Improve demo embed chat handling for location-intent prompts, enrich page context, and harden UI highlight targeting so create-account and sign-up queries produce actionable guidance instead of generic fallback text.
Summary by CodeRabbit
New Features
Improvements