Skip to content

Commit 5937d7a

Browse files
chipiclaude
andcommitted
docs+test: close v1.5.0 audit findings
Follow-up to the v1.5.0 cleanup. A four-angle audit found a handful of stale references and one coverage gap; this commit closes them all. Doc fixes (live docs that lied about current state): - CONTRIBUTING.md vocabulary schema: optional fields list said `mask_kind`/`mask_ref`; updated to `mask_spec` with the drawn-form shape, noting the v1.5.0 retirement. - vocabulary/starter/README.md: dropped the deleted `tone_lifted_shadows_subject` row, fixed the count (5 → 4), explained why mask-bound starter entries are out for now. - docs/adr/TA.md MCP component: corrected stale "27 tools across ... masks ... plus context stubs" to current 22 (vocab/edit 5 + versioning 6 + rendering 3 + ingest 3 + context 5). - docs/testing.md: bumped "27 shipped tools" → 22, replaced `tone_lifted_shadows_subject` example with `gradient_top_dampen_highlights`, dropped the dead Masks tool category, rewrote the mask-bound coverage line around `apply_with_drawn_mask` and the spatial-difference assertion. - docs/concept/02-project-concept.md § 10.1: rewrote the "configure a masking provider beyond the bundled coarse default" line; there is no bundled default. Phase 4 framed as drawn-form geometry, not PNG providers. - docs/IMPLEMENTATION.md Slice 4: prepended a "Status note (2026-05-03) — superseded by ADR-076" callout. Historical narrative preserved as build record. Append-only ADRs (partial-supersession status markers, mirroring what we did for ADR-021/022/055/057/058/074): - ADR-056 (MCP tool surface): noted the 5 mask tools and `mask_override` arg are gone; surface is 22 not 27. Error-contract rules still in force. - ADR-070 (CLI framework: Typer): noted the `masks` sub-command group is gone; CLI now has 1 sub-app (vocab) not 2. Typer choice stands. Coverage gap (test integrity): - tests/unit/mcp/tools/test_vocab_edit.py: added test_apply_primitive_with_mask_spec_routes_through_drawn_mask (proves the mask_spec branch in vocab_edit.py:_apply_primitive injects darktable:masks_history into the snapshotted XMP) and test_apply_primitive_with_invalid_mask_spec_returns_masking_error (malformed spec → MASKING_ERROR, not a stack trace). - tests/integration/cli/test_cli_edit.py: added test_apply_primitive_with_mask_spec_routes_through_drawn_mask using `--pack expressive-baseline` and `gradient_top_dampen_highlights`. Mirrors the MCP coverage at the CLI dispatch layer. Both branches now have explicit unit/integration coverage that doesn't require real darktable. The e2e tests in tests/e2e/expressive/test_mask_bound_entries.py continue to prove the binding actually shapes pixels. CI: 440 unit + 206 integration + all gates green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 76b0cb7 commit 5937d7a

10 files changed

Lines changed: 122 additions & 17 deletions

File tree

docs/CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ Some sliders have minimum granularity that prevents authoring exact zero values.
240240

241241
Starting v0.3.0, vocabulary packs are loaded by `chemigram.core.vocab.VocabularyIndex` against a per-pack `manifest.json`. A pack is a directory whose root contains `manifest.json` plus a tree of `.dtstyle` files (the layout under `vocabulary/starter/` follows `layers/L1`, `layers/L2`, `layers/L3` per the architecture doc).
242242

243-
The manifest's top-level shape is `{"entries": [...]}`. Each entry object follows the contract in `docs/adr/TA.md` `contracts/vocabulary-manifest`. Required fields per entry: `name`, `layer`, `path`, `touches`, `tags`, `description`, `modversions`, `darktable_version`, `source`, `license`. Optional: `subtype`, `mask_kind`, `mask_ref`, `global_variant`, `applies_to`. L1 entries must include `applies_to: {make, model, lens_model}` (exact-match per ADR-053).
243+
The manifest's top-level shape is `{"entries": [...]}`. Each entry object follows the contract in `docs/adr/TA.md` `contracts/vocabulary-manifest`. Required fields per entry: `name`, `layer`, `path`, `touches`, `tags`, `description`, `modversions`, `darktable_version`, `source`, `license`. Optional: `subtype`, `mask_spec`, `global_variant`, `applies_to`. L1 entries must include `applies_to: {make, model, lens_model}` (exact-match per ADR-053). Mask-bound entries declare a `mask_spec` of shape `{"dt_form": "gradient" | "ellipse" | "rectangle", "dt_params": {...}}` per ADR-076; the v1.4-and-earlier `mask_kind` / `mask_ref` fields were retired in v1.5.0 along with the PNG-mask path.
244244

