Skip to content

fix(notifications): guard open-goals banner SELECT behind table pre-flight (42P01)#275

Merged
khustup2 merged 1 commit into
mainfrom
fix/open-goals-table-preflight-42p01
Jun 19, 2026
Merged

fix(notifications): guard open-goals banner SELECT behind table pre-flight (42P01)#275
khustup2 merged 1 commit into
mainfrom
fix/open-goals-table-preflight-42p01

Conversation

@khustup2

@khustup2 khustup2 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Problem

pg-deeplake prod logs ~150 sqlstate=42P01 ERROR: relation \"hivemind_goals\" does not exist per day, spread across many orgs. Source: the SessionStart banner path (fetchOpenGoalslistOpenGoals) issues the goals SELECT unconditionally. The SessionStart context-block path already guards the same read with knownTablesOrNull() and skips it when the table is absent. On any org that never created hivemind_goals (i.e. never wrote a goal), the banner sends the SELECT on every SessionStart; the server logs 42P01 even though the client-side try/catch hides it from the user.

Fix

Add the same pre-flight in fetchOpenGoals:

  • knownTablesOrNull() returns the trusted table list → if it omits the goals table, skip the read (no server-side 42P01).
  • returns null (untrusted/lookup failed) → fall back to SELECT-then-catch, so a transient blip can't hide a table that really exists.

This mirrors the proven context-block pattern exactly and does not create the table as a side effect (the banner is read-only/best-effort).

Tests

Extended the existing mock with knownTablesOrNull and added 3 cases: skip-when-absent (asserts query never called), query-when-present, fall-back-when-null. 21/21 green.

Summary by CodeRabbit

  • Bug Fixes
    • Optimized open goals notifications by verifying table availability before querying, preventing failed requests in organizations without the required table setup.

…light

The SessionStart banner path (fetchOpenGoals -> listOpenGoals) issued the
hivemind_goals SELECT unconditionally, while the context-block path already
skips it when the table is absent (knownTablesOrNull). On any org that never
created hivemind_goals, the banner logged a server-side 42P01 on every
SessionStart even though the client-side catch hid it from the user
(~150 such errors/day in prod across many orgs).

Add the same knownTablesOrNull() pre-flight: skip the read when the trusted
table list omits the goals table, fall back to SELECT-then-catch when the
list is untrusted (null) so a transient lookup blip can't hide a real table.

Tests: extend the mock with knownTablesOrNull and add 3 cases (skip when
absent / query when present / fall back when null). 21/21 green.
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4430ba68-2f21-4838-803f-0da719cf1910

📥 Commits

Reviewing files that changed from the base of the PR and between 94b66dd and ddf5019.

📒 Files selected for processing (2)
  • src/notifications/sources/open-goals.ts
  • tests/claude-code/notifications-open-goals-source.test.ts

📝 Walkthrough

Walkthrough

fetchOpenGoals gains a pre-flight call to api.knownTablesOrNull(); if hivemind_goals is confirmed absent, the function logs and returns null without issuing the SELECT. Tests add a knownTablesMock, wire it into the DeeplakeApi mock, and cover three outcomes: absent, present, and untrusted (null).

Changes

fetchOpenGoals table-existence pre-flight guard

Layer / File(s) Summary
fetchOpenGoals guard implementation
src/notifications/sources/open-goals.ts
Calls api.knownTablesOrNull() at the start of fetchOpenGoals; returns null immediately with a log when hivemind_goals is absent from the trusted table list, leaving the existing listOpenGoals path unchanged otherwise.
Test mock wiring and pre-flight scenarios
tests/claude-code/notifications-open-goals-source.test.ts
Adds knownTablesMock defaulting to null, extends the DeeplakeApi mock class with knownTablesOrNull(), resets the mock in beforeEach, and introduces a new describe block with three tests covering the absent, present, and untrusted-list outcomes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • activeloopai/hivemind#209: Directly related — both PRs use DeeplakeApi.knownTablesOrNull() to skip the open-goals SELECT when the hivemind_goals table is known to be absent.

Suggested reviewers

  • efenocchi

🐇 A hop, a skip, a table-check done right,
No query sent when the table's out of sight!
knownTablesOrNull() guards the gate,
Returns a null before it's too late.
Three tests now prove the bunny's delight~ 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes a clear Problem section, detailed Fix explanation, and Tests section, but does not include a Version Bump section as required by the template. Add a Version Bump section indicating which version bump is appropriate (patch, minor, or major) and update package.json version accordingly, or explicitly state if no release is needed.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly summarizes the main change: adding a pre-flight guard check to the open-goals banner SELECT query to prevent 42P01 errors.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/open-goals-table-preflight-42p01

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Scope: files changed in this PR. Enforced threshold: 90% per metric (per file via vitest.config.ts).

Status Category Percentage Covered / Total
🟢 Lines 96.77% (🎯 90%) 30 / 31
🟢 Statements 95.12% (🎯 90%) 39 / 41
🟢 Functions 100.00% (🎯 90%) 8 / 8
🔴 Branches 87.10% (🎯 90%) 27 / 31
File Coverage — 1 file changed
File Stmts Branches Functions Lines
src/notifications/sources/open-goals.ts 🟢 95.1% 🔴 87.1% 🟢 100.0% 🟢 96.8%

Generated for commit 03e90b4.

@khustup2 khustup2 merged commit dea23c1 into main Jun 19, 2026
11 checks passed
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.

1 participant