Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions .scratch/rarity-filter/PRD.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# PRD: Rarity filter for affixes, sigils, and tributes

Status: complete

## Problem Statement

As a player using loot filters, I can describe *which affixes* a rule should match, but I cannot constrain a rule to a specific item rarity. With crafting, this matters: I want to keep a common/magic/rare item that matches a rule while NOT keeping a legendary that matches the same rule (because I would craft the lower-rarity item, not the legendary). Today every rarity that matches the affixes is treated the same, so I cannot express "match these affixes, but only on rare items."

Tributes already support a rarity list, but the capability is inconsistent: affixes have no rarity filter at all, sigils have no rarity filter, and the tribute key (`rarities`) differs from what the rest of the tool will use.

## Solution

Every object that has a rarity can be filtered on rarity. A rule may list one or more rarities; an empty/absent list matches all rarities.

- **Affixes**: a filter rule gains a `rarity` constraint that narrows which item rarities the rule matches, alongside the existing item-type/power constraints.
- **Sigils**: a profile gains a global `rarity` gate. Because Diablo 4 does not expose a rarity on sigils, the rarity is derived from the sigil's affixes via the existing `sigils.json` rarities map.
- **Tributes**: keep working exactly as today, but the canonical key becomes `rarity`; `rarities` continues to load as a back-compat alias.

The profile editor lets me set affix rarities through a checkbox picker. Sigil rarity editing arrives together with the fix for the broken sigil editor (#502).

## User Stories

1. As a crafter, I want to match a rule only on rare items, so that I keep craft candidates and not the equivalent legendary.
1. As a crafter, I want to select multiple rarities on one affix rule (e.g. common, magic, rare), so that I keep all viable craft bases in a single rule.
1. As a player, I want an affix rule with no rarity set to match all rarities, so that existing rules keep behaving as before.
1. As a player, I want the affix rarity constraint to be combined with item type and power, so that "rare helms with these affixes" is expressible.
1. As a player, I want a legendary that matches an affix rule restricted to `rare` to NOT be kept by that rule, so that rarity actually narrows the match.
1. As a player, I want legendary aspect / unique / mythic keep behavior to remain unchanged when I add a rarity constraint to an affix rule, so that adding rarity doesn't silently disable unrelated keep logic.
1. As a player, I want to write affix rarities as a single value or a list, so that `rarity: rare` and `rarity: [common, magic]` both work.
1. As a player, I want rarity values to be case-insensitive on input, so that `Rare` and `rare` both load.
1. As a player, I want an invalid rarity value to be rejected with a clear error, so that typos surface instead of silently doing nothing.
1. As a player, I want to set affix rarities in the profile editor via checkboxes, so that I don't have to hand-edit YAML.
1. As a player, I want the affix rarity picker to offer all six rarities (common, magic, rare, legendary, unique, mythic), so that the control is complete.
1. As a player, I want my chosen affix rarities shown as a summary next to the rule, so that I can see the constraint without opening the picker.
1. As a player, I want a global sigil rarity gate, so that I can keep only sigils of chosen rarities regardless of dungeon.
1. As a player, I want the sigil rarity gate to combine with my blacklist/whitelist as an AND, so that a sigil must pass rarity AND survive blacklist/whitelist to be kept.
1. As a player, I want a sigil whose rarity cannot be resolved to be dropped when a sigil rarity filter is active, so that unknown-rarity sigils don't leak through a `rarity: [rare]` gate.
1. As a player, I want unresolved sigil rarities logged at debug, so that gaps in the rarities map can be diagnosed.
1. As a player with an empty sigil rarity list, I want all sigil rarities to pass the gate, so that the gate is opt-in.
1. As an existing tribute user, I want my profile written with `rarities` to keep loading, so that the rename doesn't break me.
1. As a tribute user, I want `rarity` to be the documented key going forward, so that the vocabulary is consistent across the tool.
1. As a player, I want one consistent rarity vocabulary across affixes, sigils, and tributes, so that I don't have to remember per-section spellings.
1. As a player, I want the broken sigil editor (#502) usable so that I can edit sigil rules — including the new rarity gate — in the GUI.
1. As a maintainer, I want the rarity normalization logic in one shared place, so that affixes and tributes parse rarities identically.

## Implementation Decisions

Split into two releases:

- **Release 1 — affix rarity + tribute vocabulary unification.**
- **Release 2 — #502 sigil-editor fix bundled with sigil rarity (model, filter, GUI).**

### Canonical key and parsing (ADR-0001)

- Canonical profile key is singular `rarity`, holding a list. Empty/absent = match all rarities.
- The Pydantic field stays named `rarities` internally so filter code keeps reading `model.rarities`; only the serialization/validation aliases change. `rarity` is the serialization alias and a validation alias on all rarity-bearing models. On tributes, `rarities` is *also* a validation alias for back-compat.
- A single shared normalizer parses rarity input everywhere: bare string → single-element list, case-folded to lowercase, then strictly validated against the `ItemRarity` enum (invalid values raise). This replaces the tribute-specific parsing while preserving its lenient input behavior.

### Affix rarity (Release 1)

- `ItemFilterModel` gains the `rarity` constraint, a sibling of item type and power.
- Filtering: rarity is an item-level gate evaluated once per filter rule (next to the existing item-type check), not inside the per-affix matching loop. If the rule's rarity list is non-empty and the item's rarity is not in it, the rule does not match (skip to next rule).
- The affix rarity gate only narrows the affix-matching path. The separate keep paths in `should_keep` — legendary aspect/codex upgrade, global uniques, mythic-always-keep — are unchanged and still override.
- Allowed values: all six `ItemRarity` members. (Unique/mythic on an affix rule are largely inert because uniques without a matched unique-aspect are already skipped; this is accepted rather than special-cased.)

### Sigil rarity (Release 2, ADR-0002)

- `SigilFilterModel` gains a top-level `rarity` list (sibling of `priority`/blacklist/whitelist). Empty = all. It is a global gate over every sigil, not per blacklist/whitelist entry.
- Rarity derivation: scan the sigil's affixes (item affixes + inherent); the first affix found in the `sigils.json` `rarities` map yields the rarity. No match = unknown.
- Gate semantics: when the rarity list is non-empty, apply it as an AND gate *before* the existing blacklist/whitelist logic. A sigil whose rarity is not in the list is dropped.
- Unknown rarity is fail-closed: it never matches a non-empty rarity filter (sigil dropped), and the unresolved lookup is logged at debug.
- #502 fix (sigil editor cannot blacklist global top-level affixes without a dungeon) is bundled into this release as a prerequisite for adding the sigil rarity GUI control. Fix approach (ADR-0003): the editor derives a `kind` (`affix`/`dungeon`) from the rule name — no schema change. Affix-kind rows render only the affix picker (condition UI hidden) and act as a global blacklist/whitelist; dungeon-kind rows are unchanged. Name validation stays loose (combined affix+dungeon pool).

### GUI

- Affix editor (Release 1): add a "Rarities:" row to the affix group editor, directly after "Item Types:", as a read-only summary line plus a "..." button opening a new checkbox-based rarity picker that mirrors the existing item-type picker. All six rarities offered.
- Sigil editor (Release 2): add a rarity control to the sigil tab on top of the #502-fixed editor.
- Tribute editor: no change required; it reads the `rarities` field which is unaffected by the alias work.

## Testing Decisions

Good tests assert external behavior — what a profile parses into, and what `Filter.should_keep` keeps — not internal helper calls.

### Seam 1 — model parsing (`tests/config/models_test.py`)

Drives `ProfileModel(**data)` / `ItemFilterModel` / `TributeFilterModel`. Cover:

- Affix `rarity` parses from single string, from list, and case-insensitively.
- Invalid rarity value is rejected.
- Empty/absent rarity parses to all-match (empty list).
- Tribute back-compat: `rarities` still loads; `rarity` is the canonical serialized key.

Prior art: `test_camelcase_input`, `test_snake_case_input`, `test_parse_from_string`, and the existing `TributeFilterModel` parse tests in the same file.

### Seam 2 — filter behavior (`tests/item/filter/filter_test.py`)

Drives `Filter.should_keep(item)` and asserts matched profile names. Cover:

- Affix rule with `rarity: [rare]` keeps a matching rare item and does NOT match a legendary with the same affixes.
- Affix rule with empty rarity matches all rarities (regression).
- Sigil rarity gate keeps only listed rarities; AND with blacklist/whitelist.
- Sigil with unresolved rarity is dropped when a sigil rarity filter is active.

Prior art: `test_affixes`, `test_sigils`, `test_sigil_empty_lists`, and the data fixtures in `tests/item/filter/data/` (`items.py`, `filters.py`, `sigils.py`, `tributes.py`). Use `_create_mocked_filter` to reset the singleton.

### GUI

Manually verified by launching the profile editor and exercising the rarity picker. No automated GUI tests (the repo has no profile-editor tests).

## Out of Scope

- Importers: no importer changes; rarity is a user-set filter constraint, not imported build data.
- Populating or auditing the `sigils.json` rarities map. The feature consumes whatever coverage exists; gaps surface via the debug log.
- Per-entry sigil rarity (rarity attached to individual blacklist/whitelist entries). The gate is global.
- Tribute GUI changes and any change to tribute keep behavior.
- The detailed design of the #502 sigil-editor fix is recorded in ADR-0003 (kind derived from name; affix-kind hides condition UI).

## Further Notes

- ADR-0001 records the canonical-`rarity`-with-`rarities`-alias decision; ADR-0002 records the sigil rarity derivation and fail-closed-on-unknown decision; ADR-0003 records the #502 sigil-editor affix/dungeon kind split. All live in `docs/adr/`.
- CONTEXT.md defines "Item rarity" and "Rarity filter"; use that vocabulary (`rarity`, not `rarities`) in implementation and tests.
- Release 2 bundles a bugfix (#502) with a feature; keep the commits separable for review/revert.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Unify rarity key and shared normalizer

Status: complete

## Parent

`.scratch/rarity-filter/PRD.md`

## What to build

Make `rarity` the single canonical rarity key across all rarity-bearing filters, and route all rarity parsing through one shared normalizer.

End-to-end: a profile may specify rarities as a single string or a list; input is case-insensitive (`Rare` == `rare`); invalid values are rejected with a clear error; the absent/empty case means "all rarities". Tributes — the only filter that already shipped rarity — keep loading profiles written with the old plural `rarities` key, but `rarity` becomes the canonical serialized key.

Per ADR-0001: the Pydantic field stays named `rarities` internally (so filter code keeps reading `model.rarities`); only the aliases change. `rarity` is the serialization alias and a validation alias; on tributes, `rarities` is also a validation alias for back-compat. This slice introduces the shared normalize-then-strict-validate helper and rewires the tribute model onto it.

## Acceptance criteria

- [x] A tribute profile written with `rarity` loads and round-trips with `rarity` as the serialized key.
- [x] A tribute profile written with the legacy `rarities` key still loads (back-compat alias).
- [x] Rarity accepts a bare string and a list; both normalize to a lowercase list.
- [x] Rarity input is case-insensitive (`Rare`, `rare` both valid).
- [x] An invalid rarity value raises a validation error.
- [x] Empty/absent rarity normalizes to an empty list (match-all).
- [x] All rarity parsing goes through one shared helper (no tribute-specific duplicate).
- [x] Tests added in `tests/config/models_test.py` (prior art: `test_parse_from_string`, `test_camelcase_input`, existing `TributeFilterModel` parse tests).

## Blocked by

- None - can start immediately
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Fix sigil editor: global top-level affix blacklist (#502)

Status: complete

## Parent

`.scratch/rarity-filter/PRD.md`

## What to build

Fix the sigil profile editor so a user can blacklist top-level sigil affixes globally, without first picking a dungeon (#502). The current editor is dungeon-first and cannot express "blacklist this affix on every sigil."

This slice is HITL: the fix approach is settled in ADR-0003 — the sigil tab derives a `kind` (`affix`/`dungeon`) from the rule name; affix-kind rows are global and hide the condition UI; dungeon+condition rows are unchanged. It is a prerequisite for adding the sigil rarity control to the GUI (slice 04).

## Acceptance criteria

- [x] Fix approach for #502 agreed with maintainer before implementation. (ADR-0003: kind derived from name; affix-kind hides condition UI; loose name validation)
- [x] A user can add a global affix to the sigil blacklist without selecting a dungeon. (kind=affix/dungeon split in `CreateSigil`/`SigilWidget`)
- [x] Affix-kind rows render only the affix picker — condition list/buttons hidden. (`SigilWidget.setup_ui`; `test_affix_kind_has_no_condition_list`)
- [x] Existing dungeon+condition sigil rules continue to load and edit correctly. (`derive_sigil_kind` on load; `test_sigils_tab.py`)
- [x] Profiles produced by the fixed editor validate against `SigilFilterModel`.
- [x] Manually verified in the profile editor.

## Blocked by

- None - design signed off (ADR-0003); only manual GUI verify remains
30 changes: 30 additions & 0 deletions .scratch/rarity-filter/issues/03-affix-rarity-filter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Affix rarity filter (model + filter + GUI)

Status: complete

## Parent

`.scratch/rarity-filter/PRD.md`

## What to build

Add a `rarity` constraint to affix filter rules, end-to-end: profile parsing, filter behavior, and the profile editor.

End-to-end behavior: a rule may list rarities; an empty/absent list matches all. The rarity is an item-level gate evaluated once per filter rule (alongside the existing item-type check), not inside the per-affix matching loop. If the rule's rarity list is non-empty and the item's rarity is not in it, the rule does not match. The gate only narrows the affix-matching path — the separate keep paths (legendary aspect/codex upgrade, global uniques, mythic-always-keep) are unchanged and still override. All six `ItemRarity` values are accepted.

GUI: add a "Rarities:" row to the affix group editor immediately after "Item Types:", as a read-only summary line plus a "..." button opening a checkbox rarity picker that mirrors the existing item-type picker, offering all six rarities.

## Acceptance criteria

- [x] `ItemFilterModel` gains the `rarity` constraint (canonical key `rarity`, via the shared normalizer from slice 01).
- [x] A rule with `rarity: [rare]` keeps a matching rare item.
- [x] A legendary with the same affixes is NOT matched by a rule restricted to `rare`.
- [x] A rule with empty/absent rarity matches all rarities (regression).
- [x] Legendary aspect / global unique / mythic keep behavior is unchanged when a rarity constraint is present. (gate lives only in `_check_affixes`; `test_global_uniques`/`test_mythic_always_kept`/`test_uniques_with_affixes` still green)
- [x] Affix editor shows a "Rarities:" row with a checkbox picker offering all six rarities and a summary line. (`RarityPicker` in `dialog.py`)
- [x] Tests: model parsing in `tests/config/models_test.py`; filter behavior in `tests/item/filter/filter_test.py` (prior art: `test_affixes`, `_create_mocked_filter`).
- [x] GUI manually verified.

## Blocked by

- `.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md`
33 changes: 33 additions & 0 deletions .scratch/rarity-filter/issues/04-sigil-rarity-filter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Sigil rarity filter (model + filter + GUI)

Status: complete

## Parent

`.scratch/rarity-filter/PRD.md`

## What to build

Add a global sigil rarity gate, end-to-end: profile parsing, filter behavior with derived rarity, and the profile editor.

Per ADR-0002: Diablo 4 does not expose a rarity on sigils, so rarity is derived by scanning the sigil's affixes (affixes + inherent); the first affix found in the `sigils.json` `rarities` map yields the rarity. `SigilFilterModel` gains a top-level `rarity` list (sibling of `priority`/blacklist/whitelist); empty = all, and it gates every sigil globally rather than per blacklist/whitelist entry.

Gate semantics: when the rarity list is non-empty, apply it as an AND gate before the existing blacklist/whitelist logic — a sigil whose derived rarity is not in the list is dropped. Unknown rarity is fail-closed: it never matches a non-empty rarity filter (sigil dropped), and the unresolved lookup is logged at debug.

GUI: add a rarity control to the sigil tab, on top of the #502-fixed editor.

## Acceptance criteria

- [x] `SigilFilterModel` gains a top-level `rarity` list (canonical key `rarity`, via the shared normalizer from slice 01).
- [x] Sigil rarity is derived from the `sigils.json` rarities map (first matching affix wins). (`Filter._get_sigil_rarity`)
- [x] A non-empty `rarity` gate keeps only sigils of listed rarities and ANDs with blacklist/whitelist. (`test_sigil_rarity_and_blacklist_*`, `test_sigil_rarity_and_whitelist_drops_whitelisted_wrong_rarity`)
- [x] An empty `rarity` list lets all sigil rarities through (regression).
- [x] A sigil with unresolved rarity is dropped when a rarity filter is active, and the miss is logged at debug. (`test_sigil_rarity_gate_drops_unknown_rarity` asserts the debug log)
- [x] Sigil editor exposes a rarity control. (`SigilsTab` "Rarities:" row reusing `RarityPicker`)
- [x] Tests in `tests/item/filter/filter_test.py` (prior art: `test_sigils`, `test_sigil_empty_lists`, sigil fixtures in `tests/item/filter/data/`).
- [x] GUI manually verified.

## Blocked by

- `.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md`
- `.scratch/rarity-filter/issues/02-fix-sigil-editor-global-affix-blacklist.md`
9 changes: 9 additions & 0 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,12 @@ The stored Paragon overlay data attached to a profile. It represents one importe
### Paragon progression step

One board-state snapshot within a Paragon payload. Each step contains the boards and active nodes for a point in the imported build's progression.

### Item rarity

The quality tier of a droppable object: common, magic, rare, legendary, unique, or mythic. The canonical values are lowercase. For sigils the rarity is not provided by the game and is instead derived from the object's affixes.

### Rarity filter

A filter constraint listing the rarities a rule should match. An empty list matches all rarities. Spelled `rarity` (singular) in profiles; `rarities` is accepted only as a back-compat alias on tributes.
_Avoid_: `rarities` as the canonical key.
Loading