245245
The index validates eagerly: the dtstyle file must exist relative to `pack_root`, must parse cleanly, and the user-authored plugin's `<operation>` must appear in the manifest entry's `touches` list. Mismatches are `ManifestError` with the offending entry name in the message — fix the manifest, not the parser.
246246

docs/IMPLEMENTATION.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,11 @@ Phase 1 is decomposed into six slices. Slices roughly follow dependency order
169169

170170
---
171171

172-
### Slice 4 — Coarse masking + local adjustments ✅ shipped (v0.4.0)
172+
### Slice 4 — Coarse masking + local adjustments ✅ shipped (v0.4.0); architecturally superseded (v1.5.0)
173173

174-
**Scope (shipped):**
174+
> **Status note (2026-05-03):** the substance of this slice — the `MaskingProvider` Protocol, `CoarseAgentProvider`, the PNG mask registry, the five mask MCP tools, the `mask_kind`/`mask_ref` schema, the `mask_override` apply argument — was all retired in v1.5.0 (ADR-076) when we discovered darktable doesn't read external PNG raster masks. The drawn-mask path (path 4a, v1.4.0) replaced it, validated end-to-end against real darktable. The historical scope below is preserved as build-record; for current architecture see ADR-076 and `concept/04-architecture.md` § 6.
175+
176+
**Scope (shipped in v0.4.0; later superseded):**
175177

176178
- `chemigram.core.masking``MaskingProvider` Protocol + `MaskResult` (ADR-057, #17).
177179
- `chemigram.core.masking.coarse_agent.CoarseAgentProvider` — sampling-based bundled default per ADR-058. Pillow rasterizer for bbox + polygon descriptors (#17).

docs/adr/ADR-056-mcp-tool-surface.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-056 — MCP tool surface: parameter shapes and error contract
22

3-
> Status · Accepted
3+
> Status · Accepted; partially superseded by ADR-076 (2026-05-03) — the 5 mask tools (`generate_mask`, `regenerate_mask`, `list_masks`, `tag_mask`, `invalidate_mask`) and the `apply_primitive(mask_override=…)` parameter were removed in v1.5.0; the surface is now 22 tools, not 27. The error-contract and parameter-shape rules in this ADR remain in force for the surviving tools.
44
> Date · 2026-04-29
55
> TA anchor · /components/mcp-server, /contracts/mcp-tools
66
> Related RFC · RFC-010 (closes); supersedes the implementation note in ADR-033

docs/adr/ADR-070-cli-framework-typer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-070 — CLI framework: Typer
22

3-
> Status · Accepted
3+
> Status · Accepted; partially superseded by ADR-076 (2026-05-03) — the `masks` sub-command group (list/generate/regenerate/tag/invalidate) was removed in v1.5.0; the CLI now has 1 sub-command group (`vocab`), not 2. Typer remains the framework choice and the rest of this ADR's reasoning stands.
44
> Date · 2026-05-03
55
> TA anchor ·/components/cli
66
> Related RFC · RFC-020 (closes here)

docs/adr/TA.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Drawn-mask serialization for vocabulary entries that shape only part of the fram
7979

8080
Adapts subsystems 1–4 as agent-callable tools. Thin layer.
8181

82-
**Files (shipped v0.3.0):** `src/chemigram/mcp/server.py` (bootstrap + stdio), `src/chemigram/mcp/registry.py` (tool registry + `ToolContext`), `src/chemigram/mcp/errors.py` (`ToolResult` / `ToolError` / `ErrorCode`), `src/chemigram/mcp/_test_harness.py` (in-memory client/server harness), `src/chemigram/mcp/tools/*` (27 tools across vocab/edit, versioning, rendering, ingest, masks, plus context stubs).
82+
**Files:** `src/chemigram/mcp/server.py` (bootstrap + stdio), `src/chemigram/mcp/registry.py` (tool registry + `ToolContext`), `src/chemigram/mcp/errors.py` (`ToolResult` / `ToolError` / `ErrorCode`), `src/chemigram/mcp/_test_harness.py` (in-memory client/server harness), `src/chemigram/mcp/tools/*` — 22 tools as of v1.5.0 across vocab/edit (5), versioning (6), rendering (3), ingest (3), context (5). The 5 mask tools (`generate_mask`/`regenerate_mask`/`list_masks`/`tag_mask`/`invalidate_mask`) shipped in v0.3.0–v0.4.0 and were removed in v1.5.0 per ADR-076 — see CHANGELOG.
8383

8484
**Tool surface:** see TA/contracts/mcp-tools.
8585

docs/concept/02-project-concept.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ Putting it all together — what a photographer experiences from first install t
437437

438438
Install Chemigram. Initial setup creates `~/.chemigram/` with empty `taste.md`, default `config.toml`, isolated darktable configdir. Photographer spends an hour writing initial `taste.md` — what they can articulate about their preferences. Imperfect; fine.
439439

440-
Optionally configure a masking provider beyond the bundled coarse default. For production-quality work on subjects, install `chemigram-masker-sam` sibling project.
440+
Note that v1.5.0 ships drawn-form geometric masks (gradient / ellipse / rectangle) baked into vocabulary entries' `mask_spec`. There is no AI-driven masker today; subject-precise content-aware masking is Phase 4 work via a sibling project (working name `chemigram-masker-sam`) that will produce darktable drawn-form geometry, not PNG bytes. ADR-076 documents the architecture and supersedes the v1.4-and-earlier PNG-mask path.
441441

442442
### 10.2 First session — Mode A on a single image
443443

docs/testing.md

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Tests live in three directories, codified in ADR-036, refined here with concrete
103103
- Every render code path: preview, export, with/without masks, different sizes.
104104
- Full MCP session shapes: ingest → bind → apply → render → export → snapshot.
105105
- Reset / branch / checkout semantics: each followed by a render to confirm the workspace state is intact.
106-
- Mask-bound primitives: real mask materialization → real render → assert the masked region differs from the unmasked region.
106+
- Mask-bound primitives: real `apply_with_drawn_mask` synthesis → real render → assert the masked region differs *spatially* from a same-dtstyle uniform render (path 4a; v1.4.0+).
107107

108108
**Doesn't belong:**
109109
- Anything that doesn't ultimately validate pixels or filesystem state from a real darktable invocation.
@@ -119,7 +119,7 @@ These are the rules a PR must satisfy before it lands.
119119
- One unit test per input-validation path (schema rejects malformed input).
120120
- One integration test per error code the tool can return (`recoverable=True` errors all need at least one test).
121121
- One integration test for the happy path through `in_memory_session`.
122-
- If the tool produces side-effects on the filesystem (snapshot, mask, transcript): one integration test that asserts the post-state.
122+
- If the tool produces side-effects on the filesystem (snapshot, transcript): one integration test that asserts the post-state.
123123
- If the tool drives a render: one e2e test that asserts on the rendered pixels.
124124

125125
### When you add a vocabulary primitive
@@ -165,11 +165,11 @@ Every shipped primitive needs end-to-end pixel validation. The 5 starter entries
165165
| `expo_-0.5` | L3 | luma < `expo_+0.5` (relative ordering) |
166166
| `wb_warm_subtle` | L3 | warmth ratio (R+G)/(2B) increases |
167167
| `look_neutral` | L2 | composite renders without errors; specific channel asserts TBD |
168-
| `tone_lifted_shadows_subject` | L3 raster-mask-bound | masked region's shadow tones lifted vs unmasked baseline |
168+
| `gradient_top_dampen_highlights` | L3 drawn-mask-bound | masked region (top half) reads dimmer than uniform application |
169169

170170
### MCP tool surface
171171

172-
All 27 shipped tools need:
172+
All 22 shipped tools need:
173173
- Unit input-validation coverage
174174
- Integration round-trip coverage through `in_memory_session`
175175
- E2e coverage for any tool that drives a render or mutates filesystem state
@@ -180,7 +180,7 @@ Categories:
180180
- **Layer binding:** `bind_layers`, `unbind_layers`
181181
- **Rendering:** `render_preview`, `compare`
182182
- **Export:** `export`
183-
- **Masks:** `generate_mask`, `regenerate_mask`, `list_masks`, `tag_mask`, `apply_primitive(mask_override=...)`
183+
- **Drawn-mask apply:** `apply_primitive` for entries with `mask_spec` set (routes through `apply_with_drawn_mask`; e2e proves the binding shapes the rendered effect). Standalone mask MCP tools were removed in v1.5.0 per ADR-076.
184184
- **Context:** `read_context`, `propose_taste_update`, `confirm_taste_update`, `propose_notes_update`, `confirm_notes_update`, `log_vocabulary_gap`
185185
- **Lifecycle:** `ingest_workspace`
186186

@@ -194,13 +194,12 @@ Beyond per-tool coverage, the *combinations* matter:
194194
- snapshot → tag → reset → snapshot (the ADR-062 case)
195195
- branch → checkout → snapshot → checkout main → diff
196196
- tag (immutable) → re-tag (must fail)
197-
- snapshot → invalidate-mask → snapshot (mask reachability)
198197

199198
### Render paths
200199

201200
- Preview at multiple sizes (256, 1024, 4096)
202201
- Export at full resolution (`--hq true`)
203-
- Render with `apply_primitive(mask_override)` — masked region differs from unmasked
202+
- Render after `apply_primitive` with a `mask_spec`-bearing entry — masked region differs spatially from a same-dtstyle uniform render
204203
- Render after corrupt XMP — surfaced as `darktable_error`, not stack trace
205204

206205
### Context + session

tests/integration/cli/test_cli_edit.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,44 @@ def test_apply_primitive_json_emits_snapshot_hash(
9494
assert "state_after" in payload
9595

9696

97+
def test_apply_primitive_with_mask_spec_routes_through_drawn_mask(
98+
runner: CliRunner, cli_workspace_root: Path
99+
) -> None:
100+
"""An expressive-baseline mask-bound entry routes through the drawn-mask
101+
apply path; the resulting XMP carries masks_history. Coverage gate for
102+
the ``vocab_entry.mask_spec is not None`` branch in cli/commands/edit.py
103+
(the e2e suite proves the binding actually shapes pixels under real
104+
darktable; this proves the dispatch logic works without darktable).
105+
"""
106+
result = runner.invoke(
107+
app,
108+
[
109+
"--json",
110+
"--workspace",
111+
str(cli_workspace_root),
112+
"apply-primitive",
113+
"test-image",
114+
"--entry",
115+
"gradient_top_dampen_highlights",
116+
"--pack",
117+
"expressive-baseline",
118+
],
119+
)
120+
assert result.exit_code == ExitCode.SUCCESS.value, result.stdout + result.stderr
121+
payload = json.loads(result.stdout.strip().splitlines()[-1])
122+
assert payload["status"] == "ok"
123+
snapshot_hash = payload["snapshot_hash"]
124+
125+
from chemigram.core.versioning import ImageRepo
126+
127+
repo = ImageRepo(cli_workspace_root / "test-image")
128+
raw = repo.read_object(snapshot_hash)
129+
assert b"masks_history" in raw, (
130+
"drawn-mask path should inject darktable:masks_history into the XMP "
131+
"for entries with mask_spec (ADR-076)"
132+
)
133+
134+
97135
def test_apply_primitive_unknown_entry(runner: CliRunner, cli_workspace_root: Path) -> None:
98136
result = runner.invoke(
99137
app,

tests/unit/mcp/tools/test_vocab_edit.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,73 @@ def test_apply_primitive_unknown_primitive_returns_not_found(
111111
assert result.error.code == ErrorCode.NOT_FOUND
112112

113113

114+
def test_apply_primitive_with_mask_spec_routes_through_drawn_mask(
115+
context: ToolContext,
116+
) -> None:
117+
"""When entry.mask_spec is set, apply_primitive routes through
118+
apply_with_drawn_mask which injects masks_history into the XMP.
119+
120+
Coverage gate for the mask_spec branch in
121+
chemigram.mcp.tools.vocab_edit._apply_primitive — without this, only
122+
the e2e suite (requires real darktable) exercises that branch.
123+
"""
124+
from dataclasses import replace as dc_replace
125+
126+
base = context.vocabulary.lookup_by_name("expo_+0.5")
127+
assert base is not None
128+
masked = dc_replace(
129+
base,
130+
name="expo_top_gradient_test",
131+
mask_spec={
132+
"dt_form": "gradient",
133+
"dt_params": {
134+
"anchor_x": 0.5,
135+
"anchor_y": 0.5,
136+
"rotation": 0.0,
137+
"compression": 0.5,
138+
},
139+
},
140+
)
141+
context.vocabulary._by_name["expo_top_gradient_test"] = masked
142+
143+
result = _call(
144+
"apply_primitive",
145+
{"image_id": "test-image", "primitive_name": "expo_top_gradient_test"},
146+
context,
147+
)
148+
assert result.success is True, result.error
149+
snapshot_hash = result.data["snapshot_hash"]
150+
raw = context.workspaces["test-image"].repo.read_object(snapshot_hash)
151+
assert b"masks_history" in raw, (
152+
"drawn-mask path should inject darktable:masks_history into the XMP "
153+
"when mask_spec is set on the vocab entry (ADR-076)"
154+
)
155+
156+
157+
def test_apply_primitive_with_invalid_mask_spec_returns_masking_error(
158+
context: ToolContext,
159+
) -> None:
160+
"""Malformed mask_spec → MASKING_ERROR (not a stack trace through MCP)."""
161+
from dataclasses import replace as dc_replace
162+
163+
base = context.vocabulary.lookup_by_name("expo_+0.5")
164+
assert base is not None
165+
bad = dc_replace(
166+
base,
167+
name="expo_bad_mask_test",
168+
mask_spec={"dt_form": "not_a_real_form", "dt_params": {}},
169+
)
170+
context.vocabulary._by_name["expo_bad_mask_test"] = bad
171+
172+
result = _call(
173+
"apply_primitive",
174+
{"image_id": "test-image", "primitive_name": "expo_bad_mask_test"},
175+
context,
176+
)
177+
assert result.success is False
178+
assert result.error.code == ErrorCode.MASKING_ERROR
179+
180+
114181
# --- remove_module ------------------------------------------------------
115182

116183

vocabulary/starter/README.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Starter Vocabulary
22

3-
The vocabulary pack that ships with `pip install chemigram`. **Deliberately small**five entries, generic by design — so the agent loop works out of the box. Phase 2 grows the vocabulary from real session evidence, not from imagined upfront completeness; the gaps surfaced via `log_vocabulary_gap` are the seed for new entries.
3+
The vocabulary pack that ships with `pip install chemigram`. **Deliberately small**four entries, generic by design — so the agent loop works out of the box. Phase 2 grows the vocabulary from real session evidence, not from imagined upfront completeness; the gaps surfaced via `log_vocabulary_gap` are the seed for new entries.
44

55
See `docs/concept/04-architecture.md` § 5 (layer model) for the L1/L2/L3 architecture, and `docs/prd/PRD-003-vocabulary-as-voice.md` for the design rationale.
66

@@ -12,9 +12,8 @@ See `docs/concept/04-architecture.md` § 5 (layer model) for the L1/L2/L3 archit
1212
| `expo_-0.5` | L3 | exposure || Lower exposure -0.5 EV (global) |
1313
| `wb_warm_subtle` | L3 | temperature || Subtle warm white balance shift |
1414
| `look_neutral` | L2 | exposure + temperature || Neutral L2 baseline (exposure + warm-subtle WB) |
15-
| `tone_lifted_shadows_subject` | L3 | exposure | raster (subject) | Shadow lift restricted to the subject mask |
1615

17-
The mask-bound entry references `current_subject_mask`the agent generates this via `generate_mask(image_id, target="subject")` before applying.
16+
Mask-bound starter entries are out for now: the original `tone_lifted_shadows_subject` was retired in v1.5.0 alongside the PNG-mask architecture (ADR-076). Drawn-form mask-bound entries (gradient / ellipse / rectangle) live in the `expressive-baseline` pack; the starter pack stays minimal until evidence motivates a starter-level mask-bound primitive.
1817

1918
## What's not shipped
2019

0 commit comments

Comments
 (0)