feat(navigation): rank cmd+k commands and files with jev - #105916
mariusandra wants to merge 2 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
<anonymous> |
frontend/src/lib/components/Search/searchLogic.tsx:1883 |
35 | 10 |
<anonymous> |
frontend/src/lib/components/Search/searchLogic.tsx:1743 |
28 | 10 |
<anonymous> |
frontend/src/lib/components/Search/searchLogic.tsx:1641 |
20 | 10 |
<anonymous> |
frontend/src/lib/components/Search/Search.tsx:432 |
19 | 10 |
<anonymous> |
frontend/src/lib/components/Search/Search.tsx:558 |
19 | 10 |
<anonymous> |
frontend/src/lib/components/Search/Search.tsx:514 |
14 | 10 |
<anonymous> |
frontend/src/lib/components/Search/Search.tsx:946 |
14 | 10 |
loadRankedSearch |
frontend/src/lib/components/Search/searchLogic.tsx:681 |
14 | 10 |
<anonymous> |
frontend/src/lib/components/Search/Search.tsx:309 |
11 | 10 |
<anonymous> |
frontend/src/lib/components/Search/Search.tsx:713 |
11 | 10 |
<anonymous> |
frontend/src/lib/components/Search/searchLogic.tsx:1270 |
11 | 10 |
✅ Duplication (Python) — clean
New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Bundle size — 🟢 -5.1 KiB (-0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.90 MiB · 🟢 -5.1 KiB (-0.0%)
| File | Size | Δ vs base |
|---|---|---|
posthog-app/src/scenes/AuthenticatedShell.js |
241.2 KiB | 🟢 -5.1 KiB (-2.1%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.57 MiB · 22 files | no change | █████████░ 85.3% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.58 MiB · 629 files | 🔺 +243 B (+0.0%) | █████████░ 88.9% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.39 MiB · 2,330 files | 🔺 +3.3 KiB (+0.0%) | █████████░ 88.6% of 8.34 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 88.0 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 63.9 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.4 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 98.5 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 88.0 KiB | src/products.tsx |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.37 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.37 MiB · 19 files | 🔺 +40 B (+0.0%) | ████░░░░░░ 41.5% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 797.0 KiB | dist/toolbar/toolbar-app-3TRIGGPQ.css |
| 650.9 KiB | dist/toolbar/chunk-chunk-HJ6ZNDPJ.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-6JFSEK3E.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-WXZTGK7F.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-6XMQXG7P.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-RSKN4BK4.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-QKJ4TX5Z.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +54.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 945.68 MiB · 🔺 +54.4 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17630.1 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 196.2 KB |
| action | 454.1 KB | 196.2 KB |
| action-list | 564.2 KB | 196.2 KB |
| cohort | 453.1 KB | 196.2 KB |
| cohort-list | 563.2 KB | 196.2 KB |
| email-template | 452.9 KB | 196.2 KB |
| error-details | 469.1 KB | 196.2 KB |
| error-issue | 453.8 KB | 196.2 KB |
| error-issue-list | 564.1 KB | 196.2 KB |
| experiment | 561.3 KB | 196.2 KB |
| experiment-list | 564.9 KB | 196.2 KB |
| experiment-results | 566.3 KB | 196.2 KB |
| feature-flag | 566.8 KB | 196.2 KB |
| feature-flag-list | 570.5 KB | 196.2 KB |
| feature-flag-testing | 457.3 KB | 196.2 KB |
| inline-scan | 453.6 KB | 196.2 KB |
| insight-actors | 562.3 KB | 196.2 KB |
| invite-email-preview | 452.3 KB | 196.2 KB |
| llm-costs | 559.3 KB | 196.2 KB |
| session-recording | 455.3 KB | 196.2 KB |
| survey | 454.7 KB | 196.2 KB |
| survey-global-stats | 561.9 KB | 196.2 KB |
| survey-list | 564.9 KB | 196.2 KB |
| survey-stats | 561.9 KB | 196.2 KB |
| trace-span | 453.5 KB | 196.2 KB |
| trace-span-list | 564.1 KB | 196.2 KB |
| vision-observation-list | 563.3 KB | 196.2 KB |
| workflow | 453.4 KB | 196.2 KB |
| workflow-list | 563.5 KB | 196.2 KB |
| loops-review | 457.8 KB | 196.2 KB |
| query-results | 774.1 KB | 196.2 KB |
| render-ui | 857.0 KB | 196.2 KB |
| visual-review-snapshots | 457.9 KB | 196.2 KB |
⚠️ deltalite — version bump needed
It looks like the code of deltalite changed in this PR, but its version stayed the same at 0.1.8. 👀
Bump the version in rust/deltalite/python/Cargo.toml AND rust/deltalite/python/pyproject.toml (the two must match) before merging.
|
The PR is not ready to merge because flagged searches lose existing results and can remain loading before a team is available. Reviews (1) · Last reviewed commit: "feat(navigation): rank cmd+k commands an..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds feature-gated command and file ranking to the search palette. Django validates requests, selects accessible file candidates, and ranks candidates through the configured gateway, with caching and fallback paths. The frontend sends command metadata, displays ranked results, and returns to existing search when ranking is unavailable or produces no matches. The change also adds API and MCP contracts, tests, a Storybook story, tooltip catalogs, and setup documentation. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to A slow gateway response can outlive the ranking lock and permit duplicate inference. Resolve the outstanding search and fallback risks before merging. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Ranking is gated and sends a limited set of accessible file names and paths, not the full file inventory. It nevertheless introduces a new metadata transfer to a gateway for a broader set of users; the gateway’s data-handling and regional guarantees need confirmation. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
posthog/api/file_system/test/test_file_system.py-2585-2600 (1)
2585-2600: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe flag-off case passes without reaching the flag check.
In
(True, True, False), the user is staff and the team is allowlisted.CommandSearch.enabledthen checks the deployment before the flag. Tests run withDEBUG=False, andCLOUD_DEPLOYMENTis not"US".enabledreturns False before it callsposthoganalytics.feature_enabled. The 403 is caused by the region check, so the flag gate is never tested. No test shows that the gate opens when every condition is met, so a broken flag check would not fail CI.Set
CLOUD_DEPLOYMENT="US"in the override. Add a positive case that expects a non-403 response.Proposed fix
- `@parameterized.expand`([(False, True, True), (True, False, True), (True, True, False)]) - def test_experiment_gate_sends_nothing(self, staff: bool, allowlisted: bool, flag: bool) -> None: + `@parameterized.expand`( + [(False, True, True, 403), (True, False, True, 403), (True, True, False, 403), (True, True, True, 200)] + ) + def test_experiment_gate(self, staff: bool, allowlisted: bool, flag: bool, expected: int) -> None: self.user.is_staff = staff self.user.save() with ( - self.settings(COMMAND_SEARCH_JEV_TEAM_IDS=[str(self.team.pk)] if allowlisted else []), + self.settings( + COMMAND_SEARCH_JEV_TEAM_IDS=[str(self.team.pk)] if allowlisted else [], CLOUD_DEPLOYMENT="US" + ), patch("posthog.helpers.command_search.posthoganalytics.feature_enabled", return_value=flag), patch("posthog.helpers.command_search.system_one") as infer, ): ... - self.assertEqual(response.status_code, 403) - infer.assert_not_called() + self.assertEqual(response.status_code, expected) + if expected == 403: + infer.assert_not_called()
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 5c47c627-3465-41f6-8a94-57955f68896b
⛔ Files ignored due to path filters (3)
frontend/src/generated/core/api.schemas.tsis excluded by!**/generated/**frontend/src/generated/core/api.tsis excluded by!**/generated/**frontend/src/generated/core/api.zod.tsis excluded by!**/generated/**
📒 Files selected for processing (19)
docs/published/handbook/engineering/developing-locally.mdfrontend/src/layout/panel-layout/navbar/tabs/NavAppTooltip.tsxfrontend/src/lib/components/Search/Search.stories.tsxfrontend/src/lib/components/Search/Search.tsxfrontend/src/lib/components/Search/commandDescriptions.tsfrontend/src/lib/components/Search/searchLogic.test.tsfrontend/src/lib/components/Search/searchLogic.tsxfrontend/src/lib/components/Search/utils.tsfrontend/src/lib/constants.tsxposthog/api/file_system/command_search.pyposthog/api/file_system/file_system.pyposthog/api/file_system/test/test_file_system.pyposthog/egress/typesafe/README.mdposthog/egress/typesafe/client.pyposthog/helpers/command_search.pyposthog/helpers/tests/test_command_search.pyposthog/settings/web.pyservices/mcp/definitions/core.yamlservices/mcp/src/api/generated.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
🕓 This approval covered an earlier revision. There are new visual changes to review in the newer comment below. ✅ Visual changes approved by @mariusandra — baseline updated in 2 new. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (4)
posthog/helpers/command_search.py-166-166 (1)
166-166: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winIf a path contains only slashes, the endpoint crashes.
validate_file_system_pathaccepts"/".split_path("/")returns[], so[-1]raisesIndexError. One such row among the recent or owned files makes every command search for that team return 500.Fix
- "name": split_path(file.path)[-1][:200] if file.path else file.type, + "name": (split_path(file.path) or [file.type])[-1][:200],frontend/src/lib/components/Search/Search.tsx-452-456 (1)
452-456: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe theme shortcut is now hidden during every search, including when ranked search is off.
The
!isSearchingguard also applies to the existing, non-ranked path. On that path,isSearchingstays true until all loaders finish, and the person search can run for a long time. As a result, users with the flag off no longer see "Dark mode" or "Light mode" right away. Limit the guard to ranked mode.Fix
if ( - !isSearching && + !(useRankedSearch && isSearching) && normalizedQuery &&frontend/src/layout/panel-layout/navbar/tabs/NavAppTooltip.tsx-9-10 (1)
9-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe command catalog now overrides the group description.
sidebarToolMetachecks groups first, because "group type names … can match a built-in app name". The new lookupcommandDescriptions[item.path] ?? …runs before that check. A group type named like a built-in app (for examplePersons) now shows the app's description and example. Check groups first.Fix
- const description = commandDescriptions[item.path] ?? sidebarToolMeta(item).description - const example = commandExamples[item.path] const isGroup = item.iconType === 'group' || item.iconType?.startsWith('group_') || item.type?.startsWith('group_') + const description = (isGroup ? undefined : commandDescriptions[item.path]) ?? sidebarToolMeta(item).description + const example = isGroup ? undefined : commandExamples[item.path]docs/published/handbook/engineering/developing-locally.md-208-208 (1)
208-208: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the in-flight duplicate fallback.
A duplicate request that finds the lock held returns text matches; it does not wait for or reuse the ranked response.
posthog/helpers/command_search.pyuses a nonblocking lease and returns the fallback when acquisition fails. Replace “coalesced” with this behavior. (raw.githubusercontent.com)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 8e2f8968-c77b-48d9-9f86-e2afbae74240
⛔ Files ignored due to path filters (3)
frontend/src/generated/core/api.schemas.tsis excluded by!**/generated/**frontend/src/generated/core/api.tsis excluded by!**/generated/**frontend/src/generated/core/api.zod.tsis excluded by!**/generated/**
📒 Files selected for processing (11)
docs/published/handbook/engineering/developing-locally.mdfrontend/snapshots.ymlfrontend/src/layout/panel-layout/navbar/tabs/NavAppTooltip.tsxfrontend/src/lib/components/Search/Search.tsxfrontend/src/lib/components/Search/searchLogic.test.tsfrontend/src/lib/components/Search/searchLogic.tsxposthog/api/file_system/file_system.pyposthog/api/file_system/test/test_file_system.pyposthog/helpers/command_search.pyposthog/helpers/tests/test_command_search.pyservices/mcp/src/api/generated.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 57e2682c-0423-4a20-97db-bec77872da69
⛔ Files ignored due to path filters (3)
frontend/src/generated/core/api.schemas.tsis excluded by!**/generated/**frontend/src/generated/core/api.tsis excluded by!**/generated/**frontend/src/generated/core/api.zod.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
docs/published/handbook/engineering/developing-locally.mdposthog/helpers/command_search.pyposthog/helpers/tests/test_command_search.pyservices/mcp/definitions/core.yamlservices/mcp/src/api/generated.ts
Files not reviewed due to moderation or processing errors (2)
- posthog/helpers/command_search.py
- posthog/helpers/tests/test_command_search.py
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.
|
Addressed the review feedback in 8a5407b:
Validation: 42 backend and 68 frontend tests passed locally, along with frontend TypeScript, repository-wide mypy, formatting, and scoped security checks. Automated Chromium checks covered ranked results and the legacy theme shortcut during a pending person search at wide and narrow widths. Two synthetic live gateway requests passed with the configured timeouts.
The seven outstanding inline threads have replies explaining the fix or intentional scope and are resolved. Current-environment scope and the commands/files fast path remain as documented; existing search is retained on fallback. Complexity warnings in the broader existing search code remain non-blocking advisories. CI is running on this revision. |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
posthog/helpers/command_search.py-62-62 (1)
62-62: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAlign the AI gateway deadline with the two-second lock.
The supplied HTTPX client has no total request deadline.
GatewaySystemOneClient.decideuses that client directly, so a response that sends chunks less than 0.8 seconds apart can keep_transport.scoresblocked beyond the Redis lock lease. Another request can then acquire the lock and start inference while the first worker remains blocked. Ensure the configured AI gateway or proxy enforces a total deadline shorter than two seconds, or add an equivalent deadline around this call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 0ce6d54f-cd2d-40d1-8605-0374d7155a3e
📒 Files selected for processing (10)
docs/published/handbook/engineering/developing-locally.mdfrontend/src/layout/panel-layout/navbar/tabs/NavAppTooltip.test.tsxfrontend/src/layout/panel-layout/navbar/tabs/NavAppTooltip.tsxfrontend/src/layout/panel-layout/sidebarToolMeta.tsfrontend/src/lib/components/Search/Search.tsxposthog/api/file_system/test/test_file_system.pyposthog/helpers/command_search.pyposthog/helpers/tests/test_command_search.pyposthog/llm/system_one_client.pyposthog/llm/test_system_one_client.py
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.
|
The total-deadline finding in this review is fixed in 3419e8278f8. Cmd+k now uses the shared ml_inference facade with an 800 ms total network deadline. Cancellation closes an incomplete response, including a continuously streaming body, before releasing the two-second duplicate-request lock. Slow budget checks reduce the remaining deadline or skip inference; budget-cache failures also release the lock. Regression tests cover stalled headers, continuous response chunks, failed budget checks, and expired leases. Focused backend/frontend tests, repo-wide type checks, browser checks, security rules, and strict preflight passed locally. Fresh CI is running for the rebased commit; all existing inline threads remain resolved. |
8a5407b to
3419e82
Compare
|
React Doctor found 8 issues in 5 files · 8 warnings. 8 warnings
Reviewed by React Doctor for commit |
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
This branch has not been deployed
1 participant
Problem
Cmd+k users need commands and saved files ranked together by intent, without results shifting as separate searches finish.
This moves the Jev experiment from the Apps list in #105577 into the command palette.
Changes
ml_inferencefacade and its default Jevk5 model.command-search-jevflag controls rollout; HTTPS and gateway configuration remain required outside loopback.Older files outside the recent/created pools require a name/path match to enter ranking.
This bounds retrieval cost, but does not provide semantic recall across every historical file.
Semantic matches outside the text shortlist can also be missed.
The shortlist respects JevK5's single-pass option limit with a bounded total network deadline.
Production latency and ranking quality need evaluation before broader rollout.
Successful rankings cover commands and files; people, groups, accounts, and tickets remain available through the existing search fallback.
File candidates remain scoped to the current environment.
Cmd+k and the Products sidebar share command descriptions and examples.
Group tooltips preserve their descriptions when their names match built-in products.
Generated API files are mechanical.
Before:
flowchart LR A[Cmd+k query] --> B[Local command filtering] A --> C[Independent search requests] B --> D[Results update separately] C --> D classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; class A,D phYellow; class B phBlue; class C phRed;After:
flowchart LR A[Cmd+k query] --> B[Django retrieves bounded candidates] B --> F[Django shortlists 15 candidates] F --> C[Cached ranking or ml_inference facade] C --> D[Complete results or text fallback] C --> E[Existing search on denial or no matches] E --> D classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; class A,D phYellow; class B,E,F phBlue; class C phRed;Before and after, using Storybook fixtures
Before:
After:
Narrow:
Restored theme shortcut with fixed, synthetic ranking results:
Legacy search keeps the theme shortcut available while a person search is pending:
Rebased Products navigation, with the theme shortcut and synthetic ranking results:
How did you test this code?
Backend tests cover bounded retrieval, tenant isolation, rollout gates, cache isolation, and malformed gateway responses.
New deadline tests cancel stalled headers and a continuously streaming body, verifying cleanup.
Budget-cache regressions verify lock release and prevent inference after a delayed check.
Frontend tests cover atomic results, superseded requests, denied requests, empty rankings, delayed team loading, and settings metadata.
Automated Chromium checks verify ranked results and the theme shortcut at wide and narrow widths, including legacy search with a pending person request.
Live requests use invented candidates; one returned the expected ranking, while another stopped at the configured deadline.
OpenAPI regeneration, frontend type checks, and repo-wide Python type checks ran against the rebased branch.
Production performance and customer data were not tested.
Patch coverage awaits CI.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release status
Automatic notifications
Docs update
Updated the existing local development guide with inference availability, gateway setup, total deadlines, request limits, and fallback behavior.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Codex, GPT-6
Tools: shell, GitHub CLI, Playwright. No shareable session link is available.
The branch now targets master after #106735 merged, adopts the
ml_inferencefacade, and carries the Products navigation renames.Ranking does not opt into the external TypeSafe fallback.
The outstanding total-deadline review finding is addressed by cancellation in the shared inference facade.
The open-PR search found no overlapping cmd+k Jev implementation.
Fixtures are synthetic; screenshots contain repository fixtures or invented results.
CodeRabbit CLI was signed out, so this PR has no local CodeRabbit pass.
Skills used
hogli,writing-kea-logics,writing-ui-components,improving-drf-endpoints,routing-outbound-api-calls,adopting-generated-api-types,writing-tests,writing-code-comments,writing-user-facing-copy,using-kea-disposables,setting-feature-flags-in-storybook,writing-pr-descriptions,running-ci-preflight,reviewing-with-coderabbit,debugging-ci-failures,migrating-llm-gateway-callers,django-startup-time,stacking-prs,writing-dataclasses,run-posthog.