fix(review): compare inconclusive as text so Postgres doesn't 500 (#4997) - #5032
Merged
Conversation
) findHottestInconclusiveReviewTargetForRepo compared json_extract's result to a bare integer literal. SQLite's json_extract surfaces a JSON boolean as 1/0, but the self-host Postgres translation rewrites json_extract to `->>`, which always returns text -- comparing that text to an integer literal threw a type-mismatch error on every call, 396 times and escalating. CAST to text first so both backends compare text to text. This also explains why the review-burst, gate-precision, and slop calibration anomaly detectors went silent for every affected repo: all four run through one Promise.all in runOpsAlerts/computeOpsStats, so this one query throwing discarded the other three results too. The captured `vouchdev/vouch` repo tag is a real, independently registered self-host install (a third-party open-source project, not one of gittensory's own repos) -- confirms every self-host operator hit this, not just this install.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5032 +/- ##
=======================================
Coverage 94.36% 94.36%
=======================================
Files 458 458
Lines 39255 39255
Branches 14322 14322
=======================================
Hits 37042 37042
Misses 1558 1558
Partials 655 655
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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
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.
Summary
ops_anomaly_repo_error: Failed query...fired 396 times over 3 days, escalating, still firing at filing time.findHottestReviewTargetForRepopattern — traced both instead:findHottestInconclusiveReviewTargetForRepo(src/db/repositories.ts) comparesjson_extract(metadata_json, '$.inconclusive') = 1(a bare integer literal). SQLite'sjson_extractsurfaces a JSON boolean as SQL integer 1/0 (so this worked fine in tests/cloud), but the self-host Postgres translation (src/selfhost/pg-dialect.ts) rewritesjson_extract(...)to->>, which always returns text — comparing that text to a bare integer literal throws a Postgres type-mismatch error on every invocation. Confirmed via grep this is the onlyjson_extract(...) = <bare integer>comparison anywhere in the codebase; every other comparison already binds a string parameter.CAST(json_extract(...) AS TEXT) IN ('1', 'true')— text-to-text on both backends. SQLite:CAST(1 AS TEXT) = '1'(byte-identical to the old= 1semantics). Postgres:->>'inconclusive'already yields'true'/'false'.runOpsAlerts/computeOpsStatsrun all four per-repo anomaly checks through onePromise.all, so this one query throwing discardedgatePrecision/calibration/reviewBursttoo — this bug silently blinded every anomaly detector (not just the review-failure-burst one) for every repo with any AI review activity on self-host, every scan tick.vouchdev/vouchrepo tag is a real, independently-owned open-source project (verified via the GitHub API) —opsScanReposonly scans rows from this install's ownrepositoriestable, so this is a genuine third-party self-host operator's registered repo, not stale/test data or a query-bug artifact. Confirms the bug affects every self-host operator, not just this install.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #4997).Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocallynpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
test:coverage(full unsharded): not run end-to-end — ran scopedvitest --coveragefortest/unit/db-parsers.test.ts+test/unit/ops-wire.test.ts+test/unit/selfhost-pg-dialect.test.ts(94 tests) and confirmed via lcov that both branches of the changed function's null-guard are covered.actionlint/test:workers/build:mcp/test:mcp-pack/ui:openapi:check/ui:lint/ui:typecheck/ui:build/npm audit: not run — this change touches onlysrc/db/repositories.ts(a query-text change inside an existing Drizzlesqltemplate, no new API/schema/binding/dependency surface) and its tests; no workflow, MCP, UI, or dependency-manifest surface changed.Safety
UI Evidencesection. (N/A.)Notes
Part of a batch of 13 bug fixes filed from a Sentry-issue triage this session (#4994–#5006). This is #4 by priority.