Skip to content

feat(flags): add a learn more link to the flag called SQL warning - #114264

Merged
trunk-io[bot] merged 18 commits into
masterfrom
haacked/flag-called-sql-warning-link
Oct 9, 2026
Merged

trunk-io[bot] merged 18 commits into
masterfrom
haacked/flag-called-sql-warning-link

Conversation

@haacked

@haacked haacked commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Part of #88126.

Changes

  • The warning's hover ends with a "Learn more" link. A click opens the announcement in a new tab.

flag-called-warning-learn-more

  • The flag-called-move-notices flag's payload supplies the URL as {"url": "https://…"}.
  • A payload of {"url": null} shows the warning with no link. The flag has that payload today.
  • Only an https URL with a host becomes a link. The backend strips surrounding whitespace from it.
  • Any other value, or a payload that is not a JSON object, shows the warning without a link.
  • The backend logs a bad payload once per metadata request, and only for a query that gets the warning.

Warning

The https check is a security boundary. Monaco runs a command: link as an editor command, and it assigns any other non-http link to window.location.
The check runs twice. The backend checks the flag payload. The editor checks again before it builds a link, because it renders the url of any notice.

The part that needs a review from the SQL editor's owners:

  • HogQLNotice gets an optional url field. Notices from every other check leave it empty.
  • The editor builds the link in noticeLink, which puts the URL in the Monaco marker's code field. Monaco renders the marker message as plain text, and code is the only marker field it renders as a link.
  • A URL that Monaco's Uri.parse rejects drops only its link. The query's other markers still render.
  • HogQLNotice has never carried a link. feat(sql-editor): pref heuristics #51316 added a second editor action by prefixing fix with ai_prompt: instead of adding a field.
  • A field is the only option that lets the notice carry its own link without overloading another field:
Alternative Why it doesn't work
The frontend matches the warning text and adds the link The frontend then depends on copy written in Python
The URL goes in the message text Monaco renders the message as plain text, so the URL is not clickable
A url: prefix on fix, like ai_prompt: The editor offers fix as "Replace with" text, and API and MCP clients read it the same way

How did you test this code?

Test rationale: test_metadata_warns_for_flag_called_read_from_events gains parameterized cases for the link. Each catches a distinct regression:

  • An https URL: the warning carries it.
  • A command: URL: the warning has no link. This fails if the scheme check lets another scheme through.
  • https://[broken: the warning still appears, with no link. urlparse raises on this URL, and before the fix the advisory handler dropped every warning.
  • An https URL padded with spaces: the warning carries the trimmed URL. urlparse keeps trailing spaces, so API and MCP clients would get them.
  • The flag at 0% rollout, and the flag missing: no warning.
  • A separate test runs SELECT 1 with an http:// payload URL and expects no log line. This fails if the backend checks the URL before it finds a warning.
  • Nine other tests compare whole notice dicts and now expect "url": None.

noticeLink.test.ts covers the editor side, which no test ran before:

  • An https URL becomes a link with its whitespace trimmed.
  • command:, javascript:, http: and missing URLs add no link. This fails if the editor's https check goes away.
  • A URL that Monaco rejects adds no link and does not throw.

Checked by hand in a local SQL editor with a test https URL in the payload: the hover showed "(Learn more)", and a click opened the URL in a new tab. The screenshot comes from that check.

