Skip to content

fix(schema): count the blast radius in the narrowing confirmation - #698

Merged
JArmandoAnaya merged 5 commits into
mainfrom
fix/narrowing-blast-radius
Aug 19, 2026
Merged

fix(schema): count the blast radius in the narrowing confirmation#698
JArmandoAnaya merged 5 commits into
mainfrom
fix/narrowing-blast-radius

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

Summary

The schema editor's narrowing confirmation named the classes it was about to narrow and then
asked for a decision without saying what the decision cost. It now states the blast radius as a
count, which is what DESIGN.md's rule for destructive confirmations asks for: how many classes
narrow, which ones, and that nothing already labeled is invalidated by publishing.

Alongside that, three prose surfaces still described a design the editor no longer has. Since the
editor began previewing a proposed narrowing before changing the draft or publishing, docs/ui.md
and the SchemaEditor file docstring both continued to assert — with reasoning attached — that the
editor does not call POST .../schema/preview and that making the publish's own refusal legible,
rather than pre-empting it, was the editor's whole job. Both now describe the shipped flow. A third
copy of the same claim lived in DatasetScreen's docstring, which used the schema editor as its
example of attempt-shaped consent for export; export still works that way, so that sentence keeps
its point and loses the comparison that had gone stale.

What the confirmation says now

This narrows the schema
1 class narrows: “lane”.
No existing annotation becomes invalid — that is why this can be published at all. Publishing
adds a new version; earlier versions keep what they declared.

Two properties of that copy are deliberate and worth stating, because both are easy to undo by
accident.

It is agnostic to which kind of destructive change happened. An earlier draft of this change
said no annotations use what this removes, which reads well and is wrong: ChangeKind.DESTRUCTIVE
covers seven shapes, and only some are removals. Adding a required attribute is destructive, and the
class-detail panel ships that checkbox, so two clicks produced a dialog telling somebody that nothing
used what they had removed when they had removed nothing. The tempting repair — no annotations use
the classes this changes
— is false in a different case: a class carrying five hundred boxes can
drop an unused polygon geometry with no blockers at all, because orphanable_shapes scopes a
geometry change to that one shape. No existing annotation becomes invalid is true in all seven.

It says nothing about what becomes of annotations after the publish. The version this dialog
publishes is judged against the active version, while annotation writes are judged against the
batch's own pin, so an open batch pinned to the outgoing version can still write a class the new
active version does not declare. Which invariant the kernel enforces there is an open question
(#585), and the sentence this change removed — existing annotations are not touched — answered it
by implication. The replacement states only what the preview measured, in the present tense.

Scope

This is part of the class-removal flow work (#588), which stays open. Two of that issue's
requirements are untouched here. It asks the terminal orphan dialog to link to the annotations
filtered to the blocking class, and no endpoint accepts a label-class filter today, so that needs a
wire and information-architecture change of its own before the UI can offer it. It also asks that
refusals reaching the UI render through the shared code-to-prose vocabulary; two Alerts on this
screen still use the raw error code as their title, and the same pattern appears three times in
AnnotationPage, which makes it a product-wide Alert convention rather than a defect local to the
schema editor.

The narrowing refusal's grain narrowed when a geometry drop began being refused only where
annotations carry that shape (#592), and the copy here is written to that grain.

Test plan

Full bash scripts/check.sh, run in stages because the browser suites exceed this environment's
command ceiling. Every group green at d09b575:

check.sh: PASSED  ran=python,generated,frontend,docs  skipped=browser
     32s  python tests
      0s  ruff (lint)
      0s  ruff (format)
      0s  mypy
      0s  import contracts
      1s  openapi drift
      1s  generated client drift
      0s  mcp tool reference drift
      1s  version sync
      4s  frontend build
     25s  frontend tests
     10s  frontend lint
      2s  docs site build
      0s  docs projection is deterministic
      1s  docs site internal links
     77s  total
check.sh: PASSED  ran=browser  skipped=python,frontend,generated,docs
  278 passed (40.1s)
  1 passed (25.9s)
     41s  annotator + app e2e (chromium)
     26s  browser cycle, real server (chromium)
     67s  total

ui-core is 1,149 tests over 53 files, one more than before this branch. The added test covers the
plural branch of the count, and it was verified by breaking it rather than by watching it pass:
mutating the singular/plural ternary so both arms read class narrows produces

AssertionError: expected 'This narrows the schema2 class narrow…' to contain '2 classes narrow'
      Tests  1 failed | 1148 passed (1149)

The machine was idle throughout (load average 0.58 across 20 cores), so no suite was substituted for
another and no staged-gate fallback applies.

Found, not fixed

frontend/ui-core/src/screens/screens.test.tsx carries an assertion that nothing can fail:
not.toContain("internal wording must not appear") checks for a string no code path emits. It
predates this branch and sits in a block this change touches, which is the only reason it is
mentioned here.

A Radix focus-scope timeout in the ui-core suite failed once during this work and did not
reproduce across four subsequent full runs of the same commit. It is unrelated to a change that
edits copy strings, and no issue tracks it yet.

origin/main moved while this branch was in progress, gaining a cycle-test change (#696). It
touches frontend/app/cycle/ only, which this branch does not, and git merge-tree reports a clean
merge. The branch is not rebased, so the gate output above describes it as it stands.

The narrowing confirmation dialog asked to confirm without saying what
confirming costs. It now states how many classes narrow and that zero
annotations use what is removed — a measured number, since blockers
is always empty on a preview that reaches this dialog. Drops the
'existing annotations are not touched' line, which promised something
about post-publish annotation state the kernel has not decided.
The SchemaEditor docstring and docs/ui.md both asserted the editor does
not call schema/preview and that publishing's 409 is the sole way a
narrowing refusal surfaces. The editor calls preview twice, before a
class leaves the draft and before a publish, so both surfaces now
describe that flow and how it relates to the publish's own 409.
…t shape

DatasetScreen.tsx's file docstring still described the schema editor as
attempt-shaped for export consent, the same claim already corrected in
docs/ui.md and SchemaEditor.tsx. The schema editor previews first, so
DatasetScreen's docstring now says the same thing they do.
…ribute case

The dialog said 'no annotations use what this removes,' which is false
when the destructive change is an added required attribute rather than
a removal. Replace with kind-agnostic copy that states the measured
fact for all four destructive shapes, and update docs/ui.md and a
DatasetScreen docstring that referenced the old phrasing or leaned on
past-tense/forward-looking language.
@JArmandoAnaya
JArmandoAnaya merged commit da3304a into main Aug 19, 2026
15 checks passed
@JArmandoAnaya
JArmandoAnaya deleted the fix/narrowing-blast-radius branch August 19, 2026 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant