Cut the dead weight the repo audit found: -1 dependency, -7 files, no behaviour change - #163
Merged
Merged
Conversation
Five unrelated cuts from a whole-repo over-engineering pass. No behaviour change anywhere: 552 tests pass, and the emitted migration SQL is proven byte-identical. Pgvector is gone as a dependency. Nothing in src/ used it - embeddings were replaced by in-database trigram matching - but ten migration files pinned it alive through a CLR type parameter. table.Column<Vector> and Property<Vector> became <string>; the type: "vector(1536)" string that actually reaches Postgres is untouched, so `dotnet ef migrations script --idempotent` diffs clean against the pre-change output. Frozen history stays frozen in the sense that matters. Removed four things nothing called: IConnectivityService.ConnectivityChanged (raised, zero subscribers - the view models poll IsConnected on purpose, since they are transient and the service is not, so the event took ConnectivityService's whole IDisposable with it), IFxRateService.GetRateAsync and its ReadCachedAsync helper (superseded by the batch call), the IApiResponse overload of ExtractFirstMessage, and SuggestedLogoUrl. Plus two config keys - RenewalScan described a job that no longer exists, and Logging:LogLevel was never read because Serilog discards the IConfiguration argument. LoginResult, RefreshResult and ChangePasswordResult were byte-identical; they are now one AuthResult record. ResetPasswordResult stays separate on purpose - a reset issues no token pair, and folding it in would make "succeeded with no tokens" representable on the three calls where it must never happen. IBurnRateCalculator and ISubscriptionMatchService were interfaces with one implementation, in the same assembly as it, with no test fake. The concrete types are registered directly. Every other interface in Application stays: they cross into Infrastructure, which is what keeps EF Core out of that project. Finally, appsettings.Development.example.json now exists. README, CLAUDE.md and docs/debug/ANDROID_DEVICE.md all instruct you to copy it, and it had never been committed - a broken first step on every fresh clone. Values match docker-compose.yml so the documented local loop works unedited; Jwt:Secret is blank because it has to be generated. On the tests: three assertions on SuggestedLogoUrl went with the property. They were proxies for "the match was adopted", and the five SaveAsync tests that assert request.CatalogId already pin that far better - they check what actually leaves the device. FakeFxRateService.SinglePairCalls went the same way; with no single-pair path left there is nothing for it to count. Closes #158 Closes #159 Closes #160 Closes #161 Closes #162 Co-Authored-By: Claude Opus 5 (1M context) <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
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.
All five findings from the whole-repo over-engineering audit, in one branch. No behaviour change — 552 tests pass and the emitted migration SQL is proven byte-identical.
Closes #158 — drop the Pgvector dependency
Nothing in
src/used it; embeddings were replaced by in-database trigram matching. Ten migration files kept it alive purely through a CLR type parameter.table.Column<Vector>/Property<Vector>→<string>. Thetype: "vector(1536)"string that actually reaches Postgres is untouched.Proof the DDL is unchanged — generated before and after the change:
Frozen history stays frozen in the sense that matters: a database that already ran these migrations is unaffected, and no new migration is needed.
CREATE EXTENSION vectorstill runs, which is why Neon is still the right host (DEPLOYMENT.md §1).Closes #159 — dead code and dead config
IConnectivityService.ConnectivityChangedIsConnecteddeliberately, since they're transient and the service is a singleton. TookConnectivityService's entireIDisposablewith it.IFxRateService.GetRateAsync+ReadCachedAsyncGetRatesAsync; only a test helper called itExtractFirstMessage(IApiResponse)SuggestedLogoUrlRenewalScan:UtcHourLogging:LogLevelUseSerilog((_, cfg) => ...)discards theIConfigurationargumentSuggestedTierwas left alone. It's a three-value enum driving one null check, but changing it is a behaviour question rather than dead weight, and the properties aren't cleared in lockstep — worth its own pass.Closes #160 — three identical auth result classes → one
LoginResult,RefreshResultandChangePasswordResultwere byte-identical. Now oneAuthResultrecord.ResetPasswordResultstays separate on purpose: a reset issues no token pair, and folding it in would make "succeeded with no tokens" representable on the three calls where it must never happen.Closes #161 — two single-implementation interfaces
IBurnRateCalculatorandISubscriptionMatchServicesat beside their only implementation, in the same assembly, with no test fake. Concrete types are registered directly.Every other interface in
SubVora.Applicationstays — they cross into Infrastructure, which is what keeps EF Core out of that project.Closes #162 — the config example that never existed
README, CLAUDE.md and
docs/debug/ANDROID_DEVICE.mdall tell you to copyappsettings.Development.example.json. It had never been committed — a broken first step on every fresh clone.Values match
docker-compose.yml, so the documented local loop works unedited.Jwt:Secretis blank because it has to be generated. The dev database password is the same throwaway already in trackeddocker-compose.yml, auditedis_secret: falsein.secrets.baseline; the file scans clean.Verification
The two Testcontainers suites are what make the Pgvector change safe to trust: every one of their test classes calls
Database.MigrateAsync(), applying the full migration history from scratch against a realpgvector/pgvector:pg16container — including theCREATE EXTENSION vectorinAddSubscriptionCatalogand the drop inReplaceCatalogEmbeddingWithTrigram.MigrationDriftTestsseparately assertsHasPendingModelChanges() == false, so the designer-snapshot edits did not desync the model..secrets.baselineis updated for line-number drift only — removingusing Pgvector;shifted four existing entries by one line. No new detections; regenerated through the hook, not bypassed.Test changes, and why they aren't a loss of coverage
Three assertions on
SuggestedLogoUrlwent with the property. They were proxies for "the match was adopted" — and the fiveSaveAsync_*tests assertingrequest.CatalogIdalready pin that far better, because they check what actually leaves the device rather than an intermediate field nothing renders. One test was renamed fromAcceptingAMatch_AppliesNameCategoryAndLogoTogetherto...AppliesNameAndCategoryTogetherto match what it now asserts.FakeFxRateService.SinglePairCallswent the same way: with no single-pair path left on the interface, there is nothing for it to count. TheBatchCalls == 1assertion that carries the N+1 regression guard is untouched.