Conversation
Generated-By: PostHog Desktop Task-Id: 399a122f-74a0-4fa2-9ae1-0f879f7c9ec3
|
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 |
MattPua
changed the title
🤖 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.
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
InsightMeta |
frontend/src/lib/components/Cards/InsightCard/InsightMeta.tsx:137 |
119 | 10 |
InsightMetaContent |
frontend/src/lib/components/Cards/InsightCard/InsightMeta.tsx:850 |
23 | 10 |
AnnotationModal |
products/annotations/frontend/components/AnnotationModal.tsx:31 |
21 | 10 |
AnnotationsBadgeRaw |
frontend/src/lib/components/AnnotationsOverlay/AnnotationsOverlay.tsx:242 |
13 | 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 — 🟢 -16.4 KiB (-0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.84 MiB · 🟢 -16.4 KiB (-0.0%)
| File | Size | Δ vs base |
|---|---|---|
render-query/src/render-query/render-query.js |
20.11 MiB | 🟢 -41.2 KiB (-0.2%) |
posthog-app/_parent/products/annotations/frontend/components/AnnotationModal.js |
11.3 KiB | 🔺 +11.3 KiB (new) |
exporter/_parent/products/annotations/frontend/components/AnnotationModal.js |
10.2 KiB | 🔺 +10.2 KiB (new) |
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 | 🔺 +6 B (+0.0%) | █████████░ 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.51 MiB · 629 files | 🔺 +6 B (+0.0%) | █████████░ 87.2% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.33 MiB · 2,333 files | 🔺 +1.1 KiB (+0.0%) | █████████░ 87.9% 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 |
| 216.0 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 |
| 216.0 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.16 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.16 MiB · 19 files | no change | ████░░░░░░ 37.7% 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.2 KiB | dist/toolbar/toolbar-app-W34WPP75.css |
| 651.5 KiB | dist/toolbar/chunk-chunk-CUVIGMRW.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-A3TIKFFR.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-C5DYFU35.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-XBL23GUL.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-H56HC6JT.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-HHUIDF5H.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 — 🟢 -133.7 KiB (-0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 945.73 MiB · 🟢 -133.7 KiB (-0.0%)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
@greptile-apps[bot] still has a review in flight (👀) — not approving over an unfinished review. The review re-runs on the next push, or re-request one once the reviewer finishes.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 623L, 12F substantive — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (623L, 12F, two-areas, feat) |
| stamphog 2.2.0 | .stamphog/policy.yml @ a002096 · reviewed head a002096 |
|
[Medium risk] Refactors annotation modal state management and rendering. The PR is not safe to merge until later-page deep links open and an already-open modal responds to a new annotation URL. Reviews (1) · Last reviewed commit: "fix(annotations): simplify annotation mo..." |
![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. 📝 WalkthroughWalkthroughThe change centralizes annotation modal state in Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to An annotation opened by a deep link can save successfully yet remain absent from the annotation list. Fix that update path before merging. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The shared annotation flow makes it important to keep an open request tied to the project and page where it started. A delayed URL lookup or a project change could leave the modal showing the wrong context. No unauthorized access was established. 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❌ Failed checks (1 warning)
Full details: Description checkExplanation The description covers the problem, user-visible changes, screenshot, release status, documentation status, and local validation limits. However, it states that no tests were added even though the diff adds tests, and it omits required agent-context details for patch coverage, new-events-schema evaluation, and the required before-and-after flow diagrams for the modal flow change. Resolution Update the testing section to name the added regression tests and state their execution status. Add the agent-context results for patch coverage and new-events-schema assessment, include the required session link and CodeRabbit disposition, and add before-and-after Mermaid flowcharts for the annotation modal request flow.
✨ 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 1c436b5f-9079-462a-a94d-286308e0f103
📒 Files selected for processing (12)
frontend/src/layout/scenes/SceneLayout.tsxfrontend/src/lib/components/AnnotationsOverlay/AnnotationsOverlay.scssfrontend/src/lib/components/AnnotationsOverlay/AnnotationsOverlay.tsxfrontend/src/lib/components/Cards/InsightCard/InsightMeta.tsxfrontend/src/scenes/insights/EditorFilters/AnnotationsPicker.tsxproducts/annotations/frontend/components/AnnotationModal.tsxproducts/annotations/frontend/emptyState/AnnotationsPrimaryAction.tsxproducts/annotations/frontend/logics/annotationModalHostLogic.tsproducts/annotations/frontend/logics/annotationModalLogic.tsproducts/annotations/frontend/logics/annotationScopes.tsproducts/annotations/frontend/logics/annotationsLogic.tsproducts/annotations/frontend/pages/Annotations.tsx
Included review availability: This review used your included allowance. 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 @MattPua — baseline updated in 4 changed. Install the Visual Review Chrome extension to see visual review results at the top of your pull requests. |
4 updated Run: fa4dff75-fa7e-4ba5-83a3-34c9efba8cc4 Co-authored-by: MattPua <3376526+MattPua@users.noreply.github.com>
Generated-By: PostHog Desktop Task-Id: 76c2870c-dbe6-4195-aaee-61c7ca6e4238
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The deep-link-to-later-page bug was fixed (annotationsLogic now fetches the annotation directly via api.annotations.get when not yet loaded), but the second P1 concern from @greptile-apps on annotationModalLogic.ts is still live in the current diff: annotationModalLogic's afterMount only reads the host's pending request once at mount, so if the modal stays open and the URL changes to a different annotation, the form keeps editing the stale annotation and Save could overwrite the wrong record. That comment is unresolved (not marked outdated/resolved) and the fix is not present in the diff.
- Author wrote 0% of the modified lines and has 94 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot].
- Unresolved @greptile-apps P1 comment on products/annotations/frontend/logics/annotationModalLogic.ts: while the annotation modal is open, navigating to a different annotation's deep link does not refresh the form (afterMount only runs once on mount), so Save can update the wrong annotation record.
- A related greptile-apps discussion comment explicitly states the PR is not safe to merge until this is fixed; only half of that concern (first-page deep links) has been addressed in the current diff.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 613L, 13F substantive, 659L/14F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (659L, 14F, two-areas, feat) |
| stamphog 2.2.0 | .stamphog/policy.yml @ e5674a6 · reviewed head e5674a6 |
|
This P1 is valid and remains unresolved. The modal logic needs to react to the host’s subsequent create/edit actions—while retaining mount-time handling for the initial request. Please address this and re-add the |
Generated-By: PostHog Desktop Task-Id: 76c2870c-dbe6-4195-aaee-61c7ca6e4238
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
Generated-By: PostHog Desktop Task-Id: 037601e8-bbc1-48ef-bad4-bbc8beae3865
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
An independent reviewer flagged a concrete, currently-unresolved bug: since the modal component and its host logic don't refire on a second open-request while already mounted, opening a different annotation while the modal is already open can leave you editing/saving over the wrong one — and the reviewer explicitly called the PR unsafe to merge until this and a related deep-link issue are fixed.
- Author wrote 0% of the modified lines and has 94 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot].
- Greptile flagged a real P1: annotationModalLogic's afterMount only reads annotationModalHostLogic's modalRequest once, so a subsequent openModalToEditAnnotation while the modal stays mounted doesn't reset the form — a save can silently overwrite the wrong annotation.
- The same review states annotation deep links to a later results page still don't open the target annotation, contradicting the PR description's claim that this was fixed.
- Neither concern has a follow-up commit or reply from the author addressing them.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 630L, 13F substantive, 722L/15F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (722L, 15F, two-areas, feat) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 7d15b88 · reviewed head 7d15b88 |
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
products/annotations/frontend/logics/annotationsLogic.ts-135-135 (1)
135-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject malformed annotation IDs before opening the modal.
kea-routerusesurl-pattern, whose named segments accept letters and digits. Therefore,/data-management/annotations/42extramatches the route, andparseInt('42extra')produces42. The listener can then open annotation 42 for editing. Validate the complete positive integer before dispatchingopenAnnotationFromUrl.🐛 Suggested fix
- actions.openAnnotationFromUrl(parseInt(id as string)) + const annotationId = id as string + const parsedAnnotationId = Number(annotationId) + if ( + /^\d+$/.test(annotationId) && + Number.isSafeInteger(parsedAnnotationId) && + parsedAnnotationId > 0 + ) { + actions.openAnnotationFromUrl(parsedAnnotationId) + }products/annotations/frontend/logics/annotationsLogic.ts-96-99 (1)
96-99: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle failed annotation deep-link fetches.
When the ID is absent from the loaded list,
openAnnotationFromUrlawaitsapi.annotations.get. A deleted annotation or request failure rejects before the modal opens. Kea rethrows this listener rejection, so the deep link remains without an annotation modal and without the error handling used by comparable deep-link loaders.Suggested fix
-import { LemonSelectOption, LemonSelectOptions } from '@posthog/lemon-ui' +import { LemonSelectOption, LemonSelectOptions, lemonToast } from '@posthog/lemon-ui' @@ - const rawAnnotation = await api.annotations.get(annotationId) - annotationModalHostLogic.actions.openModalToEditAnnotation( - deserializeAnnotation(rawAnnotation, values.timezone) - ) + try { + const rawAnnotation = await api.annotations.get(annotationId) + annotationModalHostLogic.actions.openModalToEditAnnotation( + deserializeAnnotation(rawAnnotation, values.timezone) + ) + } catch { + lemonToast.error('Failed to load annotation') + }
🧹 Nitpick comments (1)
products/annotations/frontend/logics/annotationsLogic.test.ts (1)
38-38: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the already-loaded URL branch.
openAnnotationFromUrlskipsapi.annotations.getwhen the annotation is already loaded. The current test covers only the fetch branch. Add a case that supplies an already-loaded annotation, opens it by URL, and asserts thatapi.annotations.getis not called.A rejected-fetch case is not justified by this code alone because no failure behavior or error contract is defined. The generic happy-path guidance does not require that additional case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: e34b9e66-ee4f-4186-95b2-8b201316b0f8
📒 Files selected for processing (5)
frontend/snapshots.ymlproducts/annotations/frontend/logics/annotationModalLogic.test.tsproducts/annotations/frontend/logics/annotationModalLogic.tsproducts/annotations/frontend/logics/annotationsLogic.test.tsproducts/annotations/frontend/logics/annotationsLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
Generated-By: PostHog Desktop Task-Id: ad67af3f-aa48-4b6a-a031-dfb6772bf0e7
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)
products/annotations/frontend/logics/annotationsLogic.ts-106-106 (1)
106-106: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winValidate
AnnotationApibefore deserialization.
annotationsRetrievereturnsAnnotationApi, but the cast bypasses theRawAnnotationTypecontract.AnnotationApipermits missingdate_marker,content, andscope, and permitscreated_at: null.deserializeAnnotationpassescreated_attodayjsUtcToTimezone, which requires a non-null string. Use a typed adapter or validation step before deserialization. Update the fixture to use the generated response shape without the same cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 980f19a0-f10d-4413-b676-9bf092a1becf
📒 Files selected for processing (4)
products/annotations/frontend/components/AnnotationModal.tsxproducts/annotations/frontend/logics/annotationModalLogic.tsproducts/annotations/frontend/logics/annotationsLogic.test.tsproducts/annotations/frontend/logics/annotationsLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
Generated-By: PostHog Desktop Task-Id: ad67af3f-aa48-4b6a-a031-dfb6772bf0e7
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: 95456401-b5bf-498f-b2da-5d53c7dccdc1
📒 Files selected for processing (1)
products/annotations/frontend/logics/annotationModalLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
| : values.existingModalAnnotation.dashboard_id, | ||
| } | ||
| ) | ||
| annotationsModel.actions.replaceAnnotation(updatedAnnotation as RawAnnotationType) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '90,115p' products/annotations/frontend/logics/annotationsLogic.ts
sed -n '238,270p' products/annotations/frontend/logics/annotationModalLogic.ts
sed -n '155,260p' frontend/src/models/annotationsModel.tsRepository: PostHog/posthog
Length of output: 7652
🏁 Script executed:
printf '%s\n' '--- annotationsLogic ---'
cat -n products/annotations/frontend/logics/annotationsLogic.ts | sed -n '1,145p'
printf '%s\n' '--- annotationModalHostLogic ---'
rg -n -A35 -B15 'openModalToEditAnnotation|existingModalAnnotation|openHostModalToEditAnnotation' products/annotations/frontend/logics products/annotations/frontend/components
printf '%s\n' '--- annotationModalLogic update/open path ---'
cat -n products/annotations/frontend/logics/annotationModalLogic.ts | sed -n '1,310p'
printf '%s\n' '--- annotationsModel callers ---'
rg -n -A12 -B8 'replaceAnnotation|appendAnnotations|loadAnnotations' products/annotations frontend/src/models/annotationsModel.tsRepository: PostHog/posthog
Length of output: 42088
🏁 Script executed:
rg -n -A20 -B12 'openAnnotationFromUrl|annotationsRetrieve|openModalToEditAnnotation' products/annotations/frontend/logics/annotationsLogic.ts products/annotations/frontend/logics/annotationModalHostLogic.ts products/annotations/frontend/logics/annotationModalLogic.ts
rg -n -A18 -B8 'rawAnnotations:|replaceAnnotation:|loadAnnotations:' frontend/src/models/annotationsModel.ts
rg -n -A12 -B12 'annotations\.list|annotationsRetrieve|hidden_in_user_interface' frontend products/annotations | head -240Repository: PostHog/posthog
Length of output: 41886
Upsert visible URL-fetched annotations that are absent from the model.
When a visible annotation is fetched from the URL before annotationsModel contains it, the modal can submit an update successfully. replaceAnnotation then finds -1 and writes a non-index property, so the annotation is absent from the derived list. The list load is independent and does not guarantee that this annotation is already present.
Do not append hidden annotations because the model intentionally excludes them.
Suggested fix
const copy = state.slice()
const index = copy.findIndex((iterationAnnotation) => iterationAnnotation.id === annotation.id)
+ if (index === -1) {
+ return annotation.hidden_in_user_interface === true ? copy : [...copy, annotation]
+ }
copy[index] = annotation
return copy
|
👋 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. Install the Visual Review Chrome extension to see visual review results at the top of your pull requests. |
This branch was successfully deployed
1 participant
Problem
Changes
How did you test this code?
git diff --checkpassed.👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release status
Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Codex CLI, GPT-5
/qa-team,/writing-kea-logics,/writing-ui-components,/writing-tests,/writing-pr-descriptions, and/reviewing-with-coderabbit.crCLI is unavailable.Created with PostHog Desktop