Skip to content

feat(review): add suppression-signal data model + memory matcher - #3841

Merged
JSONbored merged 3 commits into
mainfrom
feat/review-memory-2178-2181
Jul 6, 2026
Merged

feat(review): add suppression-signal data model + memory matcher#3841
JSONbored merged 3 commits into
mainfrom
feat/review-memory-2178-2181

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds the review-memory feature (feat(review): suppression-signal data model + migration for review memory #2178-feat(review): apply review-memory suppression to review findings before surfacing (advisory only) #2181): a deterministic finding-fingerprint + suppression matcher so a maintainer-dismissed advisory finding (a false positive) stays suppressed on later re-reviews of the same repo, gated behind review.memory (operator env kill-switch AND per-repo manifest opt-in, both default off, advisory-only by construction -- only ever applied to commentGate.warnings, never blockers).
  • migrations/0114_review_suppression_memory.sql + src/db/schema.ts: new review_suppression table (repo, category, path glob, pattern hash, created-by, timestamps) with a per-repo row cap.
  • src/review/review-memory-match.ts: deterministic fingerprint (reviewMemoryFingerprint) + the suppress/demote/none matcher (matchSuppressions) -- exact hash match suppresses, same-category-different-hash demotes (kept but moved to the end so an existing max_findings cap truncates it first), no match passes through untouched.
  • src/review/review-memory-wire.ts: activation wiring (isReviewMemoryEnabled, shouldApplyReviewMemory) mirroring the existing impact-map-wire.ts pattern, plus the pure applyReviewMemorySuppression apply-to-findings function.
  • src/queue/processors.ts: wires the suppression read + apply into the unified-comment render path, fail-safe (a store-read error leaves the original gate untouched rather than throwing) and zero-cost when the flag is off (no extra D1 read).
  • src/signals/focus-manifest.ts + .gittensory.yml.example / config/examples/gittensory.full.yml: new review.memory config-as-code knob, documented in both required locations.
  • Bug found and fixed during review: pruneReviewSuppressionsOverCap's per-repo cap-eviction query used a bare SQL OFFSET with no preceding LIMIT, which is a syntax error on this driver -- eviction silently failed on every single insert (swallowed by its own best-effort .catch). The cap only appeared to hold in the original test because listReviewSuppressions independently clamps its own read-side limit parameter to the same constant, masking the fact that the underlying table was never actually trimmed. Fixed by fetching the bounded row set and slicing the overflow off in JS instead of relying on SQL-level OFFSET (Drizzle's D1 dialect silently drops a .limit(-1) "unbounded" hint from the emitted SQL, so that workaround doesn't work either). Strengthened the existing cap-enforcement test to assert the raw table row count directly, so this can't regress silently again.

Scope

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally -- 100% branch coverage on every changed line in this diff (verified via targeted lcov branch inspection against the exact diff ranges).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A -- no auth/session/CORS surface touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A -- no public API/MCP surface change.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A -- no UI change.)
  • Visible UI changes include a UI Evidence section. (N/A -- backend-only change, no UI.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

JSONbored added 2 commits July 6, 2026 07:10
#2181)

Adds the review-memory feature: a deterministic finding-fingerprint + suppression
matcher that lets a maintainer-dismissed false positive stay suppressed on later
re-reviews of the same repo, gated behind review.memory (default off, advisory only).
… gaps

pruneReviewSuppressionsOverCap's OFFSET-only query is a syntax error on this
driver (a bare OFFSET with no preceding LIMIT), so eviction silently failed on
every insert -- the per-repo cap only appeared to hold because
listReviewSuppressions clamps its own read-side limit to the same constant.
Switches to an in-JS slice over the bounded row set instead of relying on
SQL-level OFFSET, and strengthens the existing cap test to assert the
underlying row count directly rather than through the limit-clamped read path.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 9df0b54 Commit Preview URL

Branch Preview URL
Jul 06 2026, 02:31 PM

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.42%. Comparing base (ca6a6d7) to head (6d514a6).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3841      +/-   ##
==========================================
+ Coverage   93.41%   93.42%   +0.01%     
==========================================
  Files         326      328       +2     
  Lines       32855    32934      +79     
  Branches    12035    12053      +18     
==========================================
+ Hits        30691    30770      +79     
  Misses       1530     1530              
  Partials      634      634              
Files with missing lines Coverage Δ
src/db/repositories.ts 96.58% <100.00%> (+0.04%) ⬆️
src/db/schema.ts 71.18% <100.00%> (+0.49%) ⬆️
src/queue/processors.ts 94.48% <100.00%> (+0.02%) ⬆️
src/review/review-memory-match.ts 100.00% <100.00%> (ø)
src/review/review-memory-wire.ts 100.00% <100.00%> (ø)
src/signals/focus-manifest.ts 99.12% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 6, 2026
@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-06 14:50:31 UTC

21 files · 1 AI reviewer · no blockers · readiness 100/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): wrangler.jsonc (matched wrangler.jsonc), wrangler.jsonc (matched **/wrangler.jsonc).

Review summary
This PR adds the review-memory suppression data model end-to-end: schema + migration (0114) for `review_suppression`, repository accessors (`recordReviewSuppression`/`listReviewSuppressions`/prune-over-cap), a pure fingerprint+matcher module, wiring into the unified-comment render path in `processors.ts`, and full config-as-code plumbing (`review.memory` in focus-manifest, both yml examples, wrangler.jsonc, env.d.ts, docs). The gate is correctly double-locked (operator env AND per-repo manifest) and scoped exclusively to `commentGate.warnings` (never blockers), with a fail-safe try/catch around the store read so a D1 error leaves findings untouched. The previously-buggy `pruneReviewSuppressionsOverCap` (bare `OFFSET` with no `LIMIT`) is fixed by fetching the repo's rows and slicing the overflow in JS instead of relying on a driver-unsupported SQL construct. One real gap: nothing in this diff (or the visible production code) actually calls `recordReviewSuppression` outside of tests — the write/recording trigger appears to be a separate future slice, so the store stays empty in production until that ships, which is consistent with the PR's stated scope but worth flagging.

Nits — 6 non-blocking
  • The `gittensory_review_memory_suppressed_total` metric (`src/queue/processors.ts` near the review-memory apply block) increments whenever `suppressedCount > 0 || demotedCount > 0`, so a demote-only pass (nothing actually suppressed) still bumps a counter named 'suppressed' — consider splitting into separate suppressed/demoted counters.
  • `recordReviewSuppression` (`src/db/repositories.ts`) has no production caller anywhere in this diff — only `test/unit/queue.test.ts` and `test/unit/review-memory-store.test.ts` invoke it directly, so the suppression table can never actually be populated until a follow-up PR wires an actual maintainer-dismissal trigger; worth calling out explicitly in the issue thread so it isn't mistaken for a complete feature.
  • `djb2Hex` (`src/review/review-memory-match.ts:54`) is a 32-bit non-cryptographic hash; at the per-repo cap of 500 stored signals collision probability is low but non-trivial — the code comment argues this is acceptable since the module is advisory-only, which is a reasonable call but should be an explicit, acknowledged tradeoff rather than implicit.
  • `test/unit/queue.test.ts`'s 'FLAG-ON: suppresses...' test manually reconstructs `buildQualityGateWarning`'s title+detail string template to compute the expected patternHash — this couples the test to another module's exact wording and will silently stop testing the real path (rather than failing loudly) if that template changes without a corresponding update here.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2178, #2179, #2180, #2181
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (4 linked issues).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 54 registered-repo PR(s), 46 merged, 399 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 54 PR(s), 399 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 54 PR(s), 399 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 6, 2026
@JSONbored
JSONbored merged commit 38e40b2 into main Jul 6, 2026
12 checks passed
@JSONbored
JSONbored deleted the feat/review-memory-2178-2181 branch July 6, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Projects

None yet

1 participant