feat(filterengine): opt-in cluster-based noise filtering preset (#2235) by gchahcg · Pull Request #2628 · vavallee/bindery

gchahcg · 2026-09-15T11:43:36Z

Summary

Stacked on #2626 — this PR is currently based on main but its first three commits are #2626's; once that merges, GitHub will automatically shrink this diff down to just the commits below. Review order: skip straight to the commits titled feat(filterengine): ship ClusterEditionCountSignal..., docs: changelog and user guide..., and fix(filterengine): aggressive cluster preset defanged....

Adds ClusterEditionCountSignal, #2626's first real graded (non-veto) signal: it cross-checks each candidate work against how well-attested it is across every provider record sharing its title (clustered via #2626's already-shipped filterengine.BuildClusters), and can independently push a candidate toward KEEP or EXCLUDE based on that evidence. Gated behind a new metadata-profile field, ClusterFilterPreset — a closed set of server-tuned tiers (off/conservative/balanced/aggressive), never raw threshold numbers a client could hand-pick. off (the default for every existing profile) is provably byte-identical to current behavior.

Closes the rest of #2235.

Implementation notes

internal/api/authors.go's fetchAuthorBooks now builds clusters once per sync (gated behind a non-off preset, so the default path pays nothing extra) and attaches each candidate's Cluster alongside the existing structural/edition signal pass — so a well-attested cluster's keep-direction observation can offset a structure.partBookTitle veto in the same Decide call.
A non-off preset supplies its own shared threshold (applied to both KeepThreshold and ExcludeThreshold, never just one — see ThresholdForPreset's doc for why keeping the two equal matters: it's what keeps the REVIEW band permanently unreachable, exactly as it is under the v1 veto-only default). This is a separate, additive knob from KeepThreshold/ExcludeThreshold themselves, which stay locked at exactly 0/0 by feat(filterengine): scored-signal catalogue filtering engine (#2235) #2626's unchanged validateScoreThresholds.
New metadata profile settings-form control (web/src/pages/settings/MetadataTab.tsx) — a preset selector, never raw scoring numbers.

Validation

Tuning validated offline against fiction-author-dataset's 1,422 hand-verified rows across 20 authors (CC-BY-4.0). Isolating the one known, filtering-unsolvable pen-name misattribution in that dataset (Nora Roberts/J.D. Robb — OpenLibrary attributes all of J.D. Robb's "In Death" books to the Nora Roberts identity, so no per-candidate signal can separate them), the balanced preset scores recall=90.9%/precision=83.7% on the other 19 authors, versus a lower baseline with the signal off.

An independent review pass caught a real bug before this shipped: aggressive's original threshold (-1000) coincided exactly with the engine's veto weight, which — because BandFor uses a strict < — silently defanged every v1 signal whenever exactly one of them fired, not just this signal's own exclude branch. Fixed (now -950, with a genuinely looser keep gate to still earn the name), re-measured honestly, and pinned with a regression test (TestThresholdForPreset_NeverDefangsALoneVeto) that I verified fails against the old value before confirming it passes against the fix. Full writeup in that commit's message.

Why minimal / scope decisions

Three other spec-derived signals (non-Latin-script title, thin-catalog-by-subject-count, lexical-foreign-language) were also built and grid-searched during tuning; they measured zero effect on this dataset and one has an identified misfire risk on romanized non-English titles. Left out of this PR — unvalidated, not worth shipping speculatively.
No raw threshold UI. A profile can only pick one of the four named presets; the API rejects anything else.

Checklist

Tests added or updated
Doc-update gate cleared (changelog.d/2235-cluster-filter-preset.md, docs/User-Guide-Wiki.md)
Wiki pages updated — new "Cluster-based noise filtering" section

Test plan

go test ./cmd/... ./internal/...
go vet ./...
golangci-lint run --timeout=5m (0 issues)
govulncheck ./... (0 reachable vulnerabilities)
cd web && npm run typecheck && npm run lint (0 errors)
cd web && npm test (AuthorSyncNotice, MetadataTab)
Independent code-review pass on the Phase 2 diff specifically (separate from feat(filterengine): scored-signal catalogue filtering engine (#2235) #2626's review) — the aggressive-preset defanging bug above was its finding.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky

codecov · 2026-09-15T11:48:39Z

Codecov Report

❌ Patch coverage is 99.09091% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/api/authors.go	96.03%	4 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

…ee#2235) New package internal/metadata/filterengine: Signal/Observe/Candidate/Context/ Result types, BandFor (KEEP/REVIEW/EXCLUDE banding, half-open on the exclude side so keep==exclude deterministically collapses REVIEW to empty), Registry with duplicate-ID detection, and the 7 v1 signals porting authors.go's existing boolean filters (media type, junk title, language, part-book, missing date, missing ISBN, min pages) — each wrapping its existing predicate rather than reimplementing it. isPartBookTitle/anyEditionHasISBN/passesMinPagesFilter's actual logic moves into filterengine as exported functions (IsPartBookTitle/AnyEditionHasISBN/ PassesMinPagesFilter); authors.go's identically-named functions become thin wrappers in a later commit so catalogue_reconciliation.go's existing call sites are unaffected. Also adds models.FilterObservation (Signal/Weight/Confidence/Reason, Contribution() and Direction() derived rather than stored) and Book.Observations, both transient like Book.ProviderISBNs. TestRegistryIsVetoOnlyAtV1 pins the parity invariant that lets a future migration's keep_threshold=exclude_threshold=0 default reproduce today's boolean-chain behavior exactly: every DefaultSignals() entry must stay veto-weight until that default is deliberately revisited. Cluster/ClusterKey/BuildClusters are also included as explicit groundwork for the dataset's highest-value finding (cluster-level max(edition_count) discriminates real catalogue from noise; per-record does not — see cluster.go for the measurement) — no v1 signal consumes it yet; it's the prerequisite for the first graded signal, not speculative infrastructure. 97% coverage, 0 lint issues on the new package. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…avallee#2235) Migration 086 adds both columns, REAL NOT NULL DEFAULT 0 — the value that reproduces internal/metadata/filterengine's pre-vavallee#2235 boolean-chain behavior exactly for every existing profile (see filterengine's package doc and TestRegistryIsVetoOnlyAtV1). internal/api/metadata_profiles.go rejects exclude_threshold > keep_threshold outright (an inverted band is meaningless), and at v1 rejects exclude != keep entirely: no v1 signal is graded enough to populate a REVIEW band meaningfully, and there's no UI surface for one. Relaxing to `exclude <= keep` is the one-line change that enables graded filtering later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…llee#2235) doc.EditionCount was decoded from the /search.json response in both searchAuthorWorks and GetAuthorWorksSnapshot's merge step, but never actually assigned to the resulting models.Book: - searchAuthorWorks's own Book literal never referenced doc.EditionCount. - GetAuthorWorksSnapshot's primary+enrichment merge selectively copies specific fields from the enrichment (search) result onto the primary (works-endpoint) result when a work appears in both — the common case for an established author — and EditionCount wasn't one of the copied fields, so fixing searchAuthorWorks alone wasn't enough for works present in both sources. Two behavior changes as a result, both expected and correct, not accidental: - internal/metadata/aggregator_canonical.go's four EditionCount-based canonical-search ranking branches, previously always-true dead code since EditionCount was always 0, now actually discriminate. - internal/metadata/filterengine's Cluster.MaxEditionCount (the prerequisite for vavallee#2235's highest-value future signal, per that package's cluster.go doc) now has real data to aggregate instead of always reading 0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

vavallee#2235) Replaces the inline boolean checks (strict media type, junk title, language, part-book, missing date, missing ISBN, min pages) in AuthorHandler.fetchAuthorBooks with filterengine.Decide calls against the corresponding ported signal, one per existing call site — preserving every existing gating condition (singleWork exemptions, existing==nil gating on the edition-dependent filters, the editionsByForeignID comma-ok check) and every existing Skipped* counter/sample exactly as before. isPartBookTitle/anyEditionHasISBN/passesMinPagesFilter now delegate to their relocated filterengine equivalents rather than duplicating the logic, so catalogue_reconciliation.go's pre-existing call sites automatically share the same implementation the new signals wrap. TestAuthorSyncSummaryReconciles (a pre-existing test exercising every filter against a catalogue built to trip each one, asserting exact expected counts) passed unmodified against this change — strong evidence the wiring didn't shift behavior. Added two more parity tests: - TestAuthorSyncParity_ShippedDefaultReproducesBooleanChain: an independent smaller-fixture confirmation at the shipped 0/0 default. - TestAuthorSyncParity_NonZeroEqualThresholdsDiverge: documents a REAL bug caught while building this test suite. keep_threshold == exclude_threshold alone does not reproduce the boolean chain — only exactly 0/0 does, because every v1 signal is a veto with Context.Prior hardcoded to 0, so a clean candidate always scores exactly 0. A nonzero equal pair (e.g. both 50) excludes every candidate, filtered or not, since 0 < 50. validateScoreThresholds is tightened accordingly: it now rejects any nonzero value, not merely an unequal pair — the original (equal-only) version of that check would have accepted the broken 50/50 profile this test constructs directly through the repo (bypassing the now-corrected API validation) to demonstrate the failure mode the tightened check exists to prevent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…them (vavallee#2235) shouldFilterOLNoise's two call sites (GetAuthorWorksSnapshot's primary loop and its enrichment-only tail) used to drop a matched work outright, before it ever reached AuthorHandler.fetchAuthorBooks — and therefore before it ever entered AuthorSyncSummary.Total's accounting at all. That's exactly the failure mode vavallee#2235 exists to fix: a real drop with no visibility, no counter, no way to know it happened short of reading provider logs. olNoiseMatchReason (new) returns the matched pattern instead of a bare bool; both call sites now append a models.FilterObservation (Signal: models.SignalProviderOpenLibraryNoise) to the surviving book instead of skipping it, checked against the primary entry's untruncated Subjects list where available (the enrichment-tail call site was already checking the 10-item-truncated Genres before this change, so no fidelity change there). internal/metadata/filterengine gets a new ProviderNoiseSignal that replays that observation at the registry's configured (veto) weight — the seam that lets a provider flag something without needing profile/threshold context of its own. Wired into authors.go's junk-title check site (both route to the same SkippedJunk counter; there was never a separate counter for the provider-side check). Behavior change, expected and documented, not a bug: an OpenLibrary-primary author's AuthorSyncSummary.Total can now be visibly higher than before this PR, for exactly the works that used to be silently eaten pre-Total. Existing TestGetAuthorWorks_HTTP_NoiseFilter{,SearchEnrichment} rewritten to assert the new flag-not-drop behavior (all works present, noise-matched ones carrying the observation) rather than the old drop-and-count behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…d truth (vavallee#2235) testdata/works_sample.csv: an 80-row stratified sample of gchahcg/fiction-author-dataset's data/works.csv (commit 1b1751b, CC-BY-4.0), attributed in testdata/README.md, which also documents what this dataset can and can't validate here — it carries title/work_kind ground truth but no language/subjects/edition_count/ISBN/page-count/release-date, so it can only exercise JunkTitleSignal and PartBookSignal, not the other four. TestWorksSample_CoreTitlesNeverFalsePositive pins 0 false positives from either title-only signal against real books — measured against the FULL 1,422-row dataset this holds at 804/804 (100%). TestWorksSample_PartBookSignalOnlyCatchesTitleShapedBundles documents (doesn't demand 100% on) PartBookSignal's real recall on compilation/posthumous-compilation rows — 53/284 (18.7%) on the full dataset — as the concrete evidence for why the cluster-level edition-count signal cluster.go's doc describes is worth building next, not a regression to fix now. TestLanguageSignal_EmptyLanguageIsNotForeign is the named case from vavallee#2235 itself ("Summer Stars: Second Nature / One Summer", dropped for language:""), as a synthetic fixture since it isn't in the ground-truth dataset (it came from a live sync run, not curated data). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

- changelog.d/2235-scored-filter-engine.md: ### Added for the new engine, ### Changed for the AuthorSyncSummary.Total semantics shift (step 8). - docs/ARCHITECTURE.md: one row for internal/metadata/filterengine in the internal-packages table. - docs/User-Guide-Wiki.md: "minimum popularity" was already stale before this PR — MinPopularity has been unenforced since an earlier refactor (web/src/pages/settings/MetadataTab.tsx says so explicitly) — replaced with the profile fields that are actually enforced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

… stages (vavallee#2235) Moving OpenLibrary's companion-material check from "drop inside the provider client" to "flag and let filterengine decide" is what makes those works countable in AuthorSyncSummary.Total. It also, unintentionally, put them into every stage that runs over the RAW provider slice BEFORE the create loop — stages that pre-vavallee#2235 never saw them, because the client had already removed them. Three sites, all restoring the pre-vavallee#2235 result exactly: - isAuthorWorkMonitorCandidate: latestBookMonitorKeys awards the author's MonitorLatestCount slots from the raw slice, by release date, before any filtering. Companion material is usually published well after the work it accompanies, so a flagged study guide won the auction, was then excluded by ProviderNoiseSignal in the loop, and the real book it displaced was created UNMONITORED — no auto-search, no grab, and no counter saying why. This was reproduced end to end, not theorised; the regression test is the reproducer. - applyAuthorMajorityLanguageFallback: flagged works no longer vote in the author's majority-language inference. A run of English-language companion material could otherwise carry both the minimum-sample gate and the dominance ratio for an author whose real catalogue is another language, changing which real works get a language assigned and therefore which ones the language filter drops. - reconciliationRejectReason: the destructive catalogue-reconciliation path reads the same snapshot. With the client no longer dropping these works, reconciliation had silently started treating a study guide as evidence that the work it is a guide TO is still in the upstream catalogue. Routed to the catalogue_filter bucket, matching how fetchAuthorBooks attributes the same signal. models.FindObservation/HasObservation is the one lookup all four consumers (these three plus ProviderNoiseSignal, which needs the matched Reason) share, so the provider-claim probe exists once rather than four times. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…allee#2235) Five small, safe fixes, no scope expansion: 1. Consolidate the "title is empty or matches the author's own name" check, which JunkTitleSignal reimplemented inline while isAuthorWorkMonitorCandidate and reconciliationRejectReason each had their own independent copy — three copies of the same logic despite this PR's own wrap-don't-reimplement rule. New filterengine.IsJunkTitle is the one shared implementation; all three call sites now use it. 2. TestRegistryIsVetoOnlyAtV1 only checked each signal's Weight() accessor, not the Weight/Confidence on the observation it actually emits — a signal could report a veto-looking Weight() while Observe emits something else entirely, decoupled from the accessor. Added vetoFixtures(), a per-signal Candidate/Context guaranteed to trigger it, and the test now runs every DefaultSignals() member through fires() (the same assertion the per-signal tests already use) — a signal with no fixture entry fails explicitly rather than silently skipping the check. 3. Direct assertion on SkippedMediaType in TestFetchAuthorBooks_StrictMediaType — it previously only checked which books got created, not that the drop was specifically counted as a media-type skip. 4. Renamed TestAuthorSyncParity_ShippedDefaultReproducesBooleanChain to TestAuthorSyncParity_DefaultVetoWeightsMatchExpectedCounts and corrected its doc comment: it's a 3-work hardcoded-expectation fixture, not the golden comparison against the pre-vavallee#2235 chain (that's TestAuthorSyncSummaryReconciles, which passed unmodified). 5. Two stale "exclude != keep" error messages in metadata_profiles_test.go updated to state the actual tightened rule (both must be exactly 0, not merely equal to each other). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…all-site (vavallee#2235) fetchAuthorBooks called filterengine.Decide separately at 7 call sites across 8 signals, each banding and `continue`-ing immediately on EXCLUDE. A candidate tripping more than one filter was therefore only ever attributed to whichever call site the loop reached first — the package doc's claim that "a record tripping more than one filter now carries every reason it failed" was true of a single Decide call's signal list, never true across the free/junk/language/structural checks as a whole. Worse, every call site hand-built its own filterengine.New*Signal() list, so TestRegistryIsVetoOnlyAtV1 (which checks filterengine.DefaultSignals()) was guarding a construction production never actually called. Fixes both: - internal/api/authors_filterengine.go's filterEngineSignals() sources the free/structure/edition signal lists fetchAuthorBooks evaluates directly from filterengine.DefaultSignals(), split by ID into three buckets (TestFilterEngineSignals_CoversEveryDefaultSignalExactlyOnce pins the split is exhaustive). A signal the registry ships is now a signal production evaluates, full stop — proven concretely, not asserted in prose: temporarily injecting a fake graded signal into DefaultSignals() and re-running TestRegistryIsVetoOnlyAtV1, TestEverySignalHasACounter, and TestFilterEngineSignals_CoversEveryDefaultSignalExactlyOnce showed all three catch it (captured in this session, reverted before commit). - authors.go's two evaluation passes (free signals over every candidate; structural + edition-gated signals over candidates with no existing library row) now call filterengine.Decide exactly once per candidate per pass, accumulating every applicable signal's observations into one Result before checking Band — never banding after a single signal fires. The structural-pass call recombines the free signals too (cheap, pure, already proven non-excluding for anything reaching that loop), so the Result handed to banding is a literal one-candidate, all-applicable- signals ledger, not an assumption that an earlier pass contributed nothing. - authorSyncCounters + signalCounter (authors_filterengine.go) replace the dozen separate Skipped* local variables with one struct and one ID-keyed attribution map, so an EXCLUDE-banded candidate's Skipped* counter and sample come from strongestObservation(result.Observations) — the ledger's largest-magnitude observation, ties broken by registry order (matching the pre-vavallee#2235 boolean chain's check order exactly) — instead of whichever Decide call used to fire and continue first. TestEverySignalHasACounter pins every DefaultSignals() entry has a signalCounter entry. - TestEvaluateCandidate_AccumulatesEveryFiringSignal is the white-box proof multiple observations really do accumulate: a candidate tripping both JunkTitleSignal and LanguageSignal in one Decide call is asserted to carry both observations, not just the first — a black-box counter check alone can't distinguish this from the old first-checked-wins behavior, since equal-magnitude vetoes with registry-order tie-break produce the same counter either way. TestAuthorSyncParity_MultiSignalCandidateCountsOnceAttributedToStrongest is the same proof at the full sync-loop level: two multi-trip fixtures (junk+language in the free pass, part-book+missing-date in the structural pass) each land under exactly one counter, and AuthorSyncSummary.Unaccounted() stays zero. Also: - models.AuthorSyncSkippedBook gains Reason (json:"reason,omitempty", purely additive), populated from the same strongestObservation so a skip sample names the specific evidence, not just the title/language. - internal/metadata/filterengine/cluster.go's ClusterKey now calls the newly-exported metadata.StripLeadingArticle instead of maintaining an independent copy of the same regex — filterengine/predicates.go already imports internal/metadata (metadata.IsBundleTitle), so the import- direction argument for a second copy no longer held. - models.Book.EditionCount gets the godoc it was missing: populated by openlibrary's client, read by aggregator_canonical.go's ranking and filterengine/cluster.go's MaxEditionCount (unused by any v1 signal). - docs/third-party-data.md gets the CC-BY-4.0 attribution entry for testdata/works_sample.csv (fiction-author-dataset), alongside the existing per-file notice in testdata/README.md. Verification: go build ./..., go vet ./..., gofmt -l . (clean after formatting book.go), golangci-lint run ./... (0 issues), govulncheck ./... (0 vulnerabilities affecting this code), go test ./cmd/... ./internal/... (every package green, internal/api 142s). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…itle matching vavallee#2235 changed OpenLibrary's client from dropping companion material (study guides, film tie-ins, etc.) to flagging it with models.SignalProviderOpenLibraryNoise, so fetchAuthorBooks's ProviderNoiseSignal can count and report it instead of it vanishing invisibly. internal/abs/import_upserts.go's lookupUpstreamBook never routes through filterengine, though — it title-matches raw GetAuthorWorks results directly — so a flagged companion work could now win an ABS import's title match, or turn a previously-unambiguous match ambiguous, a regression an independent review pass caught before this branch was opened as a PR. Also fixes a related, currently-unreachable BuildClusters bug the same review pass found: a cluster's first-seen member having an empty ForeignID let a later, lower-EditionCount member silently re-seed CanonicalID outside the normal comparison (MaxEditionCount itself was never affected). Not reachable via OpenLibrary today, but BuildClusters is exported and provider-agnostic, and is exactly the groundwork the in-flight Phase 2 cluster-signal work builds on. And updates GetAuthorWorks's doc comment, which still claimed noise was filtered at that layer after the flag-not-drop change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

internal/models/filter_observation.go was entirely 0%-covered despite its functions being exercised indirectly through internal/metadata/filterengine's tests — go test's default per-package coverage mode doesn't attribute cross-package execution to the file where the code lives, so it needed its own direct test file regardless of the indirect exercise. Adds one. Closes the remaining flagged gaps directly: Registry.Decide (untested one-line wrapper), the nil-Book guard on LanguageSignal/ProviderNoiseSignal, AnyEditionHasISBN's ISBN10-only branch (only ISBN13 was ever exercised before), and strongestObservation's strictly-larger-magnitude branch plus recordExcluded's panic-on-unmapped-signal path (the guard TestEverySignalHasACounter exists to make unreachable in practice, but the panic itself was never actually triggered by a test). Left one flagged gap alone: internal/api/authors.go's UNIQUE/FOREIGN KEY constraint race-recovery branches (~line 2547-2573) predate this PR (confirmed via git log against main) — Codecov's diff view only flags them because this PR's counter refactor touched the surrounding lines, not because they're new logic. Simulating that exact race deterministically would need either a real concurrent goroutine racing the insert (flaky) or an injectable repo interface neither authors.go nor its tests have today — not worth adding for a genuinely pre-existing, untested-before-this-PR path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

Add unit tests for the uncovered branches Codecov flagged on vavallee#2626: - openlibrary.GetAuthorWorksSnapshot: primary-cover ImageURL, empty-title skip, and search-enrichment ratings merge (client.go:417/455/470). - api.fetchAuthorBooks: calibre relink failure (1830), works-fetch error (1861), MinPages edition-lookup failure (2253), and series-mode monitored-series load + monitor-on-discovery (2055-2072, 2356-2362). - api.fetchAuthorBooksAsync: nil-author guard (699). The Audible supplement branch (1888-1891) is left untested: it issues a live call to api.audible.com and the aggregator's audible client cannot be injected from the api test package. Coverage: GetAuthorWorksSnapshot 93.1% -> 100%, fetchAuthorBooks 77.0% -> 85.9%, fetchAuthorBooksAsync 88.9% -> 100%.

…er filter preset (vavallee#2235 Phase 2) Adds ClusterFilterPreset (migration 087) to metadata profiles: a closed set of server-tuned ClusterEditionCountSignal configurations (off/conservative/ balanced/aggressive), never raw threshold numbers a client could hand-pick. Wires filterengine.BuildClusters into fetchAuthorBooks's real sync loop for the first time and attaches each candidate's Cluster alongside the existing structural/edition signal pass, so a well-attested cluster's keep-direction observation can offset a structure.partBookTitle veto in one combined Decide call. "off" (the default for every existing profile) is provably byte-identical to current behavior: BuildClusters is skipped entirely, ClusterSignalForPreset returns nil, and the profile's own KeepThreshold/ExcludeThreshold columns (migration 086) stay locked at exactly 0/0 by the unchanged validateScoreThresholds. A non-off preset instead supplies its own shared threshold (applied to both keep and exclude, keeping BandFor's REVIEW band permanently unreachable — see ThresholdForPreset's doc) entirely independent of those two columns. Tuning validated offline against fiction-author-dataset's 1,422 hand-verified rows across 20 authors (CC-BY-4.0): isolating the one known, filtering-unsolvable pen-name misattribution in that dataset (Nora Roberts/ J.D. Robb — 471 of 1,422 rows), the "balanced" preset scores recall=90.9%/precision=83.7% on the other 19 authors, versus a lower baseline with the signal off. Adds two new fetchAuthorBooks regression/behavior tests: "off" reproduces pre-existing behavior exactly even with a generous EditionCount present, and "balanced" rescues a part-book-vetoed candidate whose cluster is well-attested — pinning the actual mechanism, not just the offline dataset numbers. New SkippedThinCluster counter/sample, surfaced through AuthorSyncNotice, and a new "Cluster-based noise filtering" preset selector in the metadata profile settings form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…e#2235 Phase 2) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…al at its exact threshold An independent review pass caught a serious bug before this branch was opened as a PR: ClusterFilterAggressive's threshold was -1000, exactly equal to vetoWeight. BandFor's strict "<" bands a lone veto's score of -1000 as KEEP, not EXCLUDE, at that exact value — so any profile on the aggressive preset would have silently stopped enforcing every v1 signal (media type, junk title, language, part book, missing date, missing ISBN, min pages) whenever exactly one of them fired, not just this signal's own exclude branch. The previously-cited "recall=92.4%" number was measuring that defanged behavior, not real rescuing. Re-measured after the fix (threshold=-950, keepWeight=700, keepMinEditionCount=1 — a genuinely looser keep gate to actually earn the "aggressive" name once the defanging inflation is gone): recall=97.0%, precision=83.8% on the same 19-author isolation, a real improvement over "balanced" at flat precision. Adds TestThresholdForPreset_NeverDefangsALoneVeto, which checks BandFor directly with each shipped preset's threshold (not just ThresholdForPreset's return value in isolation, since the bug is in the interaction with BandFor) — verified this test fails against the old -1000 value before confirming it passes against the fix. Also fixes a migration comment citing a test name that doesn't exist (TestAuthorSyncParity_ClusterFilterOffMatchesPreThresholds -> the actual TestFetchAuthorBooks_ClusterFilterOffMatchesPreExistingBehavior), and adds the new translation keys (authorDetail.lastSync.thinCluster, settings.metadata.formClusterFilter*/clusterFilterBadge) to en.json, which were relying on the i18next defaultValue fallback instead of being registered like every sibling key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

…avallee#2235) Adds tests-only coverage for the vavallee#2235 Phase 2 ClusterFilterPreset work on this branch: - metadata_profiles handler: reject an unknown clusterFilterPreset on create and update (validateClusterFilterPreset), accept a measured preset, and default an omitted preset to off. - authors_filterengine: drive the cluster.editionCountSupport exclusion counter (skippedThinCluster) via recordExcluded. - metadata_profiles repo: round-trip the cluster_filter_preset column through CreateForUser/GetByID/Update, and pin Update's empty-field defaults.

…avallee#2235) Follow-up to the Phase 2 cluster-preset coverage: close the remaining non-defensive gaps in the metadata-profile and filter-engine handlers. - metadata_profiles: cross-user Update is 404 under the tenancy gate (IDOR guard), non-numeric {id} is 400 on Get/Update/Delete, and a malformed Update body is 400. - authors_filterengine: addSample caps the cluster skipped-book sample at authorSyncSkippedSampleLimit.

vavallee · 2026-09-17T02:57:24Z

Hi @gchahcg, thanks for the huge amount of work here, and for the measurement behind #2235. The finding that edition count only separates real works from noise once records are grouped by title is genuinely useful, and the writeup was a pleasure to read.

I'm going to close this and #2626 though, and that's partly on me: nobody replied on #2235 before you started, so you built a full scoring framework without anyone saying how much of it Bindery wanted. The honest answer is less than this. A general engine with weights, bands and presets is more machinery than I want to maintain for the catalogue filters, especially with most of it (the REVIEW band, the threshold columns) unused by design.

A review also turned up a few things that would have blocked it anyway, in case they're useful:

Parity broke in the OpenLibrary client. Noise works are now flagged instead of dropped in openlibrary/client.go, so mergeAuthorWorks can fold a real Hardcover book with the same title into the flagged row and the sync then excludes it. On main that Hardcover book gets added. The parity tests didn't catch it because they compare against hand written counts through a stub that skips the OpenLibrary client.
Any preset excludes works with no edition count. Only OpenLibrary search results fill EditionCount, so a Hardcover primary author loses their whole catalogue on conservative, balanced or aggressive (0 of 2 added in a test, and skipped_thin_cluster is missing from the sync log line).
The keep bonus overrides filters the user turned on. On balanced and aggressive it cancels part book, missing date, ISBN and min pages vetoes, which undoes the box set fix from Box sets and omnibuses are only pruned when Hardcover enrichment is available, so OpenLibrary-only catalogues fill with bundle rows #1780.
Housekeeping: three commits lack a sign off (the gch ahcg identity), migration 086 collides with feat(metadata): protect hardcover daily quotas (#2593) #2607 and Feature/download client media eligibility #2633, and the CC-BY sample doesn't credit the dataset author.

If you'd like to pursue the idea in a smaller form, I'd welcome a PR for just the useful part: one optional metadata profile filter that skips an OpenLibrary work when its title group's highest edition count is below a threshold, applied only to OpenLibrary works, with 0 treated as "no data" rather than noise, never overriding a filter the user set, and with a test that runs main's current filter functions on the same records. Worth a quick comment on #2235 first so we can agree the shape before you write it.

Thanks again, and sorry for the silence on the issue that led here.

github-actions Bot added the bindery-notified Discord notification already sent for this PR label Sep 15, 2026

gchahcg marked this pull request as ready for review September 15, 2026 11:44

gchahcg force-pushed the feat/2235-phase2-cluster-signal branch from aa3b0f5 to c689b1b Compare September 16, 2026 11:33

gchahcg and others added 18 commits September 16, 2026 07:34

docs: changelog and user guide for the cluster filter preset (vavalle…

f79cf79

…e#2235 Phase 2) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J3A3uW6ZH6m56q1SLVg7Ky Signed-off-by: gchahcg <7381837@proton.me>

gchahcg force-pushed the feat/2235-phase2-cluster-signal branch from c689b1b to e3cafd3 Compare September 16, 2026 11:42

vavallee mentioned this pull request Sep 17, 2026

Feature/download client media eligibility #2633

Open

6 tasks

vavallee closed this Sep 17, 2026

This was referenced Sep 17, 2026

feat(filterengine): scored-signal catalogue filtering engine (#2235) #2626

Closed

Additive scored signals instead of first-drop-wins boolean filtering #2235

Open

gchahcg mentioned this pull request Sep 17, 2026

feat(author): drop thin edition clusters from catalogue sync (#2235) #2673

Open

7 tasks

Navigation Menu

feat(filterengine): opt-in cluster-based noise filtering preset (#2235) - #2628

feat(filterengine): opt-in cluster-based noise filtering preset (#2235)#2628
gchahcg wants to merge 18 commits into
vavallee:mainfrom
gchahcg:feat/2235-phase2-cluster-signal

gchahcg commented Sep 15, 2026

Uh oh!

codecov Bot commented Sep 15, 2026 •

edited

Loading

Uh oh!

vavallee commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gchahcg commented Sep 15, 2026

Summary

Implementation notes

Validation

Why minimal / scope decisions

Checklist

Test plan

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vavallee commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 15, 2026 •

edited

Loading