fix(selfhost): add the ams_signals conflict key so Postgres AMS ingest stops throwing - #8398
Conversation
|
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 #8398 +/- ##
==========================================
- Coverage 92.12% 89.56% -2.57%
==========================================
Files 782 98 -684
Lines 78531 22663 -55868
Branches 23720 3872 -19848
==========================================
- Hits 72347 20298 -52049
+ Misses 5062 2187 -2875
+ Partials 1122 178 -944
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 11:31:02 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
ams_signalstoREPLACE_CONFLICT_KEYSinsrc/selfhost/pg-dialect.ts. Without it,translateInsertOrthrewpg_dialect: INSERT OR REPLACE into 'ams_signals' has no known conflict keyon every self-host Postgres deployment, so the first AMS telemetry-ingest write (src/ams/ingest.ts, reached viaPOST /v1/ams/ingest) failed the request outright.The key-shape evaluation the issue asked for — the 2-column key is correct here
Per the issue's second branch: the
orb_signalscollision reasoning does not apply toams_signals, so no migration is added and the entry matches the table's existing constraint.packages/loopover-miner/lib/orb-export.ts(DEFAULT_AMS_COLLECTOR_URL = ".../v1/ams/ingest"), which derives:repoHash = hmacAnonymize(repoFullName, secret)prHash = hmacAnonymize(${repoFullName}:${prNumber}, secret)pr_hashinput: within oneinstance_id(one secret), apr_hashalready identifies(repo, PR), andrepo_hashis functionally determined by it. Two different repos can never collide on(instance_id, pr_hash)— exactly the "pr_hash's hash input already disambiguates by repo" case the issue names.UNIQUE (instance_id, pr_hash),migrations/0148_ams_signals.sql), or Postgres rejects the generatedON CONFLICTwith "no unique or exclusion constraint matching the ON CONFLICT specification". A 3-column target would need a table recreate for zero uniqueness gain.pr_hashceasing to be repo-scoped at the producer), so it isn't silently re-litigated later.Test plan
test/unit/selfhost-pg-dialect.test.ts— new regression test translating the verbatim statementsrc/ams/ingest.tsissues: no throw,ON CONFLICT (instance_id, pr_hash) DO UPDATE SET, every non-key column upserted (repo_hash,decision,reason_bucket,closed_at,received_at), and both key columns excluded from the SET listsrc/selfhost/pg-dialect.ts: 41/41 lines, 14/14 branchesnpm run typecheckcleansrc/ams/ingest.tschange needed (see the evaluation above), sodb:migrations:check/db:schema-drift:checkare unaffectedCloses #8382