[ucguy4u]

Theme

Unused store package functions/getters — API surface in packages/store that is exported via store.dart but never referenced by the app (or any other package). The live completion/collector flows use collectionProgress, SeriesCompletion.isComplete, and CollectionProgress.isComplete instead.

Follows amiro#41 / amiro#42 (merged 2026-10-23); this pass targets what remains on current main.

Removed

Item	Location	Evidence
completedSeries()	packages/store/lib/src/collection_progress.dart	No references outside its definition and package tests (see search below). App builds series completion via collectionProgress → SeriesCompletion in buildOwnEncounterPayload and uses SeriesCompletion.isComplete in cardBorder.
CollectionProgress.isSetComplete	same file	Only used by removed completedSeries and removed tests; app uses isComplete / wire SeriesCompletion.isComplete.
collectedCosmeticIds()	packages/store/lib/src/collection_highlights.dart	No production references; only collection_highlights_test.dart. Share/encounter paths pass entitlement ids directly (ownedCosmeticsProvider, buildShareUri(..., ownedCosmeticIds: ...)) without this helper.
AvatarRarityScore class and counts map	packages/store/lib/src/avatar_valuation.dart	Production only read .highest (avatar_screen.dart). `rg 'AvatarRarityScore
Tests for the above	store package tests	Exercised only removed API.

Search commands & results (pre-removal, on main)

# completedSeries — only lib definition + store test
rg -n 'completedSeries' --glob '*.dart'

# isSetComplete — only getter, completedSeries, and store test
rg -n 'isSetComplete' --glob '*.dart'

# collectedCosmeticIds — only lib + store test
rg -n 'collectedCosmeticIds' --glob '*.dart'

# AvatarRarityScore / counts — app used .highest only
rg -n 'AvatarRarityScore|avatarRarityScore' --glob '*.dart'
rg -n '\.counts' packages/store app --glob '*.dart'

No matches in Android/iOS manifests, pubspec.yaml, or generated/part files for the removed symbols.

Revert

Two commits on this branch (newest first):

git revert ed54f7eedc7496d1dee63fd8e50d4dbfff0deef7
git revert e0bf9237d7da70706aade39ac9adae391ad14c2a

Or reset the branch to main and close the PR.

Candidates (not removed)

• buildShareUri + owned list wiring — legacy amiro://share path; main share flow uses encounter payloads. Not changed (behavior / scope).
• TODOS.md mentions completedSeries / collectedCosmeticIds — documentation drift only; per cleanup rules, no doc edits in this PR.

Local verification

Head commit: ed54f7eedc7496d1dee63fd8e50d4dbfff0deef7

Environment: Flutter stable (Dart 3.13.7), melos bootstrap, native deps for thermion_dart per CI.

Before (on main, pre-change)

melos run analyze → SUCCESS (all packages: No issues found!)
melos run test:dart → SUCCESS (identity_core, avatar_core)
melos run test:flutter → store 40 tests passed; amiro_app FAILED (2 tests, see below)
cargo test (rust/core) → 5 passed

amiro_app failures on main (unchanged by this PR):

• app/test/store/store_screen_test.dart: lists every catalog item with buy affordance or owned state
• app/test/store/store_screen_test.dart: series items render after unseriesed items, under their header

After (this branch)

melos run analyze → SUCCESS (all packages: No issues found!)
packages/store: flutter test → 36 tests passed
melos run test:flutter → same 2 amiro_app store_screen_test failures as main; all other Flutter packages pass
cargo test (rust/core) → 5 passed

Analyze: pass
Affected package tests (store): pass
Full workspace melos run test: fail (pre-existing amiro_app store screen widget tests on main)

  

[ucguy4u]

Architect comments clear on this head — not an approval

Head reviewed: e0bf923 (base 5eeb347, main; draft). No earlier architect review to supersede.

Verdict: clear, no blocking comments. Pure deletion (+0/-71, 4 files) of three APIs nothing uses. One doc fix is requested before merge (should-fix below). Not security-sensitive, no cross-team architecture change.

1. No callers. git grep at this head for completedSeries, isSetComplete, collectedCosmeticIds (and snake_case / string forms) across app/, every package, tests, docs and scripts: the only remaining hits are TODOS.md lines 53 and 108 (prose). No codegen or dynamic references exist. Public barrel: packages/store/lib/store.dart exports both files wholesale, so these were public — but the package is publish_to: none and is a path dependency of amiro only. GitHub code search across the DevelopersCoffee org finds these names only in amiro, and no other repo has a pubspec referencing amiro. melos run analyze at this head: exit 0, no issues in all 9 packages (app included), so nothing references them.
2. Not planned. DESIGN.md has nothing on set completion or passport rewards. Open amiro PRs: #44 (docs, 15ebd63) does not touch these APIs and still describes them in TODOS.md; #46 (store test fix) does not use or touch them and its files don't overlap this PR. The set-completion feature they were built for (the "collector's frame", TODOS.md:108) is live and does not depend on them: app/lib/sharing/collector_frame.dart uses SeriesCompletion.isComplete from packages/sharing, fed by own_encounter.dart -> collectionProgress() which stays. collectedCosmeticIds has never had an app caller (the app shares purchased ids via ownedCosmeticsProvider).
3. The 2 failing store_screen_test.dart tests ('lists every catalog item…', 'series items render after unseriesed items…') are unaffected. #46's fix touches only store_screen.dart, pump_helpers.dart and store_screen_test.dart. CI on main (5eeb347, run 40260955072) and on this head (run 41390032797) both end "160 passed, 2 failed" with byte-identical failure output (Found 0 widgets with text "Twin Buns" at store_screen_test.dart:53). So the red flutter check here is the same pre-existing failure, not caused by this PR. rust is green. brewstack-style caveat doesn't apply: amiro has Actions on, so a red flutter check is real, but it is the same red as main.
4. My runs: melos run analyze exit 0; packages/store tests 36/36 (40 on main minus the 4 removed); sharing 97, discovery 69, nfc 5, qr 4, avatar_core 7, identity_core 14 all pass. I could NOT run the app's flutter tests locally (native-assets toolchain missing on my box), so app test results rest on the CI run above, which matches main exactly.
5. Coverage kept: remaining tests still pin free-items-count-as-owned, isComplete, ordering and catalog-ignoring in collectionProgress, so the deleted "free-only series is complete" test is covered by the existing cases.

Should-fix before merge (small, in this PR): TODOS.md still documents the removed symbols as shipped. Line 108 ("Completion engine (completedSeries, isSetComplete, SeriesCompletion.isComplete)") and line 53 ("… via collectedCosmeticIds") would point at code that no longer exists. Please reword to the surviving mechanism (collectionProgress + SeriesCompletion.isComplete -> collector's frame). #44 also edits TODOS.md but not these lines, so I don't expect a textual conflict; if you'd rather leave it to #44, say so there explicitly.

Nothing pushed, marked ready or merged by me. PR Governor merges only with the owner's yes.

[ucguy4u]

Architect comments clear on this head — not an approval.

Reviewed exact head ed54f7eedc7496d1dee63fd8e50d4dbfff0deef7 (re-fetched before posting; unchanged). This supersedes review 5732200704, which covered e0bf9237. Only the new commit ed54f7ee (AvatarRarityScore removal) needed fresh review; e0bf9237 is unchanged.

1. Semantics: avatar_screen renders the same rarity as before — verified

• Old: tally rarities in a counts map over the slots with a catalog listing, then highest = max rarity.index over the keys (strict >). New: running max over the same slots with the same strict >. Same slot order, same byAssetId[json[slot] as String?] lookup, same skip for unlisted ids.
• Null/empty: no listed item gives null in both. The screen's current == null ? null : … guard is untouched, and highestRarity != null gates the label as before.
• Ties: rarity is an enum, so equal tiers are the same value and tie-breaking cannot change the output. The counts map never fed highest beyond key presence.
• The one-line screen change drops .highest and nothing else. The variable is still CosmeticRarity?, and the call sites .label and rarityLabelStyle(context, highestRarity) are unchanged.

2. Callers — complete
rg over the whole repo at this SHA (app, all packages, tests, generated/part files, non-md): the only references are the definition, the store.dart export, app/lib/avatar/avatar_screen.dart and packages/store/test/rarity_test.dart. No other package, no generated code, no .counts readers. flutter analyze is clean for packages/store and app/lib/avatar on my checkout.

3. Tests and mutants (Flutter 3.47.5 runs on the box for packages/store; app tests can't run here, no C toolchain for thermion_dart, so I used the CI log for those)
packages/store: 36/36 pass at this SHA. Mutants against avatarRarityScore, each run against rarity_test.dart:

• KILLED: lowest wins (> to <); always null; first-listed wins; unlisted treated as common; skip the glasses slot; only last 3 slots scanned; break after the first listed; ignore the catalog.
• SURVIVED, equivalent: > to >= (same enum value).
• SURVIVED, as expected: skip body (no listed body item exists) and skip top (a Common cannot change the max when a Rare is present).
• SURVIVED, a real gap: last-listed-wins (if (true)). In the only multi-tier test, the Rare item (glasses) is the last listed slot in AvatarDefinition.slots, so "last wins" and "max wins" are indistinguishable. The old test had this same blind spot, since its map-key iteration was equally last-wins-compatible, so this PR did not introduce it. But the function was rewritten from two-pass to single-pass, and the only thing now guarding ordering independence is that one test.
• The new tests are not tautological: expectations are literal enum values, not derived from the code under test. The old counts assertions died with the field they tested.
• End-to-end coverage: app/test/avatar/avatar_screen_test.dart "shows the highest equipped rarity next to the valuation" (no 'Rare' label, then toggle glasses and 'Rare' appears) passed in CI at this SHA.

4. CI on the exact SHA (run 41509626242)

• rust: success.
• flutter: completed with failure, not green. melos run analyze succeeded; melos run test failed. From the job log, the only failing tests are the two known app/test/store/store_screen_test.dart ones ("lists every catalog item with buy affordance or owned state" and "series items render after unseriesed items, under their header"). 425 other test lines pass, including all store and avatar tests. The same job also fails on main's head 5eeb347c, and amiro#46 is the fix.
• No legacy statuses exist; the combined status shows pending with 0 contexts.
• So the red is attributable entirely to main, not to this diff. It is still red. Do not treat it as green. A green run should come after #46 merges and this branch is re-run or updated. PR Governor should gate on that, plus the owner's yes.

5. Scope and description

• 7 files, +20/−105. Only dead-code removal plus the signature simplification needed to remove the wrapper class. No behavior additions, no dependency or pubspec changes, no doc edits.
• The PR body matches the diff: both commits are listed with revert commands, the head SHA is named, and 36 store tests and the same 2 store_screen_test failures as main are reported — all confirmed above.
• Non-blocking nits:
  • The body's "Candidates" section calls out only TODOS.md drift for completedSeries and collectedCosmeticIds. TODOS.md line 35 and the plan docs/superpowers/plans/2026-09-26-store-rarity-series.md line 18 also still say avatarRarityScore returns "per-tier counts". Intentional per the no-doc-edits rule, but worth listing.
  • The avatarRarityScore name on a function that now returns a single CosmeticRarity? is a slight misnomer (highestEquippedRarity would be clearer). It is harmless, and I don't want to churn the signature again in a cleanup PR.

Recommended, non-blocking follow-up (it can go in this PR or a later one): add one test with a custom catalog where a higher tier sits in an earlier slot than a lower tier. For example, Rare hair plus Uncommon glasses should return Rare. That kills the last-wins mutant.

Verdict: clear on this head. The semantic change is behavior-preserving and every caller is covered. It is still a draft and its CI is red only from main's two pre-existing failures. Merge order: #46 first, then #44 and #45. I did not merge, mark ready or push anything. PR Governor merges only with the owner's yes.