feat(cymbal): add the path resolution service - #107633
ablaszkiewicz wants to merge 1 commit 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✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| File | Comment lines | Added lines |
|---|---|---|
rust/cymbal/src/modes/path_resolution/list_store.rs |
13 | 322 |
rust/cymbal/src/modes/path_resolution/matching.rs |
8 | 240 |
rust/cymbal/src/modes/path_resolution/config.rs |
6 | 65 |
rust/cymbal/src/modes/path_resolution/file_index.rs |
6 | 104 |
rust/cymbal/src/core/repo_slug.rs |
3 | 52 |
rust/cymbal/src/modes/path_resolution/auth.rs |
3 | 69 |
rust/cymbal/src/modes/path_resolution/mod.rs |
3 | 140 |
rust/cymbal/src/core/metric_consts.rs |
1 | 7 |
This check does not block merging. It updates on every push and clears when the share drops.
✅ Bundle size — no change
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.94 MiB · no change
No file changed by more than 1000 B.
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.5% 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.56 MiB · 629 files | no change | █████████░ 88.4% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.38 MiB · 2,332 files | no change | █████████░ 88.5% 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.4 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.1 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.4 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.38 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.38 MiB · 19 files | no change | ████░░░░░░ 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 |
|---|---|
| 800.0 KiB | dist/toolbar/toolbar-app-6DYXIWRU.css |
| 651.4 KiB | dist/toolbar/chunk-chunk-TVTQKACM.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-6JFSEK3E.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-CP4QT72J.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-OHYAIWFE.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-577JVGVT.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-GNB7IYR7.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 — no change
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 946.22 MiB · no change
|
[Medium risk] Adds a new gRPC service for mapping file paths to repository paths. The PR should not merge until callers cannot cache unloaded-list results and concurrent cold loads have a resource bound. Reviews (1) · Last reviewed commit: "feat(cymbal): add the path resolution se..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| | `OUTCOME_TIE` | Several files match, and nothing decides. | empty | | ||
| | `OUTCOME_NO_MATCH` | No file matches. | empty | | ||
|
|
||
| `SURE` and `NO_MATCH` depend only on the path and the commit, so callers may cache them. |
There was a problem hiding this comment.
NO_MATCH for every path. This guidance lets callers cache those results, so they can keep returning “no path” after the list is written or storage recovers. Say that NO_MATCH is cacheable only when list_state is LIST_STATE_LOADED.
Prompt To Fix With AI
This is a comment left during a code review.
Path: rust/cymbal/src/modes/path_resolution/README.md
Line: 50
Comment:
**No-match results can become stale.** When the list is missing or object storage fails, the service returns `NO_MATCH` for every path. This guidance lets callers cache those results, so they can keep returning “no path” after the list is written or storage recovers. Say that `NO_MATCH` is cacheable only when `list_state` is `LIST_STATE_LOADED`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // it passes its deadline. Concurrent misses for one key share a single load. | ||
| let store = self.clone(); | ||
| let key = key.clone(); | ||
| let load = tokio::spawn(async move { |
There was a problem hiding this comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: rust/cymbal/src/modes/path_resolution/list_store.rs
Line: 120
Comment:
**Cold loads can exhaust memory.** Each distinct cold key starts a background load that continues after its request ends. These loads are outside the 2 GiB cache budget; with up to 256 concurrent requests and 64 MiB allowed per list, cold-key traffic can use many GiB more than the cache and exhaust a pod. Bound concurrent loads or their aggregate memory separately from request concurrency.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| repo: request.repo, | ||
| commit: request.commit, | ||
| }; | ||
| let (list_state, outcomes) = match self.store.get(&key).await { |
There was a problem hiding this comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: rust/cymbal/src/modes/path_resolution/service.rs
Line: 46
Comment:
**Empty requests load file lists.** A request with no paths passes validation but still fetches and indexes the entire list on a cold key. That spends an object-storage read and cache work without producing any path results. Return an empty response before loading the list.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (21)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe change adds the Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No confirmed issue blocks merging this service before its caller is enabled. Confirm file-list compatibility and deployment health checks before enabling the caller. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new service limits access with a dedicated secret and validates requests, but anyone holding that secret can select file lists across teams. Distinct requests can also continue loading lists after callers disconnect. Actual deployment access controls and callers are not established, so the practical exposure remains uncertain. 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 |
|
|
c9f1d87 to
d99a91f
Compare
Adds CYMBAL_MODE=path_resolution, a gRPC service that maps the frame paths of one exception to files of the release commit. It reads the commit's file list from object storage, keeps lists in a byte-weighed in-memory cache with single-flight background loads, and matches paths statelessly. Nothing calls it yet. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
d99a91f to
9eecb21
Compare
HostHog preview —
|
HostHog preview —
|
This branch has not been deployed
1 participant
Merge order
Problem
webpack://app/./src/index.tsx,/home/runner/work/shop/shop/app/models.py).Changes
CYMBAL_MODE=path_resolution, deployed ascymbal-path-resolution, serving gRPCcymbal.path_resolution.v1.ResolvePaths..and..segments:SURESUPPORTTIENO_MATCHx-cymbal-path-resolution-secret, with rotation. It is notINTERNAL_API_SECRET, so a leak reaches this service only.bin/start-rust-service cymbal-path-resolutionand an mprocs entry.rust/cymbal/src/modes/path_resolution/README.mddocuments the contract, the config and the deployment order.Note
The service needs a deployment (headless Kubernetes service, memory sized for the cache) before the caller is turned on. The caller treats an unreachable service as "no path", so the order is safe either way.
How did you test this code?
matching.rsholds the reference vectors: prefixes, relative segments, file-name-only matches, votes, and ties.list_store.rstests catch a missing list that reaches storage on every request, concurrent misses that load twice, a load that dies with its caller, and a list that decompresses past the limit.auth.rstests catch a retired secret that still works, and an empty secret list that lets requests through.service.rstests catch results out of request order and invalid requests that get an answer.repo_pathon the frames.👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release status
Nothing calls the service yet.
Automatic notifications
Docs update
None under
docs/. The mode README in the cymbal crate covers the service.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5, 1M context)/writing-tests,/writing-code-comments,/writing-pr-descriptions,/stacking-prs.🤖 Generated with Claude Code
https://claude.ai/code/session_01DpTwx89x8Wu9Nu9B82mPEj