Not checked by hand: the editor changes in noticeLink, and the malformed and padded URL cases. The tests above cover them.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

  • This change is behind a feature flag and is not available to users

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Opus 5.5 (claude-opus-5-5)

  • Skills: /stacking-prs, /reviewing-with-coderabbit, /writing-tests, /writing-pr-descriptions, /create-pr, /simplify, /comment-cleanup, /review-code, /writing-code-comments, /writing-ui-components, /address-pr-reviews.
  • The first commit reverts 36b59a57, the feat(flags): warn when SQL reads $feature_flag_called from events #114059 commit that removed the link so this PR could carry it. An earlier session wrote the link code and did the hand check for feat(flags): warn when SQL reads $feature_flag_called from events #114059.
  • CodeRabbit CLI ran once with --deep against haacked/flag-called-sql-warning. It reported one finding, fixed in the second commit with a test case: a malformed payload URL raised ValueError and dropped every warning, and https://@/ passed the host check.
  • /simplify moved the https check into the editor as well as the backend. The backend now returns the URL that urlparse checked.
  • /review-code ran eight reviewers and found nothing blocking. Their suggestions are fixed in f5969f34: strip the payload URL, log a payload that is not an object, and move the editor check into noticeLink with a Jest test.
  • ReviewHog flagged that a URL with a leading space passed the backend check and made Uri.parse throw. The backend returns the parsed URL, and noticeLink trims the URL and catches a Uri.parse error.
  • A review comment flagged that a bad payload logged on every metadata request, even for SELECT 1. The backend now checks the URL only after it finds a warning. Claude Code on Sonnet 5.5 (claude-sonnet-5-5) wrote that fix in adbd3bf5.
  • No duplicate PR: this is the stacked follow-up that feat(flags): warn when SQL reads $feature_flag_called from events #114059 names.
  • The test URLs use example.com.

https://claude.ai/code/session_017af6ERr59kp63wJFHzd4cX

haacked and others added 12 commits October 7, 2026 16:05
HogQL metadata adds a warning on each $feature_flag_called literal that a
query compares to the events table's event column. The warning names
posthog.flag_evaluations. Queries on posthog.flag_evaluations, and literals
that are not compared to the event column, get no warning.

Claude-Session: https://claude.ai/code/session_012rGhDjtKCS47JbyFu5gMQz
… warning

- posthog/hogql/flag_called_warnings.py: the finder skips hidden aliases, so a saved expression body no longer yields a warning at offsets from its own text
- posthog/hogql/flag_called_warnings.py: the finder stops at each field instead of following its type into shared CTE types

Claude-Session: https://claude.ai/code/session_0182H46NbivitrX4jyFFg61N
…yload

- posthog/hogql/metadata.py: reads the https URL in the flag-called-move-notices payload and evaluates the flag inside the advisory try block
- posthog/hogql/flag_called_warnings.py: puts that URL on each warning
- frontend/src/queries/schema/schema-general.ts: HogQLNotice has an optional url
- frontend/src/lib/monaco/codeEditorLogic.tsx: a notice with a url shows a "Learn more" link in its hover

Claude-Session: https://claude.ai/code/session_0182H46NbivitrX4jyFFg61N
2 updated
Run: 4b37d9d6-60d5-433e-99aa-126993b62a18

Co-authored-by: haacked <19977+haacked@users.noreply.github.com>
2 updated
Run: 1fb99b25-788e-4b23-8156-30f65fdaf7ff

Co-authored-by: haacked <19977+haacked@users.noreply.github.com>
The warning keeps the flag-called-move-notices gate and evaluates the flag
inside the advisory try block. A stacked follow-up adds the link.

Claude-Session: https://claude.ai/code/session_0182H46NbivitrX4jyFFg61N
The flag-called-move-notices payload holds the announcement URL. HogQL
metadata puts it on each warning as an optional HogQLNotice url, and only
an https URL with a host passes. The editor shows it as a "Learn more" link
in the warning's hover.

Claude-Session: https://claude.ai/code/session_017af6ERr59kp63wJFHzd4cX
… malformed

urlparse raises ValueError on a URL such as https://[broken, and the
advisory handler then dropped every warning. A malformed URL now gives a
warning with no link. The host check reads hostname, which rejects
https://@/.

Claude-Session: https://claude.ai/code/session_017af6ERr59kp63wJFHzd4cX
@haacked haacked self-assigned this Oct 9, 2026
@haacked
haacked added this pull request to stack #114265 October 9, 2026 00:16
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Complexity (TypeScript) — 1 function above the limit (max 11)

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

Function Location Complexity Limit
<anonymous> frontend/src/lib/monaco/codeEditorLogic.tsx:297 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.

🚨 Comment density — 7% of added code lines are comments (13 of 184)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
frontend/src/lib/monaco/noticeLink.ts 12 29
frontend/src/queries/schema/schema-general.ts 1 2

This check does not block merging. It updates on every push and clears when the share drops.

⚠️ Bundle size — 🔺 +412 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 75.50 MiB · 🔺 +412 B (+0.0%)

File Size Δ vs base
posthog-app/_parent/products/workflows/frontend/Broadcasts/BroadcastScene.js 92.8 KiB 🟢 -1.1 KiB (-1.2%)
render-query/src/render-query/render-query.js 24.15 MiB 🔺 +1.1 KiB (+0.0%)

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.67 MiB · 23 files no change █████████░ 90.6% 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.82 MiB · 675 files no change █████████░ 94.7% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.77 MiB · 2,460 files 🔺 +3 B (+0.0%) █████████░ 93.2% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.92 MiB · 3,514 files 🔺 +168 B (+0.0%) █████████░ 90.8% of 10.92 MiB
today home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
7.79 MiB · 2,470 files 🔺 +3 B (+0.0%) █████████░ 90.8% of 8.58 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.52 MiB · 3,358 files 🔺 +168 B (+0.0%) █████████░ 90.6% of 10.51 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.32 MiB · 4,189 files 🔺 +3 B (+0.0%) ████████░░ 78.4% of 15.72 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ 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
🟢 products/dashboards/frontend/widgets/previews/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 zod/v4/locales/de.js stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/queries/Query/Query.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/activity/explore/EventsScene.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/activity/explore/EventsScene.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
839 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
317.7 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
220.8 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
99.6 KiB src/lib/api.ts
93.6 KiB src/products.tsx
69.0 KiB src/lib/lemon-ui/icons/icons.tsx
40.7 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
29.0 KiB ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
317.7 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
280.8 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 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
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.6 KiB src/lib/api.ts
93.6 KiB src/products.tsx
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
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
Size File
317.7 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
280.8 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 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
181.9 KiB src/queries/validators.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
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.6 KiB src/lib/api.ts
93.6 KiB src/products.tsx
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Size File
317.7 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
280.8 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 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
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.6 KiB src/lib/api.ts
93.6 KiB src/products.tsx
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
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
Size File
317.7 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
280.8 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 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
181.9 KiB src/queries/validators.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
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.6 KiB src/lib/api.ts
93.6 KiB src/products.tsx
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
Size File
317.7 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
316.0 KiB ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
280.8 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 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
181.9 KiB src/queries/validators.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
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.6 KiB src/lib/api.ts
93.6 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.24 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.24 MiB · 19 files no change ████░░░░░░ 39.2% of 5.72 MiB
Deferred (lazy) 2.19 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
865.8 KiB dist/toolbar/toolbar-app-KNMTP6RT.css
671.0 KiB dist/toolbar/chunk-chunk-2OSLOBKH.js
259.5 KiB dist/toolbar/chunk-chunk-DS74BCMD.js
138.2 KiB dist/toolbar/chunk-chunk-3CWTGQZU.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-ENQUYK6R.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-VD4HB63I.js
21.7 KiB dist/toolbar/chunk-chunk-XFPYVCN6.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 — 🔺 +34.7 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1006.51 MiB · 🔺 +34.7 KiB (+0.0%)

ℹ️ MCP UI apps size — 32 app(s), 17214.8 KB JS

Built size of each MCP UI app (main.js + styles.css).

App JS CSS
debug 597.9 KB 203.4 KB
action 454.2 KB 203.4 KB
action-list 564.3 KB 203.4 KB
cohort 453.2 KB 203.4 KB
cohort-list 563.3 KB 203.4 KB
email-template 453.0 KB 203.4 KB
error-details 469.7 KB 203.4 KB
error-issue 454.6 KB 203.4 KB
error-issue-list 564.9 KB 203.4 KB
experiment 561.4 KB 203.4 KB
experiment-list 565.0 KB 203.4 KB
experiment-results 566.4 KB 203.4 KB
feature-flag 566.9 KB 203.4 KB
feature-flag-list 570.6 KB 203.4 KB
feature-flag-testing 457.4 KB 203.4 KB
inline-scan 453.7 KB 203.4 KB
insight-actors 562.4 KB 203.4 KB
llm-costs 559.4 KB 203.4 KB
session-recording 455.4 KB 203.4 KB
survey 454.8 KB 203.4 KB
survey-global-stats 562.0 KB 203.4 KB
survey-list 565.0 KB 203.4 KB
survey-stats 562.0 KB 203.4 KB
trace-span 453.6 KB 203.4 KB
trace-span-list 564.2 KB 203.4 KB
vision-observation-list 563.4 KB 203.4 KB
workflow 453.5 KB 203.4 KB
workflow-list 563.6 KB 203.4 KB
loops-review 457.9 KB 203.4 KB
query-results 789.9 KB 203.4 KB
render-ui 873.5 KB 203.4 KB
visual-review-snapshots 458.0 KB 203.4 KB
⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • System Status loaded (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

✅ Backend coverage — all changed backend lines covered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (35 / 35)

All changed backend lines are covered ✅

Per-product line coverage (touched products)
Product Coverage Lines
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
demo ███████████░░░░░░░░░ 55.9% 1,508 / 2,700
data_tools ████████████░░░░░░░░ 61.2% 90 / 147
warehouse_sources_queue █████████████░░░░░░░ 66.8% 1,921 / 2,875
aeo ███████████████░░░░░ 76.3% 617 / 809
batch_exports ████████████████░░░░ 80.1% 21,381 / 26,689
apm █████████████████░░░ 84.1% 1,306 / 1,553
mcp_analytics █████████████████░░░ 85.7% 4,844 / 5,651
ml_inference █████████████████░░░ 86.3% 524 / 607
warehouse_suggestions █████████████████░░░ 87.3% 2,252 / 2,580
posthog_ai ██████████████████░░ 87.7% 4,226 / 4,817
cdp ██████████████████░░ 88.4% 4,816 / 5,451
notebooks ██████████████████░░ 88.4% 15,201 / 17,196
today ██████████████████░░ 88.6% 2,866 / 3,233
mcp_registry ██████████████████░░ 89.0% 1,596 / 1,794
product_tours ██████████████████░░ 89.1% 1,337 / 1,500
webmcp ██████████████████░░ 89.6% 240 / 268
cohorts ██████████████████░░ 89.6% 8,458 / 9,438
signals ██████████████████░░ 89.8% 67,150 / 74,803
dashboards ██████████████████░░ 89.8% 7,107 / 7,911
data_modeling ██████████████████░░ 90.1% 10,839 / 12,026
ai_training ██████████████████░░ 90.4% 349 / 386
tasks ██████████████████░░ 90.6% 80,614 / 89,007
visual_review ██████████████████░░ 90.7% 9,842 / 10,856
data_warehouse ██████████████████░░ 90.8% 14,831 / 16,339
canvas ██████████████████░░ 90.9% 7,748 / 8,522
exports ██████████████████░░ 91.0% 9,698 / 10,663
business_knowledge ██████████████████░░ 91.0% 8,791 / 9,656
autoresearch ██████████████████░░ 91.1% 10,139 / 11,125
managed_warehouse ██████████████████░░ 91.2% 11,053 / 12,114
error_tracking ██████████████████░░ 91.6% 16,797 / 18,328
wizard ██████████████████░░ 91.8% 6,008 / 6,548
streamlit_apps ██████████████████░░ 91.8% 3,087 / 3,362
stamphog ██████████████████░░ 92.0% 8,242 / 8,963
conversations ██████████████████░░ 92.0% 29,714 / 32,284
managed_migrations ██████████████████░░ 92.5% 1,600 / 1,730
early_access_features ███████████████████░ 92.6% 1,339 / 1,446
alerts ███████████████████░ 92.9% 7,246 / 7,797
review_hog ███████████████████░ 93.0% 14,864 / 15,990
web_analytics ███████████████████░ 93.1% 23,971 / 25,751
surveys ███████████████████░ 93.1% 6,626 / 7,118
notifications ███████████████████░ 93.2% 1,152 / 1,236
approvals ███████████████████░ 93.3% 4,493 / 4,816
cross_project_dashboards ███████████████████░ 93.4% 880 / 942
slack_app ███████████████████░ 93.5% 14,854 / 15,882
context_layer ███████████████████░ 93.7% 3,409 / 3,640
workflows ███████████████████░ 93.7% 16,423 / 17,518
marketing_analytics ███████████████████░ 93.8% 20,074 / 21,402
messaging ███████████████████░ 93.8% 4,468 / 4,762
billing_alerts ███████████████████░ 93.9% 2,090 / 2,226
customer_analytics ███████████████████░ 93.9% 26,079 / 27,773
mcp_store ███████████████████░ 93.9% 9,008 / 9,593
experiments ███████████████████░ 94.2% 33,570 / 35,647
ai_observability ███████████████████░ 94.2% 26,434 / 28,069
replay_vision ███████████████████░ 94.3% 29,048 / 30,807
legal_documents ███████████████████░ 94.3% 2,289 / 2,427
logs ███████████████████░ 94.5% 15,850 / 16,770
actions ███████████████████░ 94.6% 973 / 1,029
endpoints ███████████████████░ 94.8% 9,298 / 9,812
reminders ███████████████████░ 94.8% 760 / 802
growth ███████████████████░ 94.8% 11,268 / 11,880
skills ███████████████████░ 94.9% 7,031 / 7,410
annotations ███████████████████░ 95.0% 816 / 859
tracing ███████████████████░ 95.2% 3,631 / 3,815
data_catalog ███████████████████░ 95.5% 4,419 / 4,628
access_control ███████████████████░ 95.5% 7,779 / 8,143
product_analytics ███████████████████░ 95.7% 28,602 / 29,879
alerts_platform ███████████████████░ 95.8% 5,012 / 5,234
revenue_analytics ███████████████████░ 95.9% 1,878 / 1,959
data_quality ███████████████████░ 95.9% 8,569 / 8,933
feature_flags ███████████████████░ 96.2% 26,868 / 27,932
warehouse_sources ███████████████████░ 96.4% 453,805 / 470,819
security ███████████████████░ 96.7% 1,452 / 1,501
pulse ███████████████████░ 97.4% 2,023 / 2,078
metrics ████████████████████ 97.7% 4,406 / 4,509
analytics_platform ████████████████████ 98.0% 2,775 / 2,833
field_notes ████████████████████ 99.4% 172 / 173

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

✅ Hogbox preview — ready, open the preview

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds: a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit 941f6b6 · box box-30e208222322 · ready in 1436s (push → usable) · build log · rebuilds on every push, torn down on close

@haacked haacked added the reviewhog ($$$) Reviews pull requests before humans do label Oct 9, 2026
@haacked
haacked requested a balanced review from Copilot October 9, 2026 00:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@posthog

posthog Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 1 should fix, 0 consider.

Published 1 finding (view the review).

Resolved comments: 1 left for you

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 2169b53d-f142-49e8-9eb5-94a7a1d39fb2

📥 Commits

Reviewing files that changed from the base of the PR and between cdbbb72 and 1fb4bd1.


📒 Files selected for processing (5)
  • frontend/src/lib/monaco/codeEditorLogic.tsx
  • frontend/src/lib/monaco/noticeLink.test.ts
  • frontend/src/lib/monaco/noticeLink.ts
  • posthog/hogql/metadata.py
  • posthog/hogql/test/test_metadata.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.



📝 Walkthrough

Walkthrough

HogQL notices now support an optional URL. When the move-notices feature flag provides a valid HTTPS URL with a hostname, the metadata path passes it to generated warnings. Monaco markers render a “Learn more” link when a notice has a URL and a Monaco instance is available. Tests cover missing or disabled flags and valid and invalid URLs.


Priority: ⬇️ Low

Merge Risk

Merge Risk: ⚪ Minimal · up to 1fb4b

The announcement link can proceed through normal checks; no actionable merge-blocking issue remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1fb4b

The server and editor independently restrict notice links to HTTPS, and rejected URLs leave the warning usable without a link. No security vulnerability was established in the inspected changes. Risk remains low rather than minimal because the actual editor link-opening implementation and permissions governing the announcement configuration were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A party able to control the move-notices payload can select an accepted HTTPS destination for applicable teams whose queries produce this warning. SQL text triggers warning generation but is not the destination source. The maximum configured team population and the privileges required to edit the payload remain unverified.

Security Findings and Attack Paths

  • inferred — The inspected controls counter direct command, JavaScript, and HTTP URL injection into marker links. No bypass was established. This conclusion is bounded by the unavailable real Monaco parser and link-opener implementation, rather than treating mocked parsing tests as end-to-end security proof.

Trust Boundaries and Controls

  • observed — The backend accepts only string payload URLs parsing as HTTPS with a hostname, strips surrounding whitespace, and logs rejected payloads with a bounded representation. The editor independently checks HTTPS before passing the URL to Monaco, including notice categories outside this specific warning producer.

Resilience and Maintainability Implications

  • observed — Invalid link configuration degrades to an unlinked warning rather than making the query invalid. Monaco parser exceptions likewise degrade to an unlinked marker, containing link-related failures within the existing advisory presentation path.

Hardening Proposals

  • proposed — Validate URI conversion and link opening against the resolved Monaco implementation, including malformed HTTPS and control-character cases. This would close the dependency-boundary proof gap; it is not evidence of an observed exploit.



Pre-merge checks | Passed 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check Passed The description follows the required template and clearly explains the problem, user-visible changes, security behavior, testing rationale, release status, documentation status, and agent context. It …


✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

The editor renders any notice url as a Monaco link, so it now checks the
url with isHttpsUrl and passes Monaco the trimmed value. The backend
returns the URL that urlparse checked, without the surrounding whitespace
that Monaco's URI parser rejects.

Claude-Session: https://claude.ai/code/session_017af6ERr59kp63wJFHzd4cX
@posthog

posthog Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

Comment thread posthog/hogql/metadata.py Outdated
@posthog posthog Bot removed the reviewhog ($$$) Reviews pull requests before humans do label Oct 9, 2026
@trunk-io

trunk-io Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Scenes-App/Experiments ExperimentWithHealthFindings play-test The test failed because it couldn't find an element with the text 'Why?', likely due to the text being split across multiple elements. Logs ↗︎
Scenes-Other/Startup program NeedsBillingDetails smoke-test The test timed out while waiting for loading indicators or spinners to disappear. Logs ↗︎
Scenes-App/OAuth/Authorize DefaultScopes smoke-test The test timed out while waiting for loading indicators or spinners to disappear. Logs ↗︎
Scenes-App/Project Homepage ProjectHomepage smoke-test The test timed out while waiting for loading indicators or spinners to disappear. Logs ↗︎

... and 2 more

View Full Report ↗︎ ⋅ Docs

The backend strips the payload URL before parsing, because urlparse keeps
trailing whitespace. A flag payload that is not a JSON object logs a
warning. The editor builds the notice link in noticeLink, which a Jest
test covers for https, command, javascript and http URLs.

Claude-Session: https://claude.ai/code/session_017af6ERr59kp63wJFHzd4cX
noticeLink trims the URL once and catches a Uri.parse error, so a URL that
isHttpsUrl accepts and Monaco rejects drops only the link. Every rejected
announcement payload logs one warning with the payload.

Claude-Session: https://claude.ai/code/session_017af6ERr59kp63wJFHzd4cX
@haacked
haacked marked this pull request as ready for review October 9, 2026 01:25
@haacked
haacked requested a review from a team as a code owner October 9, 2026 01:25
@haacked haacked added the stamphog Request AI approval (no full review) label Oct 9, 2026
@parameterai

parameterai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Risk: No findings

This change adds an optional "Learn more" URL to the HogQL $feature_flag_called warning: the flag payload supplies an https-only URL that the backend validates and the Monaco editor re-validates before rendering it as a link. The follow-up diff since the last review is only a master merge that regenerated codegen type files; the PR's own security checks (backend https/host validation with ValueError handling, frontend isHttpsUrl guard in noticeLink) are unchanged and remain sound, so no new risks are introduced.

Sentinel reviewed 941f6b6 · Review settings

stamphog[bot]

This comment was marked as outdated.

Comment thread posthog/hogql/metadata.py Outdated
if move_notices is None or not move_notices.enabled:
return []
return flag_called_on_events_warnings(hogql_ast, context)
return flag_called_on_events_warnings(hogql_ast, context, _flag_called_announcement_url(move_notices.payload))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bad flag payload logs a warning on every metadata request, even for queries that never mention the event

The URL is validated, and the hogql_flag_called_announcement_url_invalid log line is written, before the code looks for $feature_flag_called in the query. The editor requests metadata repeatedly as a person types. So if someone saves a typo in the flag payload (say http:// instead of https://), every team in the rollout writes that log line for every SQL query they edit, including SELECT 1. The description says it "logs one warning", which reads as once, not once per request. Nothing breaks for users; it is log volume only.

Find the warnings first and only validate the URL when there is at least one, for example by setting url on the returned notices in _flag_called_on_events_warnings after the list comes back non-empty. Or leave it and reword the description.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, thanks. Fixed in adbd3bf. The code now looks for the warnings first and checks the URL only when it finds at least one. A query without $feature_flag_called, such as SELECT 1, no longer logs anything. A new test covers that case.

A query that does contain the literal still logs once per metadata request while the payload is bad. I'll reword the description so it doesn't read as a single log line.

Base automatically changed from haacked/flag-called-sql-warning to master October 9, 2026 16:24
@stamphog
stamphog Bot dismissed their stale review October 9, 2026 16:25

The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.

@trunk-io

trunk-io Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

😎 Stack merged successfully - details.

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Oct 9, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved yet — waiting on the conditions below.

Re-add the stamphog label to request another review once you have addressed this.

The prerequisites gate refused this pull request because it has merge conflicts. Nothing else was evaluated against the code. The deny-list, size, and tier gates all passed, so the conflicts are the only thing blocking it.

To move forward, the author can resolve the conflicts by merging or rebasing onto the base branch and pushing the result. Regenerating the schema and snapshot files after the rebase will keep the generated output current. Once that's done, the pull request can be re-evaluated, or the author can ask a human reviewer to take a look.

  • georgemunyoro reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites ✗ merge conflicts present
deny-list ✓ no deny categories matched
size ✓ 198L, 7F substantive, 1974L/25F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (1974L, 25F, cross-cutting, feat)
stamphog 2.4.1 .stamphog/policy.yml @ unknown · reviewed head 1fb4bd1

@trunk-io
trunk-io Bot merged commit 66fd67f into master Oct 9, 2026
306 checks passed
@trunk-io
trunk-io Bot deleted the haacked/flag-called-sql-warning-link branch October 9, 2026 19:45
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-09 20:13 UTC Run
prod-us ✅ Deployed 2026-10-09 20:27 UTC Run
prod-eu ✅ Deployed 2026-10-09 20:28 UTC Run

This branch was successfully deployed

1 active deployment
preview-pr-114264 — 941f6b68 Deployed Oct 9, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants