perf(ai): stage identity key sets for the profile queries - #764
Conversation
The hook resolved a single key set, which covered profile_list but not profile_sessions, where three separate sets feed the query. It now returns an array of stages, each naming the parameter it binds, and they run concurrently. profile_list becomes a one element array and still produces byte identical output against the single-query form.
profile_sessions read 4,474,325 rows and 440 MiB to return 4. The identity chain is the cause: target_identity_rows ran 58 times and the query made 173 base table scans, because target_anonymous_ids, target_session_ids and visitor_ids were each referenced as subqueries at seven sites and ClickHouse inlines a CTE per reference. All three are tiny per-profile key sets, 2 to 3 values here, so prepareSql now resolves them concurrently and the seven sites bind parameters instead. profileActivityCte takes the resolved keys and falls back to the original subqueries when they are absent, so profile_detail, which shares that helper, is untouched. Measured on the busiest website, three runs each: 1290ms to 580ms, 2.22x. Bound directly the query reads 623,095 rows and 50.7 MiB instead of 4.5M and 440 MiB. Output verified identical across three visitors and date ranges returning 28, 3 and 3 sessions, comparing the single-query form with plugins applied to both sides. An earlier comparison looked like a mismatch only because execute() enriches geo through applyPlugins and raw chQuery does not.
Three more visitor_ids subqueries live inside profileActivityCte itself, so profile_sessions was still re-running that chain for them. Binding them the same way takes it from 2.22x to 2.37x, 1467ms to 619ms on the busiest site. The prepare stages are now a shared profileIdentityPrepareStages helper rather than being spelled out inside profile_sessions. profile_detail deliberately does not opt in. It shares the helper and was the obvious next candidate, but measured at 574ms to 627ms, 0.92x: it returns a single row from a comparatively cheap query, so three prepare round trips cost more than the repeated subqueries they remove. It keeps the subquery fallback and is unchanged. Output verified identical for both builders across three visitors and date ranges, with plugins applied to both sides.
* feat(dashboard): add editable organization business context * fix(dashboard): preserve context provenance and organization setup
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Greptile SummaryThis PR adds multi-stage ClickHouse identity-key preparation for profile sessions and carries a broader organization business-context feature through dashboard, RPC, services, auth, and the Insights worker. The staged profile path improves query cost, while the business-context flow adds editable and AI-generated organization briefs with provenance and concurrency handling.
Confidence Score: 3/5The PR is not yet safe to merge because staged identity reads can return an inconsistent profile history and generated public-source content can be mislabeled as team-authored context. Two correctness defects affect the semantic integrity of profile-session results and the provenance supplied to investigations; the worker lifecycle, missing staged-path coverage, and explicit typing rule also need follow-up. Files Needing Attention: packages/ai/src/query/builders/profiles.ts, packages/services/src/organization-business-context.ts, apps/insights/src/organization-business-context.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
ProfileRequest[Profile sessions request] --> Prepare[Resolve identity key stages]
Prepare --> Anon[Anonymous IDs]
Prepare --> Sessions[Session IDs]
Prepare --> Visitors[Visitor IDs]
Anon --> ProfileQuery[Execute profile sessions query]
Sessions --> ProfileQuery
Visitors --> ProfileQuery
ProfileQuery --> Plugins[Apply result plugins]
Plugins --> ProfileRows[Session history]
Dashboard[Organization settings] --> RPC[Business-context RPC]
RPC --> Store[Organization metadata service]
RPC --> Queue[Insights queue]
Queue --> Worker[Business-context generator]
Worker --> PublicPages[Validated website pages]
Worker --> Store
Store --> ContextLoader[Investigation context loader]
ContextLoader --> Agent[Signal selection and investigation agent]
Reviews (1): Last reviewed commit: "fix(dashboard): match business context t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return [ | ||
| { | ||
| as: "targetAnonIds", | ||
| column: "anonymous_id", | ||
| params, | ||
| sql: `${identity} SELECT anonymous_id FROM target_anonymous_ids`, | ||
| }, | ||
| { | ||
| as: "targetSessionIds", | ||
| column: "session_id", | ||
| params, | ||
| sql: `${identity} SELECT session_id FROM target_session_ids`, | ||
| }, | ||
| { | ||
| as: "targetVisitorIds", | ||
| column: "anonymous_id", | ||
| params, | ||
| sql: `${identity} | ||
| SELECT anonymous_id FROM target_anonymous_ids | ||
| UNION DISTINCT | ||
| SELECT {visitorId:String} AS anonymous_id`, | ||
| }, |
There was a problem hiding this comment.
The three identity sets and the final profile query now run as four independent ClickHouse requests. If identity events are ingested between those requests, or cached prepare results have different ages, the final query can combine anonymous and session IDs from different data states. This can omit newly linked activity or return an inconsistent session history. Resolve the related sets from one staged query or otherwise preserve a consistent identity snapshot.
Knowledge Base Used:
| let origin: "team" | "website" = "team"; | ||
| if (generated?.draft?.content === brief.content) { | ||
| origin = | ||
| current.profile?.origin === "team" && current.profile.content | ||
| ? "team" | ||
| : "website"; |
There was a problem hiding this comment.
When an unchanged generated draft replaces a nonempty team-authored profile, this code keeps origin: "team" solely because of the previous profile. Regeneration is not required to retain the earlier team assertions, so an AI rewrite based on public sources can be labeled as team-supplied context. Investigation prompts then give those generated claims stronger provenance than they have.
Knowledge Base Used: Analytics and intelligence workflows
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const remaining = Math.max(1, 120_000 - (performance.now() - started)); | ||
| await bounded( | ||
| markBusinessContextGeneration({ | ||
| ...input, | ||
| status: "failed", | ||
| error: signal.aborted | ||
| ? "Generation took too long. Try again; your saved context is unchanged." | ||
| : failure, | ||
| }), | ||
| AbortSignal.timeout(Math.floor(remaining)) | ||
| ); |
There was a problem hiding this comment.
If this attempt to persist the failure state times out or encounters the service's four-second organization lock timeout, the new error escapes the catch. Because the job has only one attempt and no recovery sweep repairs these metadata-backed generations, the stored state remains running. The dashboard can therefore report generation in progress until the passive 180-second expiry is observed. Guard or retry this checkpoint so the original failure cannot strand the lifecycle.
Knowledge Base Used: Analytics and intelligence workflows
| allowedFilters: ["anonymous_id"], | ||
| requiredFilters: ["anonymous_id"], | ||
| prepareSql: profileIdentityPrepareStages, |
There was a problem hiding this comment.
The production profile_sessions path now depends on execute() resolving three prepare stages, but the profile tests call customSql directly and exercise only the inline-subquery fallback. Add an execution-level test for prepared parameter binding, empty identity sets, and staged-builder batch routing. Otherwise, regressions in the new path introduced by this PR can pass while production results fail.
Knowledge Base Used: Analytics query engine
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export async function generateOrganizationBusinessContext( | ||
| payload: unknown | ||
| ): Promise<void> { |
There was a problem hiding this comment.
This worker boundary introduces payload: unknown, while the repository requires explicit types instead of any, unknown, or never. Define a concrete parsed job-payload type here. The same new pattern appears in the organization metadata helpers and contextError, so those instances must also be corrected before merging.
Context Used: Basic guidelines for the project so vibe coders do... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Extends the
prepareSqlhook from #761 to the two remaining profile queries, and records where the technique stops paying.profile_sessions
It read 4,474,325 rows and 440 MiB to return 4 sessions.
target_identity_rowsran 58 times across 173 base-table scans, becausetarget_anonymous_ids,target_session_idsandvisitor_idswere each referenced as subqueries at ten sites, and ClickHouse inlines a CTE per reference.All three are tiny per-profile key sets — 2 to 3 values on the site measured. They now resolve concurrently in a prepare pass and the sites bind parameters. Bound directly, the query reads 623,095 rows and 50.7 MiB.
Three of those sites live inside
profileActivityCteitself, which is why this lands at 2.37x rather than the 2.22x from binding only the outer ones.profile_detail is deliberately not staged
It shares the same helper, so enabling it was a one-line change. Measured, it came out slower: it returns a single row from a comparatively cheap query, so three prepare round trips cost more than the repeated subqueries they remove.
That is the boundary on this technique — staging pays when the repeated subquery is expensive relative to a round trip, and loses when it is not.
profile_detailkeeps the subquery fallback and is unchanged.Verification
Output identical for both staged builders across three visitors and date ranges returning 28, 3, 3 and 1, 1, 1 rows, with plugins applied to both sides.
That last detail matters: an earlier comparison looked like a mismatch purely because
execute()enriches geo throughapplyPluginswhile rawchQuerydoes not, so the staged side had more data. Comparing raw SQL output againstexecute()is not a valid check for any builder with plugins.673 tests pass, typecheck and lint clean. Also carries #762 and #763, already reviewed.
Summary by cubic
Stages the identity key sets for the profile queries so
profile_sessionsresolves them once up front instead of re-running subqueries 58 times — reads drop from 4.47M rows / 440 MiB to 623K rows / 50.7 MiB, 2.37x faster;profile_detailkeeps the subquery fallback because staging measured slower there. This also carries the organization business context feature: an editable, AI-generated brief per organization that the insights agent consumes as anorganization_profilesource.New Features
Refactors
metadataon organization create/update.Written for commit 05737fa. Summary will update on new commits.