fix(face-review): GA-readiness fixes from the #834 correctness review - #977
Merged
Conversation
…c and add BDD conventions
…s still boot An install that raised facialRecognition.maxDistance before `suggestions` existed carries no suggestions block, so the merge supplied the 0.7 default — inverting the band against any recognition distance >= 0.7. In config-file mode buildConfig threw and the server crash-looped on upgrade; on a database source every settings page 400'd, because the whole config is PUT on each save. deriveSuggestionBand raises an UNSET band to maxDistance + 0.2 (clamped to the schema ceiling of 2, disabling the feature if no valid band exists). A band the admin set explicitly is left alone, so a real misconfiguration still surfaces.
…very moved face executeRepair wrote source='manual' for every move. A manual identity link is the strongest verdict in the system — getManualLinkedFaceIds and the pending eligibility anti-join both exclude such a face from every scan and every suggestion queue, permanently. So accepting a scan suggestion on a large cluster made every one of those faces unreviewable forever, while the response reported locked: 0 and the DTO promised 'plain moves stay undurable unless the caller opts in'. The relink still always happens — leaving the face on the destination while it carries the source person's identity is the torn state FaceIdentityBackfill silently reverts. Only the strength is now conditional: 'manual' when the caller asked to lock, 'owner-person' otherwise. Route keys include the flag so a mixed batch cannot collapse into one write. entireCluster carries no lock field by design and keeps today's durable default. Four medium tests asserted the old behaviour, one of them named 'records a human placement for a moved face even when lock: false' with a comment stating the flag no longer changes durability. That was a deliberate decision that contradicted the DTO and the picker's lock checkbox; this commit resolves the contradiction the other way and the tests now pin the new behaviour, with a lock:true positive control alongside.
…top rendering NaN
Five call sites passed .toLocaleString() into a {count, plural} argument. ICU
computes # as value - offset, so "2,952" - 0 is NaN: the whole-cluster confirm
read "This moves all NaN faces to Anna" and its button "Move all NaN". Only
above a thousand, which is exactly where a whole-cluster move is least
reversible.
Adds a render test at 2952 with a discriminating control (the formatted string
still yields NaN) plus a source scan so a new call site cannot reintroduce it.
One existing test asserted values.count === '5' as a string, which was only
true because of the formatting; it now pins the number.
…ss another user's queue Every verdict row is stamped with the target's identity, and face_identity.id is a cross-owner key whose anti-join in getPendingForPerson has no ownership filter. So a row written against a face the caller does not own suppresses the ASSET OWNER's personal suggestion queue for their own face — and a confirm also runs clearNegativeForTarget, whose identityId arm can delete a rejection that owner recorded, leaving no trace it existed. The personal twin has guarded this since S11, with a comment naming the exact hazard. Its space counterpart checked only Editor role and reachability. Space membership grants rights inside the space; it does not grant the right to record a durable verdict about another user's face. Guard sits before the reachability check so an unowned face 400s rather than returning a silent false. dismiss delegates to reject and inherits it; both are pinned, alongside a positive control that the same call succeeds when the editor does own the face.
The lock bucket's eligibility read (getEligibleFaceIdsForPerson) runs outside the write transaction and its own comment calls itself "advisory only". reattributeFaces and detachFaces both re-check placement at write time; replaceFaceIdentities did not, so a concurrent reassign landing between the read and the write could leave asset_face.personId pointing at the new person while this call still re-pointed the old person's identity onto the face. replaceFaceIdentities now accepts an optional requirePersonId: when set, each chunk is filtered (inside the caller's transaction) to faces still assigned to that person before writing, and returns the asset-face ids actually written. The lock bucket passes requirePersonId, scopes drainPendingForFaces/clearNegativeForTarget to the written ids, and counts locked from what was actually written rather than what was requested. The move path (executeRepair) keeps calling replaceFaceIdentities without requirePersonId, since reattributeFaces has already re-pointed personId inside the same transaction by the time it runs. Added medium coverage for the guard: a raced face is excluded from the write and the returned ids, a face still on the required person writes normally, requirePersonId omitted keeps writing unconditionally, and an empty input returns []. Updated two pre-existing F12/F13 tests whose duplicate-lock-id assertions hard-coded the old count-what-was-requested semantics for `locked`.
…n suppress another user's queue" This reverts commit b51afc0.
… be decided The space face-suggestion flow is cross-owner by design — the canonical fixture has an Editor acting on a third user's asset, and the pending read deliberately spans cross-owner contributions. Requiring face ownership denied the feature's primary flow and shipped a red medium suite. The underlying leak (a non-owner's space verdict suppressing the asset owner's personal queue via the cross-owner identity arm, and a confirm deleting that owner's rejection) is still open. Slice 4 now records the three candidate fixes and their trade-offs instead of a fix that cannot ship.
face-verdict.service.ts fans out to four reads that bind one parameter per id and were never chunked: getManualLinkedFaceIds and getPersonVerdictTokens (face-identity.repository.ts), getNegativeVerdictTokens (face-person-verdict.repository.ts), and getClusterMuteMap (face-repair-decline.repository.ts). minFaces is admin-settable, so a full-library scan can pass every flagged face (or suspected owner) in the instance and exceed Postgres's 65,535 bind-parameter ceiling, permanently failing scans at high minFaces. Three of the four return a Map, which the existing @ChunkedArray/ @ChunkedSet decorators cannot merge, so all four are chunked with an explicit loop of 1000 instead, matching every sibling write path in this feature. No decorator changed, so the generated server/src/queries/*.sql docs are unchanged.
Slice 1 (B1):
- Add the equality-boundary case. An install sitting exactly at the 0.7 default
band is the most common upgrade shape, and mutating the guard from > to >=
left the whole file green while reintroducing the crash for those users.
- Round the derived value to two decimals: 0.7 + 0.2 is 0.8999999999999999,
and updateConfig persists it and the admin number input renders it.
- Warn when the ceiling forces suggestions off. The invariant check in
buildConfig is gated on suggestions.enabled, so it cannot fire afterwards —
without this the admin gets no diagnostic at all.
- Document the derivation in the admin docs (the slice's own step 5, missed).
Slice 3 (B3):
- Tighten the source guard from toLocaleString\(\) to toLocaleString\(.
A locale-aware call such as toLocaleString('de-DE') formats identically and
evaded the narrower pattern; verified by mutation that it now fails.
Slice 2 (B2):
- Correct two comments that contradicted the code after the behaviour change.
face_repair_scan_in_flight_uq was UNIQUE (status) WHERE status IN
('pending','running'), which is unique on the VALUE of status — so
one 'pending' row and one 'running' row could coexist. Two admins
crossing the pending -> running transition at the same time both
succeeded, and pruneSupersededScans then deleted the loser's
flagged-face rows mid-flight.
Fixed with a constant-expression unique index (ON (true) WHERE ...)
so every in-flight row evaluates to the same key and at most one can
exist regardless of which in-flight status it carries. The migration
first demotes every in-flight row but the newest to 'failed', so an
instance that already hit the old race can still apply this migration
instead of crash-looping on index creation. Table decorator and
migration_overrides payload are updated to match (schema-drift.spec.ts
stays green), and the new migration name is appended to
revert-to-immich.sql.
PersonSuggestionScan and SpacePersonSuggestionScan set removeOnComplete but not removeOnFail, unlike every sibling job option in getJobOptions. A job that fails while removeOnFail is unset permanently occupies its stable per-person dedup jobId, and BullMQ silently ignores every later add() with the same id — one transient failure and that person's suggestion queue never refills again, with no log and no admin-visible symptom. removeOnFail protects new failures only; it does nothing for jobIds already stuck in the failed set. The existing prefix sweep (removeFailedJobsByJobIdPrefix) has one production caller, SharedSpaceService.onBootstrap, gated by SystemMetadataKey.SharedSpaceFaceJobCleanupState — already true on every instance that has ever booted, so folding the new prefixes into that gate would make the sweep for them a permanent no-op exactly where it is needed. Added a second, independent one-time sweep under its own SystemMetadataKey.PersonSuggestionScanJobCleanupState so it actually runs and clears the already-stuck jobIds.
…ck them
18 count-bearing admin.face_cleanup_* keys had been flattened to a bare
{count} interpolation in de/fr/it/nl/pl/es/ru/zh_Hans/zh_Hant, so they
rendered ungrammatical text like "1 Gesichter" / "1 visages" / "1 лиц" for
an admin whose queue was down to exactly one item. The guard written to
catch this (face-cleanup-plurals.spec.ts) only ever registered `en` and
read en.json, so it stayed green while the bug shipped in all nine
translations.
- Extract the fork's nine-locale list into web/src/lib/i18n/fork-locales.ts
(FORK_LOCALES) and have fork-string-parity.spec.ts and
face-cleanup-i18n-coverage.spec.ts import it instead of each keeping
their own copy, which could silently drift apart.
- face-cleanup-plurals.spec.ts now derives the plural-bearing key list from
en.json (any admin.face_cleanup_* whose English value carries
`{count, plural`) and asserts every fork locale's translation of it also
carries an ICU plural clause. A key missing entirely from a locale falls
back to English and is fork-string-parity's concern, not this guard's.
- zh_Hans / zh_Hant are not valid BCP47 tags: constructing IntlMessageFormat
with the underscore code throws, and svelte-i18n silently falls back to
the raw, unparsed message — so a render assertion registered under
'zh_Hans' would pass no matter what the translated content says. The
locales are now registered under their hyphenated convertBCP47 form
(exactly how the app's own initLanguage does it) while still reading the
underscore-named source files, with tests proving that registration is
load-bearing rather than decorative.
- Rewrote the 18 keys in all nine locales using each language's own CLDR
plural categories rather than a mechanical copy of English's two
branches: one/other for de/es/fr/it/nl, one/few/many/other for pl/ru,
and other-only for zh_Hans/zh_Hant (Chinese has no plural inflection).
- Fixed i18n/nl.json's face_cleanup_review_picker_create: a bare apostrophe
before `{query}` opens an ICU quoted-literal section, so the placeholder
printed literally instead of being substituted. Replaced the apostrophes
with a plain double quote (matching English's own `"{query}"`), which is
not ICU-significant, and added a render assertion plus a positive
control proving the old shape really did leak the placeholder.
No apostrophe guard was added for the other locales — a naive /'\{/ regex
produces false positives across existing correct ICU escapes (e.g. oauth
and delete_group keys), and this was the only genuine occurrence.
face_repair_decline.personId is ON DELETE CASCADE (unlike face_person_verdict.personId, which is SET NULL and already re-pointed by retargetVerdictPersonId), so mergePersonProfile deleting the source person silently destroyed any cluster mute recorded against it. The console's "stop showing me this cluster" decision vanished and the cluster resurfaced on the very next scan, even though face-repair.merge-consistency.spec.ts's own header claims both durable Face Cleanup facts survive a merge — they didn't, this one just had no test. Added retargetDeclinePersonId (server/src/utils/face-decline-merge.ts, mirroring face-verdict-merge.ts's style) and call it in mergePersonProfile immediately after retargetVerdictPersonId, so it commits or rolls back with the rest of the merge. A person carries at most one type='person' decline row (createClusterMutes deletes-then-inserts), so there are three cases: only the source has a mute (move it), only the survivor has one (or neither does — no-op), or both have one, in which case the survivor's row absorbs the union of both suspected-owner sets and the source row is dropped. Leaving two rows for the survivor would be worse than the bug: getClusterMuteMap does a plain Map.set() per row it reads, so which row "wins" would depend on read order. Edge case: suspectedOwnerIds carries no FK, so if the survivor's mute already lists the source person as a suspected owner, the union keeps that now-dangling id. Left as-is — nothing upstream of this function enforces referential integrity on that column either, and pruning it is out of scope for a merge-time re-pointing step.
All seven gaps were confirmed by watching the named mutation survive the existing suite (green), then adding the assertion and watching it fail (red), then reverting (green again): - person.service.ts confirmFaceSuggestion: deleting the face-ownership requireAccess call (Permission.PersonCreate) survived unit tests. - person.service.ts handlePersonSuggestionScan: removing `visibility: spaceVisibleAssetVisibilities` from the personal searchFaces call survived (would let Locked/Hidden assets become suggestable). The space-scan call's copy of the same arg is confirmed redundant by the search repository's unconditional spaceVisibilityGate on that branch, so it is deliberately left unpinned. - identityId was expect.any(String) at every verdict-write assertion in person.service.spec.ts and shared-space.service.spec.ts. Pinned to the concrete id the mocked ensure*Identity call returns, and added an argument-pin on the shared-space reject/ignore call site (the OTHER ensureSpacePersonIdentity call site, inside confirm, was already pinned). - clearNegativeForTarget: all 6 production call sites (3 in person.service.ts, 1 in shared-space.service.ts, 2 in face-repair.service.ts) were deletable with zero failures. Added an argument assertion at each, matching that site's real target shape (personId vs spacePersonId). - face-repair.service.ts executeRepair: the C6 cross-owner guard could never fire because every existing test resolves both people to the same owner. Added arrangeDifferentOwnerMove and a test that gives getById differing ownerIds per id, asserting the route is skipped and nothing is written. - face-repair.service.ts triggerScan: hardcoding all seven scan-override params to their defaults survived — the Advanced Scan modal was decorative. Added an assertion pinning all seven fields to non-default values. - Deleted two tests that could never fail: the "admin guard" test in face-repair.scan.spec.ts asserted only that a QueueName constant is defined (the real guard is already proven by face-repair-admin.controller.spec.ts's authenticated-route tests), and face-repair.scan-defaults.spec.ts, which echoes its own mocked config values back at itself — verified vacuous by hardcoding the service to ignore its config input entirely and watching the test still pass.
…etion cascades
- web/vite.config.ts: enable clearMocks: true globally. Ran the whole web unit
suite twice after enabling it (5255/5255, identical to the pre-change
baseline both times) — nothing relied on cross-test mock-call leakage, so
no fixes were needed.
- Three testid assertions targeted testids that exist in no component
(locks-section, manual-review-load-more, move-rest-selection-btn) and so
could never fail. Deleted each: none has a real counterpart to point at
(resolutions has no locks concept, the manual-review page uses a scroll
sentinel with no load-more button, and the rest section was deliberately
built with no separate commit button — its real guard,
`resolveFaces).not.toHaveBeenCalled()`, already covers the behaviour).
Per the brief's correction, no generic testid-guard test was added.
- scan/+page.svelte's polling test never advanced past the completed-scan
poll. Rewrote it to advance to the poll that returns a completed scan, then
advance another 20s (past POLL_MAX_MS) and assert getLatestScan's call
count is unchanged — this fails if stopPolling() is skipped in the
!isActive branch.
- [personId]/page.spec.ts only ever asserted `goto` was NOT called. Added a
positive assertion on the Apply-success path (goto called with
Route.faceCleanupScan()) and on the empty-state Cancel button (the other
goto call site on this page) — both currently unexercised. The manual
sibling (people/[personId]) already had positive coverage for its own
target, Route.faceCleanupPeople(); left as-is.
- Added page-load.spec.ts for all 7 face-cleanup +page.ts files (the repo's
existing convention), covering the authenticate(url, { admin: true }) gate
on each and the declined/+page.ts 307 redirect to the unified resolutions
page.
- Added medium tests for three FK-cascade paths nothing exercised against a
real database: deleting an asset_face cascades its face_person_verdict row
(ON DELETE CASCADE); deleting the actor user degrades actorId to NULL (and
a scan's requestedBy the same way) while listNegativeVerdicts still renders
the row via its LEFT JOIN; deleting a shared space cascades its
shared_space_person row, which degrades the verdict's spacePersonId to
NULL while identityId (an independent FK straight to face_identity)
survives untouched.
Full web unit suite green (347 files / 5269 tests) with check:svelte and
check:typescript clean. Server: ran the full medium suite twice; the
resulting failures are pre-existing, environmental (Postgres connection
exhaustion under full-suite parallelism — a different set fails each run)
and reproduce on files this commit never touches; every touched/added medium
file passes cleanly in isolation.
…d files person.service.ts carried the fork's entire face-suggestion engine inline — the four suggestion-scan job handlers and the five confirm/reject/ignore/ dismiss/get endpoints — none of which interleave with upstream PersonService logic. Moves that code, verbatim, into a new FaceSuggestionService and FaceSuggestionController, matching the isolation already applied to face-repair.service.ts, classification.service.ts and shared-space.service.ts. person.controller.ts returns to its origin/main content; person.service.ts keeps only the genuine in-place hooks (verdict clearing in reassignFaces/ reassignFacesById, the re-scan queue in update(), the backfill-completion queue, and the bootstrap sweep). Medium tests that exercised the moved methods through PersonService now use a second service instance sharing the same MediumTestContext dependencies (MediumTestContext.getService), added to test/medium.factory.ts for this purpose. Behaviour is unchanged: every route path, DTO and job name is identical, the unit suite's pass count is identical before and after (5655 passed / 14 skipped), and rebuilding + regenerating the OpenAPI spec and both generated clients produces a zero diff in open-api/, packages/sdk/ and mobile/openapi/.
getJobCounts is a pure delegate upstream. The fork made it async and prepended removeDanglingActiveJobs (cleaning up BullMQ "active" list entries with no backing job hash or lock, first found on the fork's own PeopleBackfill queue) directly inside it, so every read — including upstream's own call paths (queue.service.ts's admin queue poll, media.service.ts, person.service.ts's recognition-queue checks, storage-migration.service.ts, and this repository's own getTelemetryMetrics) — silently mutated Redis. Adds getJobCountsWithRepair(name), carrying today's repair+delegate body, and restores getJobCounts to upstream's exact pure form. Every caller listed above turned out to be either byte-identical to origin/main or, in getTelemetryMetrics's case, a passive per-request-shaped telemetry read that should never write on a read either way — verified individually against origin/main rather than taken on the original spec's word, which mislabeled several of them as fork-added. The one caller that genuinely needs the repair is waitForQueueCompletion: it blocks in a loop deciding whether a queue has actually drained, and a dangling active entry would otherwise read as perpetually "still active" — a comparatively rare, blocking wait, not a per-request read, so paying for the repair there does not reintroduce the problem. Adds a test asserting getJobCounts performs no Redis writes even with a dangling active entry present, and renames the existing repair test to target getJobCountsWithRepair directly.
…, placeholders.spec.ts and e2e utils 15a. FacePersonVerdictRepository sat inside upstream's alphabetized repository list in base.service.ts (BASE_SERVICE_DEPENDENCIES, the constructor parameter list, and the static create() ctx list) instead of beside its four siblings (FaceIdentityRepository, FaceRepairRepository, FaceRepairScanRepository, FaceRepairDeclineRepository). An upstream insertion into that stretch would conflict on every rebase, and a careless resolve would silently desync the positional lists and inject the wrong repository into every service. Moved it beside its siblings in all four affected files (base.service.ts x3, repositories/index.ts, test/utils.ts x3, test/medium.factory.ts x2) — the full unit suite passing is the proof the positional lists still agree. 15b. web/src/lib/i18n/placeholders.spec.ts iterated every file in i18n/, including the ~80 translator-owned locale files this fork does not maintain — which is why mr.json and ms.json had been hand-patched to keep the suite green. Scoped it to en.json + FORK_LOCALES (the shared module fork-locales.ts already created for this exact purpose) and reverted the mr.json/ms.json patches now that Weblate content is out of scope. The module's own doc comment already claimed this suite was scoped correctly; it wasn't — now it is. 15c. Reverted two upstream-owned e2e files to origin/main: asset.e2e-spec.ts (an upstream live-photo test edited to load a fork video fixture from across the server/e2e directory boundary — no independent justification for the real-video requirement was found, so it reverts to upstream's makeRandomImage() bytes) and utils.ts's isQueueEmpty, whose paused-aware semantics change affects all ~97 callers of the shared waitForQueueFinish helper, including upstream's own jobs.e2e-spec.ts (which deliberately waits on a paused queue). No currently-existing fork spec was found to depend on the changed semantics at its actual call site, so no fork-local isQueueEmptyIgnoringPaused was added — the brief's own instruction gates that addition on such a spec existing. utils.ts's DB URL change to use the fork-added playwrightDbPort export was kept; it isn't part of the isQueueEmpty regression and playwrightDbPort has no upstream equivalent to fall back to. Verified with `cd server && pnpm test` (identical pass/fail shape, only the expected +1 from slice 14's new test), `cd web && pnpm exec vitest run`, `cd web && pnpm run check:svelte && pnpm run check:typescript`, and `cd e2e && pnpm exec tsc --noEmit` (e2e suites intentionally not run — they need a live stack).
…st bootstrap H8 split onBootstrap into two independent cleanups, each behind its own state key — the shared-space sweep is already marked done on every booted instance, so appending prefixes to it would never have run. That makes three removeFailedJobsByJobIdPrefix calls on a first boot (PeopleBackfill and FacialRecognition for shared-space faces, PeopleBackfill for the suggestion scans), not two. The test's actual intent — sweep on the first bootstrap only — is unchanged and still asserted: the second boot calls nothing, because both state keys are set. Also pins the new sweep's arguments so the prefixes cannot silently drift.
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.
Implements the GA-readiness spec produced by the #834 correctness review. Based on
feat/face-review-unifiedand targets it, so the fixes can be reviewed separately from the 321-commit feature branch.What's fixed
Blockers
suggestions.maxDistance: 0.7, which inverts against anyfacialRecognition.maxDistance >= 0.7. Config-file installs threw at boot; database installs returned 400 from every settings page, since the whole config is PUT on each save.deriveSuggestionBandnow raises an unset band tomaxDistance + 0.2, clamped to the schema ceiling of 2, and warns when no valid band exists. An explicitly set inverted band still surfaces.lock: falsewas not honoured. Every move wrotesource: 'manual', the strongest verdict in the system: both engines exclude such a face from every scan and every suggestion queue thereafter. Accepting a scan suggestion on a large cluster made all of it unreviewable while the response reportedlocked: 0. The relink still always happens (skipping it leaves the torn stateFaceIdentityBackfillreverts); only its strength is now conditional..toLocaleString()into an ICUpluralarg, so#computed"2,952" - 0. The whole-cluster confirm read "This moves all NaN faces to Anna" — only above 1 000 faces, i.e. where the action is least reversible.High severity
personIdon one person with the identity pointing at another.replaceFaceIdentitiesnow re-checks placement inside the transaction and returns what it actually wrote, which also corrects an inflatedlockedtally.MAX_PARAMETERS_EXCEEDED: Max number of parameters (65534) exceeded.status, so onependingand onerunningrow could coexist and two full scans could race, deleting each other's snapshot. Now a constant-expression index; Postgres rejects the second withKey ((true))=(t) already exists.removeOnFail, plus a sweep that actually runs (the existing one is gated behind a flag already set on every booted instance).{count}in nine locales ("1 Gesichter", "1 лиц"). The guard written to prevent this registered onlyen.face_repair_decline.personIdisON DELETE CASCADEand merge deletes the source person, so merging discarded the console's cluster mute.Test hardening
Every item below was confirmed by applying the mutation and watching the suite stay green before writing the assertion:
confirmFaceSuggestion's face-ownership check survived 349/349.visibility: spaceVisibleAssetVisibilitiessurvived — that mutation makes faces on Locked and Hidden assets suggestable.identityId, the cross-scope key the design rests on, wasexpect.any(String)at 9 sites.clearNegativeForTargetcall sites were deletable with zero failures.executeRepaircould never fire: onegetByIdmock served both source and destination.Also:
clearMocks: trueenabled globally; three assertions targeting testids that exist in no component removed;page-load.spec.tsadded for all seven face-cleanup routes; medium coverage for the deletion cascades (asset_face, user, and shared space, which had no coverage anywhere). Two tests that could not fail were deleted.Fork isolation
person.service.ts/person.controller.tsinto fork-ownedface-suggestion.service.ts/face-suggestion.controller.ts.person.controller.tsreturns to zero diff againstorigin/main. Proof of no behaviour change:git status --porcelain open-api packages/sdk mobile/openapiis empty after a full regenerate.getJobCountsrestored to upstream's pure delegate form; the Redis repair moved togetJobCountsWithRepairfor the fork's own call path.FacePersonVerdictRepositorymoved beside its four siblings in every positional DI list;placeholders.spec.tsscoped to the fork's nine locales (so Weblate can't turn the suite red on a future rebase) and themr.json/ms.jsonhand-patches reverted; two upstream-owned e2e files restored.Verification
check:sveltetsc --noEmit(server + web)Also validated on a real library (58 206 faces / 2 348 people): clean boot and migration, and the H7 index applied as the constant-expression form.
Known limits
memory.service). Every file this work touched passes in isolation. CI shards differently; treating CI as the arbiter.pendingand onerunningscan) is verified by source inspection, not live execution — the global-setup DB already has the fixed index, so the pre-fix state can't be seeded without dropping it first.docs/superpowers/specs/2026-08-11-...) was fact-checked after writing and had ~25 errors corrected during implementation. Individual slice line numbers are stale by design — the commits are the source of truth.