fix(storage): honest Promise<number | void> type for commit-latency recorder

[kriszyp]

Summary

Fixes the org-wide red main build (tsc TS2345) introduced by #1688.

resources/DatabaseTransaction.ts:367 passed commitResolution to recordCommitLatency(_: Promise<void>, ...), but commitResolution is declared Promise<number | void> | void. The as Promise<void> cast on the transaction.commit() assignment (line 360) was ineffective: TypeScript's assignment narrowing filters the declared union and re-widens to the Promise<number | void> member, so the cast never took effect and the argument stayed Promise<number | void>.

Fix

commit() honestly resolves to number | void — the coordinated-retry RETRY_NOW_VALUE sentinel, which is checked in the resolve callback (line 397). So:

• Widen recordCommitLatency's parameter to Promise<number | void> (the honest type).
• Drop the misleading/ineffective as Promise<void> cast and correct the adjacent comment.

recordCommitLatency only records the submit→settle timing and ignores the resolved value, so the per-attempt latency metric is unchanged — the transient-conflict retry still rejects and re-issues commit(), re-arming per attempt.

Where to look

• resources/DatabaseTransaction.ts:45 — widened recorder parameter.
• resources/DatabaseTransaction.ts:360 — cast removed; comment corrected.

Tests

• npm run build (tsc) — clean.
• unitTests/resources/transactionCommitLatency.test.js — 2 passing.
• npm run test:unit:resources — 1162 passing, 14 pending.

Refs #1688.

Generated by an LLM (Claude Opus 4.8).

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the type of commitResolution in DatabaseTransaction.ts from Promise<void> to Promise<number | void> to correctly match the return type of transaction.commit(), which can resolve to a coordinated-retry sentinel number. It also removes an unnecessary type cast and updates the relevant comments to document this behavior. There are no review comments to address, so no further feedback is provided.

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Note: three open PRs fix this same recordCommitLatency type error — only one should land.

#1853 (this), #1887, and #1869 all edit these same lines in DatabaseTransaction.ts, so whichever merges first leaves the other two with a conflict. Of the three, this is the cleanest fix: it makes the parameter type honest (Promise<number | void>, matching commit()'s actual result), drops the ineffective as Promise<void> cast, and corrects the comment.

• Return complete starts_with results for astral Unicode values #1887 re-adds an as Promise<void> cast at the call site (effective this time, but it keeps the misleading Promise<void> parameter and the cast), bundled into an unrelated astral-Unicode fix.
• fix: thread {transaction} into removeEntry for audit:false table deletes #1869 widens the parameter to Promise<unknown> — honest but less precise, dropping the number | void information — also bundled into an unrelated audit:false delete fix.

Suggest landing this dedicated fix and dropping the type change from the other two.

—
Generated by Barber AI

[kriszyp]

Yeah ,we are really going to make sure we land this fix!

[cb1kenobi]

Update: the duplicate-fix note above is stale.

Since that comment, #1890 (fix(types): widen recordCommitLatency parameter so the build type-checks) closed and #1899 (Fix recordCommitLatency parameter type for the widened commitResolution union) merged — landing the same TS2345 fix with the looser Promise<unknown> | void type. Those were the actual competing PRs, not #1887/#1869 as previously stated (#1869 no longer touches this file; #1887's edit here is unrelated, further down in the same method).

#1853 is now a clean refinement on top of #1899's merged fix: narrows the parameter to the more precise Promise<number | void> (matching commit()'s actual resolved type, including the RETRY_NOW_VALUE sentinel) and updates the adjacent comments. mergeable: true against current main — no conflict.

—
Generated by Barber AI