Skip to content

⚡ Bolt: optimize UAT repository initialization #22297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BrianCLong wants to merge 2 commits into from
Closed

⚡ Bolt: optimize UAT repository initialization #22297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a493942
perf(db): optimize UAT repository initialization
google-labs-jules[bot] Mar 31, 2026
5c4e604
perf(db): optimize UAT repository initialization
google-labs-jules[bot] Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,7 @@
## 2026-06-21 - [Avoid Unnecessary Redis pings]
**Learning:** Repeatedly calling `redis_client.ping()` as a precondition check before actually executing redis commands creates an unnecessary N+1 round-trip performance bottleneck. If a command fails due to connection issues, the underlying library will raise an exception (like `redis.exceptions.ConnectionError`) regardless.
**Action:** Avoid explicit `ping()` calls for connection validation before issuing commands. Rely on exception handling blocks around the actual commands.

## 2026-03-31 - [Promise-based Repository Initialization Guard]
**Learning:** Calling schema setup functions like `ensureTable` (containing `CREATE TABLE/INDEX IF NOT EXISTS`) on every database operation creates a redundant round-trip bottleneck. Using a promise-based singleton guard ensures this logic runs only once per application lifecycle while safely handling concurrent calls.
**Action:** Always implement a module-level `initializationPromise` to guard repository schema setup logic.
50 changes: 31 additions & 19 deletions server/src/db/repositories/uat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import baseLogger from '../../config/logger.js';

const logger = baseLogger.child({ name: 'uat-repo' });

// BOLT OPTIMIZATION: Guard initialization to prevent redundant schema checks on every operation.
// This reduces database round-trips for high-frequency UAT event logging.
// Pattern also used in high-throughput tickets repository.
let initializationPromise: Promise<void> | null = null;

export type UATCheckpoint = {
run_id: string;
checkpoint: string;
Expand All @@ -13,26 +18,33 @@ export type UATCheckpoint = {
};

async function ensureTable() {
const pool = getPostgresPool();
try {
await pool.query(`
CREATE TABLE IF NOT EXISTS maestro_uat_checkpoints (
id SERIAL PRIMARY KEY,
run_id TEXT NOT NULL,
checkpoint TEXT NOT NULL,
verdict TEXT NOT NULL,
evidence_uris JSONB,
actor TEXT,
created_at TIMESTAMPTZ DEFAULT NOW()
if (initializationPromise) return initializationPromise;

initializationPromise = (async () => {
const pool = getPostgresPool();
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 31, 2026

Choose a reason for hiding this comment

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

P1 Badge Reset init guard when pool acquisition fails

If getPostgresPool() throws during the first call (for example during transient DB startup/config failures), that rejection is captured in initializationPromise and never cleared, so every later ensureTable() call reuses the same rejected promise and the repository cannot recover without a process restart. Before this change, each call retried pool acquisition; with this guard, recovery is blocked unless the promise is reset on pool-init failure.

Useful? React with 👍 / 👎.

try {
Comment on lines +23 to +25
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

getPostgresPool() call outside try-catch blocks retries on pool errors.

If getPostgresPool() throws (e.g., pool not initialized), the promise rejects but initializationPromise isn't cleared, preventing retry attempts. Consider moving the pool acquisition inside the try block:

Proposed fix 
   initializationPromise = (async () => {
-    const pool = getPostgresPool();
     try {
+      const pool = getPostgresPool();
       await pool.query(`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
initializationPromise = (async () => {
const pool = getPostgresPool();
try {
initializationPromise = (async () => {
try {
const pool = getPostgresPool();
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@server/src/db/repositories/uat.ts` around lines 22 - 24, The
initializationPromise currently calls getPostgresPool() outside the try block,
so if getPostgresPool() throws the promise rejects but initializationPromise
remains set; move the getPostgresPool() invocation into the existing try block
inside the async IIFE (where the rest of the setup runs) and in the catch
handler clear or reset initializationPromise (e.g., set initializationPromise =
null) before rethrowing or returning, so subsequent calls can retry; adjust code
around initializationPromise and getPostgresPool() to ensure pool acquisition
and error handling occur inside try/catch.

await pool.query(`
CREATE TABLE IF NOT EXISTS maestro_uat_checkpoints (
id SERIAL PRIMARY KEY,
run_id TEXT NOT NULL,
checkpoint TEXT NOT NULL,
verdict TEXT NOT NULL,
evidence_uris JSONB,
actor TEXT,
created_at TIMESTAMPTZ DEFAULT NOW()
);
CREATE INDEX IF NOT EXISTS idx_uat_run ON maestro_uat_checkpoints(run_id);
`);
} catch (e: any) {
logger.warn(
{ err: e },
'ensureTable maestro_uat_checkpoints failed (mock mode?)',
);
CREATE INDEX IF NOT EXISTS idx_uat_run ON maestro_uat_checkpoints(run_id);
console.log(`);
} catch (e: any) {
logger.warn(
{ err: e },
'ensureTable maestro_uat_checkpoints failed (mock mode?)',
);
}
initializationPromise = null;
}
Comment on lines +38 to +44
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check how tickets.ts handles initialization errors to confirm if swallowing is intentional

# Find tickets.ts and examine its ensureTable or similar initialization pattern
fd -t f 'tickets.ts' --exec cat {} \; | rg -A 20 -B 5 'initializationPromise|ensureTable'

Repository: BrianCLong/summit

Length of output: 5487

🏁 Script executed:

cd server/src/db/repositories && cat -n uat.ts | head -50

Repository: BrianCLong/summit

Length of output: 1873

Re-throw the error to match the pattern in tickets.ts and prevent silent failures.

The catch block logs the error and clears initializationPromise for retry, but doesn't re-throw. This causes the promise to resolve successfully even when table creation fails, so callers like addUATCheckpoint() are unaware initialization failed.

The codebase already uses the correct pattern in tickets.ts, where ensureTable() re-throws after logging and resetting the promise. This allows callers to decide how to handle initialization failures (catch and proceed, or propagate). Update uat.ts to match:

Re-throw to propagate initialization failure 
    } catch (e: any) {
      logger.warn(
        { err: e },
        'ensureTable maestro_uat_checkpoints failed (mock mode?)',
      );
      initializationPromise = null;
+     throw e;
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (e: any) {
logger.warn(
{ err: e },
'ensureTable maestro_uat_checkpoints failed (mock mode?)',
);
CREATE INDEX IF NOT EXISTS idx_uat_run ON maestro_uat_checkpoints(run_id);
console.log(`);
} catch (e: any) {
logger.warn(
{ err: e },
'ensureTable maestro_uat_checkpoints failed (mock mode?)',
);
}
initializationPromise = null;
}
} catch (e: any) {
logger.warn(
{ err: e },
'ensureTable maestro_uat_checkpoints failed (mock mode?)',
);
initializationPromise = null;
throw e;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@server/src/db/repositories/uat.ts` around lines 37 - 43, The catch block in
ensureTable (uat.ts) currently logs the error and sets initializationPromise =
null but does not re-throw, causing callers like addUATCheckpoint to think
initialization succeeded; update the catch to follow the pattern used in
tickets.ts by re-throwing the caught error after logging and resetting
initializationPromise so the failure propagates to callers (reference
initializationPromise, ensureTable, addUATCheckpoint).

})();

return initializationPromise;
Comment on lines +43 to +47
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

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

medium

The current implementation of ensureTable has a subtle inconsistency in its return value when an error occurs. If the initialization fails, initializationPromise is set to null inside the catch block. However, the function then returns initializationPromise at line 46. Because pool.query is asynchronous, the first caller will have already received the Promise object before the catch block executes. But if the catch block finishes before line 46 is reached (which could happen if an error occurs synchronously before the first await), the function would return null.

Additionally, since the error is caught and swallowed, the promise resolves successfully. This means callers awaiting ensureTable() will proceed even if the table was not created, leading to subsequent query failures. It is better to ensure the function always returns the promise and consider whether callers should be notified of the failure.

    } catch (e: any) {
      logger.warn(
        { err: e },
        'ensureTable maestro_uat_checkpoints failed (mock mode?)',
      );
      initializationPromise = null;
    }
  })();

  return initializationPromise!;

}

export async function addUATCheckpoint(c: UATCheckpoint) {
Expand Down
Loading