Skip to content

feat(data-modeling): add lineage search result navigator - #108501

Open
sakce wants to merge 7 commits into
masterfrom
posthog/improve-model-lineage-search
Open

sakce wants to merge 7 commits into
masterfrom
posthog/improve-model-lineage-search

Conversation

@sakce

@sakce sakce commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The Models lineage search highlights matching nodes, but it does not show the full match list. Users cannot move through several name matches or lineage selector results with clear graph context.

Changes

  • Show ranked name matches in a floating graph panel.
  • Show upstream, downstream, and bidirectional selector cones in the same panel.
  • Order selector results by distance from the anchor, then by name.
  • Let users select matches with the arrow keys.
  • Center the selected node after Enter, a row click, or a previous or next action.
  • Keep the search input active and preserve a trailing + while users refine the anchor.
  • Keep plain-search matches visible and preserve existing lineage selector pruning.
  • Add a stronger selected-node state and an accessible result announcement.
  • Capture result focus without search text or model identifiers.

Screenshots

Matching models

Lineage search results panel with six matching models

Focused result

Lineage graph centered on the second selected search result

Downstream selector

Downstream lineage selector with the anchor and ordered models

Verification

  • pnpm --filter=@posthog/frontend exec jest --runInBand products/data_modeling/frontend/lineage/lineageSearch.test.ts products/data_modeling/frontend/lineage/modelsLineageLogic.test.ts
  • Targeted oxfmt and oxlint checks
  • git diff --check
  • Live stack browser checks with persisted models for plain and downstream searches

Created with PostHog Desktop

Show ranked model matches over the lineage graph and let users select and focus each result. Preserve lineage selector behavior and record privacy-safe focus telemetry.

Generated-By: PostHog Desktop
Task-Id: bdecc588-d61b-4442-87da-951f4b4f03d5
@trunk-io

trunk-io Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

✨ Submitted to Merge by Jovan Sakovic (@sakce). It will be added to the merge queue once all branch protection rules pass. See more details here.

@github-actions

github-actions Bot commented Sep 29, 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) — 3 functions above the limit (max 50)

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/lemon-ui/LemonButton/LemonButton.tsx:147 50 10
LineageNode products/data_modeling/frontend/lineage/LineageNode.tsx:193 42 10
LineageGraphContent products/data_modeling/frontend/lineage/LineageGraph.tsx:59 14 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 — 4% of added code lines are comments (28 of 668)

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
products/data_modeling/frontend/lineage/modelsLineageLogic.ts 10 188
products/data_modeling/frontend/lineage/lineageSearch.ts 6 60
products/data_modeling/frontend/lineage/LineageGraph.stories.tsx 5 77
products/data_modeling/frontend/lineage/LineageSearchResults.tsx 3 108
products/data_modeling/frontend/lineage/LineageGraph.tsx 2 14
products/data_modeling/frontend/lineage/ModelsLineageTab.tsx 2 76

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

⚠️ Bundle size — 🔺 +40.8 KiB (+0.1%)

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

Total: 69.00 MiB · 🔺 +40.8 KiB (+0.1%)

File Size Δ vs base
posthog-app/_parent/products/experiments/frontend/scenes/ExperimentsStaffToolsScene.js 37.7 KiB 🔺 +37.7 KiB (new)
posthog-app/src/scenes/experiments/staff/ExperimentsStaffTools.js removed 🟢 -37.7 KiB (-100.0%)
posthog-app/_parent/products/customer_analytics/frontend/scenes/CustomerAnalyticsAccountScene/CustomerAnalyticsAccountScene.js 46.1 KiB 🔺 +16.5 KiB (+55.9%)
posthog-app/_parent/products/feature_flags/frontend/staff/FeatureFlagsStaffToolsScene.js 58.6 KiB 🔺 +6.3 KiB (+12.1%)
posthog-app/_parent/products/data_modeling/frontend/ModelsScene.js 39.5 KiB 🔺 +5.8 KiB (+17.1%)
render-query/src/render-query/render-query.js 20.18 MiB 🔺 +3.5 KiB (+0.0%)
posthog-app/src/scenes/web-analytics/WebAnalyticsScene.js 230.3 KiB 🔺 +2.3 KiB (+1.0%)
posthog-app/_parent/products/signals/frontend/inbox/InboxScene.js 491.0 KiB 🔺 +1.8 KiB (+0.4%)

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.58 MiB · 22 files 🔺 +3.4 KiB (+0.2%) █████████░ 86.0% 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.52 MiB · 629 files 🔺 +3.4 KiB (+0.1%) █████████░ 87.5% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.35 MiB · 2,339 files 🔺 +6.1 KiB (+0.1%) █████████░ 88.2% 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.9 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.5 KiB src/lib/api.ts
88.7 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
272.3 KiB src/taxonomy/core-filter-definitions-by-group.json
216.9 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.5 KiB src/lib/api.ts
98.8 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.7 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.17 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.17 MiB · 19 files 🔺 +2.5 KiB (+0.1%) ████░░░░░░ 37.9% 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
808.0 KiB dist/toolbar/toolbar-app-ZL2NNVOT.css
651.6 KiB dist/toolbar/chunk-chunk-HHMSQDN2.js
259.4 KiB dist/toolbar/chunk-chunk-OMXHH7OQ.js
138.3 KiB dist/toolbar/chunk-chunk-A5ADGIAJ.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-JQKUHDU5.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-DHBF5L3O.js
21.0 KiB dist/toolbar/chunk-chunk-ZIBMJ4N2.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 — 🔺 +734.9 KiB (+0.1%)

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

Total: 948.39 MiB · 🔺 +734.9 KiB (+0.1%)

✅ Playwright — all passed

All tests passed.

View test results →

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Adds search result navigation to the lineage graph UI.

The PR appears safe to merge; no new actionable issue or outstanding previous finding remains.

Reviews (2) · Last reviewed commit: "fix(data-modeling): bound and correct th..."

Comment thread products/data_modeling/frontend/lineage/modelsLineageLogic.ts Outdated
Comment thread products/data_modeling/frontend/lineage/LineageSearchResults.tsx
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Lineage search now displays matching models and supports result selection and keyboard navigation. Search results include ordered lineage matches, type filtering, and an accessible result announcement. The selected graph node receives distinct styling. Focusing a result sends a request that fits the graph viewport to that node. Stories exercise search and selector focus, and tests cover ordering, filtering, navigation, focus requests, debounce reset, loading, and unmatched searches.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 1f6ce

Search results and graph focus work as described. Screen readers may hear duplicate announcements, and the graph may occasionally fail to center on a selected node right after a layout change. Both are minor and bounded, so the change is mergeable with follow-up.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1f6ce

The change is concentrated in the lineage interface. Review found no confirmed new access to data outside the loaded graph or transmission of search text or model identifiers, though some scope and recovery behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Search and focus operate on the lineage nodes loaded into this frontend flow; the reviewed source does not establish how authorization or tenant changes govern replacement of that node set.

Trust Boundaries and Controls

  • observed — Normal UI focus actions originate from displayed results; analytics checks current-result membership, and graph focus checks current-layout membership. The focus reducer itself does not validate an ID against current results.

Resilience and Maintainability Implications

  • inferred — During debounce, a plain-name result may be selectable before the graph leaves its prior selector-pruned layout. The layout check prevents focusing an absent node, but the panel and graph can temporarily disagree.

Hardening Proposals

  • proposed — If the loaded node set can change authorization scope without clearing search state, validate focus requests against current results and clear them on scope replacement.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, user-visible changes, screenshots, and verification steps. It omits the template sections for Release status, Automatic notifications, Docs update, and Ag…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@coderabbitai coderabbitai Bot 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.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
products/data_modeling/frontend/lineage/LineageSearchResults.tsx-35-43 (1)

35-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the live region from the visible result count.

The count region updates only when results.length changes. When a query changes both the selection and the count, it can announce the total again because the hidden region already includes result X of Y. Removing aria-live from the visible count preserves the hidden result and empty-state announcements.

Proposed fix
-                <span aria-live="polite">
+                <span>
                     {results.length} {results.length === 1 ? 'result' : 'results'}
                 </span>

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 92aab098-8d32-44ed-816b-de693992f31e

