Skip to content

fix(sql-editor): let users retry when a table's columns fail to load - #108413

Draft
posthog[bot] wants to merge 4 commits into
masterfrom
posthog-self-driving/fixsql-editor-let-users-retry-when-a-5b20ba
Draft

posthog[bot] wants to merge 4 commits into
masterfrom
posthog-self-driving/fixsql-editor-let-users-retry-when-a-5b20ba

Conversation

@posthog

@posthog posthog Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • In the SQL editor sidebar, a table or view can get stuck on "Couldn't load columns". The node gives no retry and no hint. The only way out is to collapse the node and expand it again.
  • When one request for a batch of tables fails, hydrateTableFieldsFailure marks every table in that batch as failed.
  • On the backend, an unmapped hogql column type on a saved query raised a KeyError in hogql_definition. That failed the whole catalog build, so every schema request for that team failed too. The FloatArrayDatabaseField mapping that caused one such burst landed in fix(data-warehouse): read back float array and map fields on synced views #106886. This PR removes the class of failure.
  • Nothing counted how often users see the node, or whether they recover.

Origin

  • Conversations: ticket
  • First signal: 2026-09-29
  • Inbox report: open
  • Likely cause: 7ebcc86
  • Task started by: auto-start, after the report was rated P3 and ready to fix

Changes

  • "Try again" under "Couldn't load columns". Clicking it sends the column request again for that one table only. This works for tables, views, managed views, endpoints, and lazy joins. The node matches the existing "Couldn't load your schema" retry.

  • Unmapped saved-query column types become unknown columns. Before, they failed the whole catalog build. This matches how warehouse tables already resolve new-style columns in products/warehouse_sources/backend/models/table.py.

  • Database.serialize skips a view whose fields fail to serialize. It records the view in _serialization_errors the same way as warehouse tables, instead of failing the batch.

  • New events to measure recovery:

    Event When Properties
    sql-editor-columns-load-failed A column request fails while the sidebar is mounted table_count, is_retry
    sql-editor-columns-retry-clicked The user clicks "Try again" none
    sql-editor-columns-retry-succeeded A retried request succeeds table_count
  • Not in this PR: a failure reason in the node. Most frontend failures are "Failed to read response body", a transport error with no useful message for the user.

Before After: error After: retry clicked
before-error after-error after-recovered

How did you test this code?

  • queryDatabaseLogic.test.ts: the existing error-node test now also checks the retry node, and that retryTableFields sends a new DatabaseSchemaQuery for only that table. Catches: the retry node missing, or the retry not reaching hydrateTableFields.
  • test_database.py::test_serialize_database_saved_query_with_unmapped_column_type: fails with KeyError: 'NotARealDatabaseField' without the fix, passes with it. Catches: one unknown column type failing the whole schema build.
  • Ran the serialize tests in test_database.py, ruff, oxlint, oxfmt, and the frontend typecheck for the changed files.
  • Rendered the LazySchema SQL editor story with a local mock that fails the first column request for one view. Clicked "Try again", and the columns loaded (screenshots above). The mock change is not committed.
  • Not done: manual testing against a real backend.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Fully autonomous

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

  • Started from an inbox report about the SQL editor sidebar. The report asked for the FloatArrayDatabaseField mapping, but it was already on master, so this PR hardens the lookup instead.
  • The retry node goes through the tree's onItemClick with the table name in the node record. The node builders are pure functions and have no access to logic actions.
  • Duplicate search (gh pr list --state open) found no open PR for this.
  • Committed test data is invented. Nothing from the session is in the diff.

Created with PostHog Desktop from this inbox report.

🤖 Generated with Claude Code

Add a "Try again" node under "Couldn't load columns" in the SQL editor sidebar. It hydrates only that table again. Capture events for the failure, the retry, and a successful retry.

On the backend, an unmapped column type on a saved query now becomes an unknown field. Before, it raised a KeyError and failed the whole catalog build. Schema serialization also skips a view whose fields fail to serialize, instead of failing the batch.

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

Generated-By: PostHog Desktop
Task-Id: 630d9020-d41a-4793-a279-36c6560bc88e
@posthog posthog Bot added the self-driving label Sep 29, 2026
@trunk-io

trunk-io Bot commented Sep 29, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@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) — 16 functions above the limit (max 52)

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/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx:599 52 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx:474 49 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:3447 30 10
createFieldNode frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1051 21 10
resolveFieldTraverserTarget frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:587 20 10
getFieldTypeIcon frontend/src/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx:247 17 10
getTableKindLabel frontend/src/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx:282 17 10
getFieldTypeIconClassName frontend/src/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx:217 16 10
createTopLevelFolderNode frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1598 15 10
getHydrationTableNamesForNode frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:319 14 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:3181 14 10
createPropertyDefinitionChildren frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:420 13 10
createSourceFolderNode frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1529 13 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx:450 11 10
createViewNode frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1379 11 10
<anonymous> frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:3859 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.

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

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

Total: 68.96 MiB · 🔺 +43.1 KiB (+0.1%)

File Size Δ vs base
render-query/src/render-query/render-query.js 20.17 MiB 🔺 +9.1 KiB (+0.0%)
posthog-app/_parent/products/data_modeling/frontend/nodeDetail/NodeDetailScene.js 33.3 KiB 🔺 +8.0 KiB (+31.6%)
exporter/src/queries/schema.js 1.23 MiB 🔺 +7.4 KiB (+0.6%)
posthog-app/src/queries/schema.js 1.23 MiB 🔺 +7.4 KiB (+0.6%)
posthog-app/src/scenes/notebooks/NotebookScene.js 37.0 KiB 🔺 +5.6 KiB (+17.7%)
posthog-app/_parent/products/mcp_analytics/frontend/MCPAnalyticsScene.js 185.5 KiB 🔺 +3.9 KiB (+2.1%)
exporter/src/exporter/scenes/ExporterNotebookScene.js 3.69 MiB 🔺 +1.3 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.58 MiB · 22 files no change █████████░ 85.8% 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 🔺 +1.1 KiB (+0.0%) █████████░ 87.4% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.35 MiB · 2,339 files 🔺 +1.2 KiB (+0.0%) █████████░ 88.1% 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.5 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.5 KiB src/products.tsx

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.16 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.16 MiB · 19 files no change ████░░░░░░ 37.8% of 5.72 MiB
Deferred (lazy) 2.10 MiB · 44 files 🔺 +60 B (+0.0%) 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
805.4 KiB dist/toolbar/toolbar-app-5UT2PX3W.css
651.7 KiB dist/toolbar/chunk-chunk-N5H2P45W.js
259.4 KiB dist/toolbar/chunk-chunk-UFQG5EMJ.js
138.3 KiB dist/toolbar/chunk-chunk-6OY66GIY.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-VY4JKXM4.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-MAVTJURD.js
21.0 KiB dist/toolbar/chunk-chunk-JBZQMGNO.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 — 🔺 +371.3 KiB (+0.0%)

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

Total: 947.68 MiB · 🔺 +371.3 KiB (+0.0%)

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

📝 Walkthrough

Walkthrough

The sidebar now displays retry controls for field-loading errors and requests hydration when a retry is clicked. Retry outcomes are tracked. Saved-query field mapping now falls back to an unknown field type, and view serialization handles QueryError and ResolutionError by recording the error and skipping the view.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to ffa85

While searching, users whose columns failed to load will not see the new Try again option. Retry analytics can also be slightly skewed. Fix the search path before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ffa85

Saved-view errors are less likely to block an entire schema request, and column retries use the existing access-controlled request path. No material security regression was identified, though not every downstream consumer or runtime reuse pattern was verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The new retry can repeat a schema request for one accepted table, but it reuses the existing team-scoped query path and suppresses duplicate in-flight hydration. The handled backend failures move from request-wide failure toward an unknown field or an omitted view.

Trust Boundaries and Controls

  • observed — The client supplies a table name and connection ID, while the backend resolves the connection with team and user context and limits serialization to requested tables. Client retry state is not the authorization control.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is mostly complete. It explains the problem, user-visible changes, backend safeguards, testing, release status, screenshots, and agent context. It does not state which skills were invo…
✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • 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)
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx-3941-3960 (1)

3941-3960: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Retry state is tracked in an unbounded, unscoped cache set, and the analytics events fire for every hydration failure.

cache.retriedTableNames is not typed, and the handlers read and mutate it directly. A retried table stays in the set until a success or failure action arrives for it. hydrateTableFields returns early when toLoad is empty, or when the table is already loading, loaded, or missing. In that case neither action fires, so the entry stays in the set. A later automatic hydration failure for the same table is then reported as is_retry: true. This skews the analytics.

sql-editor-columns-load-failed also fires for every hydration failure, not only for retries. This matches the PR goal, but the table_count value does not identify the failed tables.

Clear the entry when the retry is a no-op, or track it in a reducer instead of untyped cache.

The posthog.capture event names are inline literals. Check that they follow the naming convention of neighboring sql-editor-* events.


ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 16dabf43-2d33-4512-a6e8-591e483f22b1

📥 Commits

Reviewing files that changed from the base of the PR and between b6aa261 and cf85a86.

📒 Files selected for processing (6)
  • frontend/src/scenes/data-warehouse/editor/sidebar/QueryDatabase.tsx
  • frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.test.ts
  • frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx
  • posthog/hogql/database/database.py
  • posthog/hogql/database/test/test_database.py
  • products/data_modeling/backend/models/datawarehouse_saved_query.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 1 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

Revert the legacy column lookup change, which the report did not cover. Simplify how the logic stores the retried table names.

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

Generated-By: PostHog Desktop
Task-Id: 630d9020-d41a-4793-a279-36c6560bc88e

@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/backend/models/datawarehouse_saved_query.py-595-595 (1)

595-595: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add the unknown-type fallback for legacy columns.

If a persisted string-form column has a base type absent from LEGACY_CLICKHOUSE_HOGQL_MAPPING, direct indexing raises KeyError and can abort catalog construction. Use the existing UnknownDatabaseField fallback.

🐛 Suggested fix
-                fields[column] = LEGACY_CLICKHOUSE_HOGQL_MAPPING[hogql_type_str](name=column)
+                fields[column] = LEGACY_CLICKHOUSE_HOGQL_MAPPING.get(
+                    hogql_type_str, STR_TO_HOGQL_MAPPING["UnknownDatabaseField"]
+                )(name=column)

ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 2a130d5f-e793-4a75-b8dc-c2b34bead272

📥 Commits

Reviewing files that changed from the base of the PR and between cf85a86 and 488f310.

📒 Files selected for processing (2)
  • frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx
  • products/data_modeling/backend/models/datawarehouse_saved_query.py

Limit details: You’ve used all 12 included reviews currently available.

Clear a table from the retried set when the retry sends no request. Otherwise a later automatic failure reports as a retry.

Legacy string columns with an unmapped type now also become unknown fields instead of raising a KeyError.

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

Generated-By: PostHog Desktop
Task-Id: 630d9020-d41a-4793-a279-36c6560bc88e

@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)
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx-3948-3949 (1)

3948-3949: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Retain the retry marker only when hydration starts a request.

If tableFieldsStatus[tableName] is already loading, hydrateTableFields skips the table and starts no request. Keeping cache.retriedTableNames in that branch can cause the existing request to be counted as a retry.


ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 386d5791-8f3b-4f5b-8fdf-96eee2ac965b

📥 Commits

Reviewing files that changed from the base of the PR and between 488f310 and bd12fa0.

📒 Files selected for processing (3)
  • frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx
  • posthog/hogql/database/test/test_database.py
  • products/data_modeling/backend/models/datawarehouse_saved_query.py

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

If the table was already loading, the retry sends no request. The in-flight load must not count as a retry.

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

Generated-By: PostHog Desktop
Task-Id: 630d9020-d41a-4793-a279-36c6560bc88e

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

🟠 Major · Pass hydration state to search-result nodes. · queryDatabaseLogic.tsx:1277

frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1277
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Pass hydration state to search-result nodes.

When a column load fails, searchTreeData builds nodes without hydration. getTableFieldsState therefore returns ready, so the field error and “Try again” nodes do not appear in search results.

Search results intentionally include fields. Pass databaseFieldsComplete and tableFieldsStatus into the searchTreeData selector dependencies, selector arguments, and tableNodeOptions.


ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 2421cb46-8b12-4b0b-bcf5-33fb3ce5b640

📥 Commits

Reviewing files that changed from the base of the PR and between bd12fa0 and ffa85c6.

📒 Files selected for processing (1)
  • frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx

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

@posthog

posthog Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

👋 Visual changes detected for this PR.

Review and approve in PostHog Visual Review

If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix.

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant