Surface per-image mask ids so pre-existing masks become deletable - #91
Merged
Conversation
Fixes #77. Global and category masks each have a dedicated listing endpoint returning id (GET /api/masks, GET /api/categories/<c>/masks); per-image masks didn't, so a mask created outside the current browser session had no id to delete it by -- the viewer could only track delete-ability for masks created in the current session, via a client-side createdMasks array. Adds GET /api/runs/<run_id>/snapshots/<name>/masks/own, mirroring the existing per-scope listing pattern (same scoping predicate the DELETE endpoint already uses: name + viewport). Wires it into the viewer's resolveMaskIds() as a 4th id-bearing pool alongside session-created, global, and category masks, at both call sites (the chip list and InteractiveImagePane's overlay). createdMasks itself is left as-is -- partially redundant now, but a shared three-scope mechanism where untangling just the per-image third is a separate, more invasive change than this fix requires. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrQeh9CXrigN1cvpJuyzy4
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.
Fixes #77.
Summary
id(GET /api/masks,GET /api/categories/<category>/masks). Per-image masks didn't — the only listing touching them,GET /api/runs/<run_id>/snapshots/<name>/masks, returns the merged (global ∪ per-image ∪ category) view withidstripped by design. The DELETE endpoint for per-image masks already existed and already scoped correctly; the only gap was that a client had no way to learn a pre-existing per-image mask'sid, so the viewer could only offer a delete button for masks created in the current browser session (tracked client-side).GET /api/runs/<run_id>/snapshots/<name>/masks/own, mirroring the existing per-scope listing pattern exactly (same scoping predicate the DELETE endpoint already uses:name+viewport). No changes torender.py/applicable_masks()/compare().resolveMaskIds()as a 4th id-bearing pool (session-created, global, category, now own-per-image), at both call sites — the mask-chip list andInteractiveImagePane's overlay.createdMasks(session-tracking) is deliberately left untouched — architect-reviewed call: it's a shared three-scope mechanism, and untangling just the per-image third is a separate, more invasive change than this fix requires.Design went through an
architectSOLID review (SOUND, no concerns) before implementation, and the build went throughpropose-review(proposer → plan-reviewer → implementer + test-author → code-reviewer), with independent APPROVE verdicts at each gate.Test plan
backend/.venv/bin/pytest backend/tests -q— 145 passed, including a new test that creates masks in global/category/other-viewport scopes alongside the target per-image mask and asserts/masks/ownreturns only the one per-image mask, withid, plus two 404 testsnpm test -w viewer— 56 passed;npm run lint -w viewer— clean/masks/own(never added tocreatedMasks) still gets a working delete control — confirmed to fail without the fix (git stashregression check)npm run test:e2e -w examples/demo-app— 17/17 passed (new test added)fetch(not a live browser draw, genuinely bypassing session-tracking), confirms a delete control appears in a fresh browser navigation and a realDELETEround-trips against the real backend — confirmed to fail without the fixdocs/API.md,backend/CODEMAP.md,viewer/CODEMAP.md,examples/demo-app/CODEMAP.mdupdated🤖 Generated with Claude Code
https://claude.ai/code/session_01NrQeh9CXrigN1cvpJuyzy4