📥 Commits

Reviewing files that changed from the base of the PR and between bbad9e7 and befac46.

📒 Files selected for processing (7)
  • products/data_modeling/frontend/lineage/LineageGraph.stories.tsx
  • products/data_modeling/frontend/lineage/LineageGraph.tsx
  • products/data_modeling/frontend/lineage/LineageNode.tsx
  • products/data_modeling/frontend/lineage/LineageSearchResults.tsx
  • products/data_modeling/frontend/lineage/ModelsLineageTab.tsx
  • products/data_modeling/frontend/lineage/modelsLineageLogic.test.ts
  • products/data_modeling/frontend/lineage/modelsLineageLogic.ts

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

@trunk-io

trunk-io Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

Return focus to the search input after mouse-based result navigation. Users can continue typing to narrow the query without another click.

Generated-By: PostHog Desktop
Task-Id: bdecc588-d61b-4442-87da-951f4b4f03d5
Show upstream, downstream, and bidirectional selector cones in the floating result navigator. Order models by distance from the anchor and preserve the trailing selector while users refine a search.

Generated-By: PostHog Desktop
Task-Id: bdecc588-d61b-4442-87da-951f4b4f03d5

@coderabbitai coderabbitai Bot 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.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
products/data_modeling/frontend/lineage/LineageSearchResults.tsx-52-52 (1)

52-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the second aria-live region.

Line 46 already has a polite live region for the selected result or the empty state. Line 52 adds another live region for the count label. When results change, screen readers can announce both regions, so users hear duplicate or competing updates. Keep one live region, and put the count in its text if needed.

Proposed fix
-                <span aria-live="polite">{resultCountLabel}</span>
+                <span>{resultCountLabel}</span>

Source: Learnings


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 1fe3a79a-02d0-4b5b-a8f5-980b3c4ff725

📥 Commits

Reviewing files that changed from the base of the PR and between befac46 and 63bb8fc.

📒 Files selected for processing (7)
  • products/data_modeling/frontend/lineage/LineageGraph.stories.tsx
  • products/data_modeling/frontend/lineage/LineageSearchResults.tsx
  • products/data_modeling/frontend/lineage/ModelsLineageTab.tsx
  • products/data_modeling/frontend/lineage/lineageSearch.test.ts
  • products/data_modeling/frontend/lineage/lineageSearch.ts
  • products/data_modeling/frontend/lineage/modelsLineageLogic.test.ts
  • products/data_modeling/frontend/lineage/modelsLineageLogic.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.

The lineage logic captures a search-focus event, but the product package
never declared posthog-js, so esbuild and tsgo both failed to resolve it.

Also addresses review feedback on the search result navigator:
- focusNodeIds signals "no viewport focus requested" with null, not undefined
- result rows use aria-current rather than aria-pressed, which presented them
  as toggles; the live region already announces the active result, so the
  duplicate one on the count label goes away

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 8d64b915-89d0-4476-9c8d-e8209d172986
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 29, 2026 17:29
Both play functions pressed ArrowDown and Enter in the same tick. The arrow key
moves the selection through a kea listener, which runs on the next tick, so Enter
always focused the first result and the viewport centred the wrong node.

Pruning the graph also rekeys lineageGraphLogic, so react-flow unmounts while ELK
lays the cone out again. The selector story held element references from before
that, and a detached element reports a zero-sized rect that passes any centring
check. Read the canvas on every poll instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 8d64b915-89d0-4476-9c8d-e8209d172986

sakce commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor Author

Everything on this PR is green except the Visual Review gate, which needs a human to approve. All 16 visual-regression shards pass; Visual regression tests pass, Complete Visual Review run and PostHog Visual Review / storybook are all red for the same reason — 4 unresolved snapshots on the Visual Review run linked from the PostHog Visual Review / storybook check. Each new commit starts a fresh run and supersedes the previous one, so open the run from that check rather than from a link in a comment.

I pulled the baseline and current PNGs and checked all four. Every one belongs to this PR's own lineage-search stories, and each diff is intended:

Snapshot Result What changed
...--search-focus--light / --dark changed (1.3% / 1.1%) The story now searches monthly instead of monthly_report, so the results panel appears and the run ends on "1 of 2" after the Previous-result click.
...--selector-focus--light / --dark new New story. Shows revenue_summary+ pruning the graph to 4 models with the second result selected and centred.

Also fixed in this branch while getting CI green:

  • products/data_modeling never declared posthog-js, so esbuild and tsgo both failed to resolve the import the search-tracking call added.
  • Both story play functions pressed ArrowDown and Enter in the same tick. The selection moves through a kea listener that runs on the next tick, so Enter always focused the first result and the viewport centred the wrong node. They now wait for the selection before pressing Enter.
  • Pruning the graph rekeys lineageGraphLogic, so react-flow unmounts while ELK relays out. SelectorFocus held element references from before that, and a detached element reports a zero-sized rect that passes any centring check. It reads the canvas on every poll now.

Nothing else is outstanding — both review threads are answered and resolved, and the branch merges cleanly.

🦉 via talyn.dev

Review findings on the search result navigator.

The panel rendered one button per match with no cap, and a plain search runs on
every keystroke, so a one-letter term on a large warehouse mounted thousands of
buttons into a list that shows about eight rows. It now renders a window of 50
that holds the selection, which keeps arrow navigation over every result.

Nodes and edges load in parallel, so the panel could answer a lineage selector
from an empty edge list and report a model as having no dependents. It now waits
for edges in the modes that walk them, the same guard viewsTabLogic uses.

The live region was mounted together with its first text. A region inserted with
its text already set is not announced, so the first search said nothing. It moves
to ModelsLineageTab where it stays mounted.

Arrow keys restored the caret in an input that never lost focus, which moved it
away from where the user had put it.

Also: one shared selector for the lineage walk rather than two traversals per
settled search, one lowercase pass per name in matchNodesByName rather than two
per comparison, the still-accurate half of the deleted fitView comment, and
sentence case for the keyboard hint.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 8d64b915-89d0-4476-9c8d-e8209d172986

sakce commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Worked through 11 findings from three review passes. Nine are fixed in 1f6ce8b, one was already fixed earlier in this branch, one I'm declining.

Fixed

  • Unbounded result list. searchResults has no cap and a plain search runs on every keystroke, so a one-letter term on a large warehouse mounted one LemonButton per match into a list that shows about eight rows. It now renders a 50-row window that holds the selection, so arrow navigation still reaches every result and the count stays accurate.
  • Lineage cone from a half-loaded edge list. Nodes and edges load in parallel, so nodesLoading === false while edgesLoading === true is reachable. A selector like orders+ ran against an empty edge list and the panel reported "1 model · downstream" as a real answer. showSearchResults now waits for edges in the modes that walk them, matching the guard in viewsTabLogic's lineageNames. Covered by a new logic test, which I confirmed fails without the fix.
  • First screen-reader announcement dropped. The live region was mounted together with its first text, and a region inserted with its text already set is not announced. It moves to ModelsLineageTab and stays mounted, fed by a new searchResultAnnouncement selector.
  • Arrow keys moved the caret. focusSearchInput() ran from the ArrowUp/ArrowDown branch, where focus never left the input, so its only effect was setSelectionRange. Put the caret mid-term, press ArrowDown, and the next character landed at the end instead. The click / previous / next paths still restore it, since focus genuinely returns there.
  • Lineage walk ran twice. searchResults called orderedNodesForLineageSearch while visibleNodes called nodeIdsForLineageSearch, which wraps the same function. Both invalidate together, so every settled selector search built the adjacency maps and walked the graph twice. They now share one orderedLineageNodes selector.
  • Per-keystroke matching cost. matchNodesByName lowercased both names inside the comparator, which runs O(n log n) times. It now lowercases each name once during the filter pass. Same ordering, same results.
  • Deleted comment. Restored the half that still describes live behaviour: why an empty focusNodeIds means refit everything rather than fit nothing.
  • Keyboard hint copy. Now ↑↓ to select · Enter to center · Esc to clear - parallel, sentence case, and "center" says what Enter actually does.
  • Two live regions was already fixed earlier in this branch; the count label lost its aria-live and the sr-only region is the single announcement.

Declining: full-graph re-render on arrow-key selection

The cost is real, but the suggested fix does not reach it. Memoising decoratedNodes on [layout, currentNodeId, nodeState, nodeCallbacks] cannot help, because nodeState is an inline closure over selectedSearchResult - the exact value an arrow key changes - so the memo misses on every press. React.memo on LineageNode does not help either, since data is a fresh object each pass. Making this actually cheaper means having LineageNode read the selection from the logic itself, which changes how per-node state reaches the graph and also touches the pre-existing currentNodeId and isHighlighted paths. That is a redesign rather than a fix, and decoratedNodes already rebuilt unconditionally before this PR. Worth doing, but as its own change.

One thing I left alone and want to flag: buildAdjacencyMaps spreads into a new array per edge, so it is O(D²) in a node's degree. Now that the traversal runs once instead of twice this PR no longer makes it worse, so I kept it out of the diff - but a hub table feeding several hundred models pays for it on every lineage search.

Verified locally: jest 42/42 on the lineage suite, tsgo clean, oxfmt clean, and all 7 LineageGraph stories pass against a locally built Storybook.

🦉 via talyn.dev

@sakce
sakce removed the request for review from a team September 29, 2026 18:34
@sakce sakce added the stamphog Request AI approval (no full review) label Sep 29, 2026

@coderabbitai coderabbitai Bot 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.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · 🩺 Stability & Availability · LineageGraph.tsx:90-95

products/data_modeling/frontend/lineage/LineageGraph.tsx:90-95
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

The focus attempt can run before the requested node is measured. The effect depends on layout, but it does not depend on React Flow node measurements or retry after measurement. Gate fitView on node readiness, or schedule the focus after the nodes are measured.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: e71f85d8-4849-498f-9733-9c2f135c57d3

📥 Commits

Reviewing files that changed from the base of the PR and between 782848f and 1f6ce8b.

📒 Files selected for processing (6)
  • products/data_modeling/frontend/lineage/LineageGraph.tsx
  • products/data_modeling/frontend/lineage/LineageSearchResults.tsx
  • products/data_modeling/frontend/lineage/ModelsLineageTab.tsx
  • products/data_modeling/frontend/lineage/lineageSearch.ts
  • products/data_modeling/frontend/lineage/modelsLineageLogic.test.ts
  • products/data_modeling/frontend/lineage/modelsLineageLogic.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.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ 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 9913540 · box box-723f64c6e290 · ready in 647s (push → usable) · build log · rebuilds on every push, torn down on close

@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.

stamphog can't auto-approve this pull request because two gates refused it. The deny-list gate flagged it for deps_toolchain, most likely because it touches pnpm-lock.yaml and products/data_modeling/package.json (the new posthog-js dependency). The tier gate classified it as T2-never: a 796-line, 12-file feature spanning two areas, including a shared LemonButton component change.

Please ask a human reviewer to look at it. To make review easier, you could split the dependency change and the shared LemonButton tweak into separate pull requests from the lineage search navigator work.

  • coderabbitai[bot] reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✗ matches: deps_toolchain
size ✓ 631L, 10F substantive, 796L/12F incl. docs/generated/snapshots — within ceiling
tier ✗ classified as T2-never: T2-never (796L, 12F, two-areas, feat)
stamphog 2.3.1 .stamphog/policy.yml @ unknown · reviewed head 1f6ce8b

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Sep 29, 2026
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 29, 2026 18:41
@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

  • @PostHog/platform-ux (owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). For a skipped owner, the locator is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

@posthog

posthog Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ Visual changes approved by @sakce — baseline updated in 9913540.

View this run in PostHog

2 changed, 2 new.

Install the Visual Review Chrome extension to see visual review results at the top of your pull requests.

4 updated
Run: 39fe746b-9d9e-4476-bb95-0fbb6099e73b

Co-authored-by: sakce <49978945+sakce@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
preview-pr-108501 — 99135408 Deployed Sep 29, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants