feat(signals): filter inbox reports by scout name#73022
Conversation
Adds an optional per-scout sub-filter under the Scout entry in the inbox Source dropdown. Reports can be narrowed to those authored by specific scouts via a new comma-separated scout query param on the report list endpoint, matched against the skill_name each scout stamps on its emitted signals in ClickHouse. The selection mirrors to the URL (?scout=...) so filtered views are shareable.
|
😎 This pull request was merged. |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
products/signals/backend/temporal/signal_queries.py:632
**Scout Results Stop At 300**
When a scout matches more than 300 reports, this limit truncates the ID set before Django calculates the count or paginates the queryset. Matching reports outside that set never appear on later pages, and the inbox reports a count of at most 300.
### Issue 2 of 3
products/signals/backend/temporal/signal_queries.py:619
**Limited Set Has No Stable Order**
When more than 300 reports match, the outer `SELECT DISTINCT` has no ordering before the limit is applied. Separate list and count requests can therefore receive different 300-report subsets, causing reports to appear or disappear across refreshes even though the underlying signals did not change.
### Issue 3 of 3
frontend/src/scenes/inbox/components/shell/InboxSearchFilterBar.tsx:105
**Hydrated Selection Stays Collapsed**
`expanded` reads `scoutFilter` only when this component mounts. If URL navigation or persisted-state hydration adds a scout while the popover remains mounted, the active selection stays collapsed, so the stated auto-expand behavior does not occur until the component is remounted.
Reviews (1): Last reviewed commit: "feat(signals): filter inbox reports by s..." | Re-trigger Greptile |
|
Note 🤖 stamphog reviewed The new scout-filter query reuses the flagged 300-row cap with no stable ordering before the Django queryset filters/paginates on the result — Greptile's two unresolved P1 comments on signal_queries.py are real: matches beyond 300 silently vanish and the set can shift between list/count calls, producing inconsistent pagination and counts. Neither is addressed in the current diff.
Gate mechanics and policy version
Updated in place — this replaces 1 earlier stamphog review(s) on this PR. |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/src/scenes/inbox/InboxScene.js |
683.6 KiB | 🔺 +1.8 KiB (+0.3%) |
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.24 MiB · 22 files | no change | ███░░░░░░░ 27.5% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.25 MiB · 3,014 files | no change | ████████░░ 84.9% of 9.71 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/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers 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/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.5 KiB | ../node_modules/.pnpm/posthog-js@1.407.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 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 |
| 236.0 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.1 KiB | ../node_modules/.pnpm/posthog-js@1.407.1/node_modules/posthog-js/dist/module.js |
| 167.1 KiB | src/queries/validators.js |
| 154.3 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 |
| 105.8 KiB | src/lib/api.ts |
| 94.0 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 |
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.18 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.18 MiB · 17 files | no change | ████░░░░░░ 38.1% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 714.6 KiB | dist/toolbar/toolbar-app-7BRHIX2A.css |
| 545.3 KiB | dist/toolbar/chunk-chunk-HYYFIAXK.js |
| 484.2 KiB | dist/toolbar/chunk-chunk-QS5AHYGW.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-3HAV52CR.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-4WXEV7ZF.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-EL3T2QEA.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-HSIJKX5O.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +10.2 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1359.25 MiB · 🔺 +10.2 KiB (+0.0%)
⚠️ Backend snapshots — 11 updated (11 modified, 0 added, 0 deleted)
Query snapshots: Backend query snapshots updated
Changes: 11 snapshots (11 modified, 0 added, 0 deleted)
What this means:
- Query snapshots have been automatically updated to match current output
- These changes reflect modifications to database queries or schema
Next steps:
- Review the query changes to ensure they're intentional
- If unexpected, investigate what caused the query to change
⚠️ Backend coverage — 82.0% of changed backend lines covered — 5 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████░░░░ 82.0% (24 / 29)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/signals/backend/views.py |
50.0% | 846–848, 850–851 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 30007105288 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.6% |
8,412 / 21,250 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
██████████████░░░░░░ 69.2% |
29,524 / 42,687 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 79.6% |
21,198 / 26,630 |
cdp |
████████████████░░░░ 81.0% |
3,144 / 3,881 |
data_modeling |
█████████████████░░░ 82.8% |
5,529 / 6,675 |
wizard |
█████████████████░░░ 84.8% |
935 / 1,102 |
notebooks |
█████████████████░░░ 85.3% |
7,266 / 8,522 |
agent_platform |
█████████████████░░░ 86.4% |
3,807 / 4,405 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
cohorts |
██████████████████░░ 87.8% |
4,488 / 5,114 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
exports |
██████████████████░░ 88.4% |
6,949 / 7,861 |
data_warehouse |
██████████████████░░ 88.8% |
11,867 / 13,358 |
business_knowledge |
██████████████████░░ 89.0% |
4,391 / 4,936 |
engineering_analytics |
██████████████████░░ 89.2% |
5,641 / 6,323 |
conversations |
██████████████████░░ 89.3% |
16,820 / 18,834 |
dashboards |
██████████████████░░ 89.4% |
5,983 / 6,693 |
visual_review |
██████████████████░░ 89.4% |
5,818 / 6,505 |
error_tracking |
██████████████████░░ 89.7% |
10,174 / 11,342 |
alerts |
██████████████████░░ 89.9% |
4,054 / 4,508 |
early_access_features |
██████████████████░░ 90.1% |
1,031 / 1,144 |
mcp_analytics |
██████████████████░░ 90.1% |
2,763 / 3,065 |
streamlit_apps |
██████████████████░░ 90.4% |
2,501 / 2,767 |
links |
██████████████████░░ 90.6% |
183 / 202 |
slack_app |
██████████████████░░ 90.7% |
9,028 / 9,951 |
marketing_analytics |
██████████████████░░ 91.0% |
11,792 / 12,964 |
stamphog |
██████████████████░░ 91.1% |
4,056 / 4,450 |
product_analytics |
██████████████████░░ 91.3% |
5,811 / 6,362 |
mcp_store |
██████████████████░░ 91.9% |
4,257 / 4,634 |
notifications |
███████████████████░ 92.6% |
1,011 / 1,092 |
ai_observability |
███████████████████░ 92.7% |
14,995 / 16,168 |
surveys |
███████████████████░ 93.0% |
5,730 / 6,163 |
web_analytics |
███████████████████░ 93.0% |
14,178 / 15,237 |
managed_migrations |
███████████████████░ 93.1% |
1,424 / 1,530 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
workflows |
███████████████████░ 93.6% |
6,382 / 6,818 |
legal_documents |
███████████████████░ 94.1% |
1,568 / 1,667 |
endpoints |
███████████████████░ 94.1% |
8,640 / 9,177 |
messaging |
███████████████████░ 94.3% |
2,682 / 2,845 |
tracing |
███████████████████░ 94.5% |
2,670 / 2,826 |
skills |
███████████████████░ 94.5% |
2,893 / 3,061 |
revenue_analytics |
███████████████████░ 94.6% |
3,651 / 3,858 |
review_hog |
███████████████████░ 94.7% |
6,814 / 7,199 |
logs |
███████████████████░ 95.4% |
9,935 / 10,416 |
experiments |
███████████████████░ 95.7% |
25,036 / 26,159 |
growth |
███████████████████░ 96.1% |
3,245 / 3,376 |
replay_vision |
███████████████████░ 96.2% |
14,980 / 15,579 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
feature_flags |
███████████████████░ 96.5% |
17,110 / 17,734 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
customer_analytics |
███████████████████░ 96.5% |
8,796 / 9,113 |
access_control |
███████████████████░ 96.8% |
851 / 879 |
warehouse_sources |
███████████████████░ 97.1% |
328,358 / 338,088 |
data_catalog |
███████████████████░ 97.4% |
2,367 / 2,429 |
analytics_platform |
████████████████████ 98.0% |
2,153 / 2,197 |
metrics |
████████████████████ 98.2% |
2,491 / 2,536 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
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.
… snapshot Address review feedback and fix CI: - Order the scout/source report-id ClickHouse filters by max(timestamp) DESC before the cap so list and count calls see the same truncated set (was a non-deterministic SELECT DISTINCT ... LIMIT 300), and share the cap via a named _REPORT_ID_FILTER_CAP constant. - Auto-expand the scout sub-filter when a selection is hydrated into an already-mounted popover (URL nav / persisted state), not just at mount. - Add the new `scout` param to the inbox-reports-list MCP tool-schema snapshot. Generated-By: PostHog Code Task-Id: 9c045ba1-0a64-47b0-8bb7-c97821d16820
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93fc55067a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Additive, backward-compatible scout sub-filter mirroring the existing source_product filter shape; author is on the owning team, and the two substantive P1 correctness issues raised by Greptile (300-cap determinism/ordering) were fixed with named constant + deterministic ORDER BY, verified in a follow-up review reply. New tests cover the added ClickHouse query.
- Author wrote 7% of the modified lines and has 192 merged PRs in these paths (familiarity MODERATE).
- chatgpt-codex-connector[bot], greptile-apps[bot] reviewed the current head.
- Codex's P2 note that the 300-report cap is applied before Django's tab/status filters (so a scout with >300 reports could show fewer results on some tabs) was left unaddressed beyond the author's rationale — it's the same pre-existing tradeoff as the source_product filter, not a regression, so non-blocking, but worth a follow-up if scout signal volume grows.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 272L, 7F substantive, 345L/13F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (345L, 13F, cross-cutting, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 06066f0 · reviewed head 93fc550 |
|
Retaining stamphog approval — delta since last review classified as |
|
👋 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8014c7cb3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Scout source, narrowing to reports authored by specific scouts. | ||
| scoutFilter: [ | ||
| [] as string[], | ||
| { persist: true }, |
There was a problem hiding this comment.
Scope persisted scout filters by project
When a user selects a scout in one project and then opens a bare /inbox in another project, this persisted scoutFilter rehydrates the first project's team-specific skill_name; the bare-URL path then mirrors it back into the URL and reportListLogic sends scout=..., so the new project's inbox can look empty until the user notices and clears the Source filter. Persist this filter per project/team, or make scout selections URL-only instead of using the global kea-localstorage entry.
Useful? React with 👍 / 👎.
| JSONExtractBool(metadata, 'deleted') as is_deleted, | ||
| JSONExtractString(metadata, 'extra', 'skill_name') as skill_name, | ||
| timestamp | ||
| FROM ({_deduped_signals_subquery()}) |
There was a problem hiding this comment.
Bound scout filtering before deduping signals
When a scout filter is active on a signal-heavy team, this new query still calls the unbounded _deduped_signals_subquery(), so ClickHouse groups every signal document for the team before it can discard all but the selected skill_names. That makes filtering to one scout pay the full-history argMax cost for each list/count request; push a extra.skill_name IN ({scout_names}) candidate bound into the dedup scan so the work scales with the selected scouts rather than the team's entire signal history.
Useful? React with 👍 / 👎.
Problem
The inbox Source filter can narrow reports to the Scout source as a whole, but a project with a large scout troop (dozens of scouts on patrol) has no way to see what one specific scout has been reporting. You can filter to "Scout" but not to "the error tracking scout".
Changes
Adds an optional per-scout sub-filter nested under the Scout entry in the Source dropdown. Filtering by source still works exactly as before, and you can now additionally (or independently) pick specific scouts.
With a scout selected, the list narrows to reports authored by that scout, the chip labels itself "Scout · Error tracking" (disambiguating from the same-named source), and the selection mirrors to the URL (
?scout=signals-scout-error-tracking) so filtered views are shareable:How it works:
scoutquery param on the signal report list endpoint (comma-separatedskill_nameslugs), documented viaOpenApiParameter. It resolves matching report ids through a newfetch_report_ids_for_scout_namesClickHouse query that matches theextra.skill_nameeach scout already stamps on its emitted signals, mirroring the existingsource_productfilter (same argMax dedup, deleted-signal exclusion, and 300-report cap). It combines withsource_productas an AND.inboxFiltersLogicgains a persistedscoutFilterwith URL sync (scout=),reportListLogicforwards it to the list API, and the Source popover renders a collapsible "Filter by scout" section under the Scout row, listing the team's scout configs (selected scouts stay listed even if their config was deleted, so they can be untoggled). Scout slugs are team-specific and dynamic, so shared links accept any slug rather than validating against a static set like sources do — an unknown scout just matches nothing server-side.How did you test this code?
TestFetchReportIdsForScoutNames(2 cases) to the existing ClickHouse-backed harness intest_signal_queries.py— catches a brokenextra.skill_nameJSON path or lost deleted-signal/argMax handling, which would silently empty every scout-filtered view. No existing test exercises this query.inboxFiltersLogic.test.tswith scout slugs — catches a refactor that runs scout params through the static valid-set validation used for sources, which would silently drop every scout from shared links.ruff, oxlint/oxfmt, fulltypescript:check, repo-wide mypy, andhogli ci:preflight --fixall pass.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Claude Code at Andy's direction (add a way to filter inbox reports by individual scout, keeping the existing Scout source filter). Skills invoked: /improving-drf-endpoints, /writing-tests, /run-posthog.
Decisions: reused the scout
skill_namealready stamped on signals in ClickHouse rather than adding any new persisted state; mirrored the existingsource_productfilter shape for the backend param; nested the scout list inside the Source popover (collapsed by default, auto-open while active) instead of adding another top-level filter chip; scoped the sub-filter options toscoutFleetLogic's configs, which the inbox scene already loads. No backend viewset test was added for the param plumbing itself — the existingsource_producttwin has none either, and the ClickHouse query (where the real logic lives) is covered.