fix(types): widen recordCommitLatency parameter so the build type-checks#1890
Closed
kriszyp wants to merge 1 commit into
Closed
fix(types): widen recordCommitLatency parameter so the build type-checks#1890kriszyp wants to merge 1 commit into
kriszyp wants to merge 1 commit into
Conversation
The recordCommitLatency() function only reads settlement timing and never touches the resolved value, so Promise<unknown> is the honest parameter type. This allows callers like commitResolution (which can be Promise<number | void> due to RETRY_NOW_VALUE on coordinated retry) to satisfy the parameter type without a cast. Fixes the TS2345 error that prevents `npm run build` from type-checking cleanly. This unblocks the adapter-workflow CI check, which treats the non-zero tsc exit as a failure even though tests run (noEmitOnError is off). Refs #1688 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
|
Reviewed; no blockers found. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request widens the parameter type of 'recordCommitLatency' in 'resources/DatabaseTransaction.ts' from 'Promise' to 'Promise' to accommodate callers resolving with other values during coordinated retries, and adds an explanatory comment. There are no review comments, and I have no feedback to provide.
kriszyp
marked this pull request as ready for review
July 21, 2026 22:23
kriszyp
added a commit
that referenced
this pull request
Jul 22, 2026
Format Check failed on the new record-scoped-write-auth DESIGN.md row (unformatted markdown table) — run prettier --write to fix. The Next.js adapter integration jobs (Node 20/22/24) failed on a pre-existing TS2345 in DatabaseTransaction.ts (commitResolution can be Promise<number | void> on coordinated retry, recordCommitLatency only accepted Promise<void>). Harper's own CI tolerates this with `npm run build || true`, but the adapter workflow does not. Apply the same fix already validated on #1890 (open, unmerged): widen the parameter to Promise<unknown> since the function only reads settlement timing and never touches the resolved value. Integration Tests 3/6 (uWS HTTP) fails on an unrelated pre-existing flake (Blob lifecycle drop_table "Invalid column family specified in write batch") — confirmed present on main as of 2026-07-20, unaffected by this branch's diff. Left as-is; expected to pass on rerun. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
kriszyp
added a commit
that referenced
this pull request
Jul 23, 2026
Format Check failed on the new record-scoped-write-auth DESIGN.md row (unformatted markdown table) — run prettier --write to fix. The Next.js adapter integration jobs (Node 20/22/24) failed on a pre-existing TS2345 in DatabaseTransaction.ts (commitResolution can be Promise<number | void> on coordinated retry, recordCommitLatency only accepted Promise<void>). Harper's own CI tolerates this with `npm run build || true`, but the adapter workflow does not. Apply the same fix already validated on #1890 (open, unmerged): widen the parameter to Promise<unknown> since the function only reads settlement timing and never touches the resolved value. Integration Tests 3/6 (uWS HTTP) fails on an unrelated pre-existing flake (Blob lifecycle drop_table "Invalid column family specified in write batch") — confirmed present on main as of 2026-07-20, unaffected by this branch's diff. Left as-is; expected to pass on rerun. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Widen the
recordCommitLatency()parameter type fromPromise<void>toPromise<unknown>to accommodate callers that resolve withRETRY_NOW_VALUE(a number) on coordinated retry.Why
The
recordCommitLatency()function only reads settlement timing via.then()and never touches the resolved value —Promise<unknown>is the honest parameter type. This eliminates the TS2345 type error that has preventednpm run buildfrom type-checking cleanly since commit-latency work in #1688.The error cascades to the Next.js adapter integration workflow: even though Harper's own CI tolerates it (
noEmitOnErroris off, so tests run), the adapter workflow builds harper and treats a non-zero tsc exit as a failure. This unblocks any PR flagged by the adapter-changes detector.Test plan
npx tsc --project tsconfig.build.json --noEmitexits 0 (no type errors)npm run buildcompletes successfullynpm run test:unit:resourcespasses: 1238 passing / 14 pending (baseline)Notes
Only
resources/DatabaseTransaction.tschanged: therecordCommitLatencyparameter type + explanatory comment. No cast removed, no declared type changed elsewhere — the existingas Promise<void>at the assignment survives for the surrounding retry logic.Refs #1688
🤖 Generated with Claude Code