refactor(naming): settle suspend/hold split; rename revise->update, arrive->record-arrival - #318
Merged
Merged
Conversation
The reversible-pause verb is suspend on grant-shaped aggregates (Agent, Permit) and hold on execution/container aggregates (Run, Campaign, Visit). A naming audit flagged this as a possible inconsistency; investigation of the code semantics and the standards corpus (license/account suspension, X.509, PackML Held, Bluesky pause) showed the split tracks a real axis: the pause verb pairs with the aggregate's terminal verb (suspend with revoke/deprecate, hold with abort/stop/close). All five share the resume recovery verb, which is the symmetry that matters. Document the rule so it reads as intentional, and add a guard to the naming-r3-reviewer so the coexistence is not re-flagged as an R2 family-symmetry violation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A naming audit flagged "revise" as a lone synonym for the corpus's
dominant field-mutation verb "update": this slice has the exact
update_<subject>_<field> shape (set/replace/clear with PUT
semantics) as update_asset_settings, update_method_launch_spec, and
the rest of the update family, but used a unique verb.
Rename the slice, command (UpdateAgentBudget), MCP tool, event
(AgentBudgetUpdated, to match the family's *Updated events), error
(AgentCannotUpdateBudgetError), and all prose to join the update
family. The REST URL was already noun-based (POST /agents/{id}/budget)
and is unchanged. openapi.json regenerated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A naming audit flagged "arrive" as the one intransitive verb in the
Visit lifecycle slice family: "arrive a visit" breaks the transitive
verb-subject skeleton every sibling follows (start/hold/resume/
complete/cancel/abort/void a visit). The operator action is an
explicit gesture (the docstrings call it that), so "record arrival"
reads cleanly and transitively.
Rename the slice, command (RecordVisitArrival), MCP tool, route
function, and URL (POST /visits/{visit_id}/record-arrival, matching
the kebab multi-word style of /enter-maintenance). The event
VisitArrived, status Arrived, arrived_at column, and the
VisitCannotArriveError transition guard keep the arrival-fact
vocabulary (command verb differs from event past-participle, as with
StartRun/RunStarted). AuthZ matrix doc + Visit command-name lock
test updated; openapi.json regenerated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
xmap
added a commit
that referenced
this pull request
Aug 2, 2026
…kind (#602) * Make hold claims cause-scoped so a release cannot drop another concern's hold RunStatus.HELD was a single anonymous bit. Both gate holders guarded `status is RUNNING` and folded every other status to a silent no-op, so a hold arriving at an already-HELD run appended NOTHING to the Run stream. `last_hold_placed_by_enforcer` then scanned backward for the most recent RunHeld and compared its envelope principal, which answers "did I place the most recent hold" and is the right question only when holds cannot overlap. The outcome therefore depended on arrival order: - kill-switch first: the gate's hold is dropped, the latest RunHeld is the kill-switch's, the grant does not resume. Correct, and the only order covered (test_release_does_not_clear_a_foreign_hold seeds it first). - consequence gate first, revocation arriving during the co-signature wait: the REVOCATION is dropped, the latest RunHeld is the gate's own, and RatificationGranted resumes the run with the revocation unenforced. The second case does not recover. AuthorityRevocationHolder subscribes to PolicyGrantRevoked only, re-delivery re-derives the same Decision ids and no-ops, and there is no sweep, so the kill-switch never fires again for that revocation. RunSupervisor issues commands as its own pinned principal, so the revoked grant does not gate continued supervision of the resumed run. The window is a human co-signature wait, so it is wide by design, not a race. Fix: holds carry their own identity and cause, and a run resumes only when no claim remains. - RunHeld gains claim_id + cause; RunResumed gains released_claim_id. - New HoldClaimReleased discharges one claim while others remain, leaving the run HELD. Load-bearing: one event cannot both clear a claim and decide for the concerns still holding, so without it a releaser must either resume a run others want held or hold forever. - Run.hold_claims folds the claims off the same stream. Nothing stores them. - Claim ids are derived, uuid5(ns, "run|cause"), one per (run, cause), so holder and releaser agree without a lookup and re-delivery is idempotent. This keeps the same-cause collapse that already existed. - hold_run admits RUNNING or HELD, enforcing alternation per CLAIM, which is what the PackML / Bluesky strict-not-idempotent precedent protected. - resume_run has three outcomes: resume when last, HoldClaimReleased when others remain, RunHoldClaimsRemainError (409) naming the blockers when the caller holds nothing. Legacy streams replay unchanged: a claimless RunHeld folds to a fixed LEGACY_CLAIM_ID, a bare RunResumed clears every claim, and the operator's resume owns the legacy claim. Without that, every run HELD at ship time would have been permanently unresumable. Seven existing tests had pinned the faulty behaviour as correct, three asserting the defective outcome outright, so this is a corrected rule rather than a filled coverage gap. Those are updated and the ordering regression is covered in both orders end to end. Verified: unit + architecture 42,488 passed; integration 1,099 passed; pyright src tests strict 0 errors; ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Say once whether a sub-concept command carries its aggregate Two rules governed the same name and disagreed. The R7 logbook-entry family table said the append command is `Append<EntryNoun>s`, bare. This page said sub-concept commands keep the aggregate qualifier, and cited `AppendProcedureActivities` as its exemplar. The codebase honoured both: Operation shipped the qualified form on three appenders, Decision and Run shipped the bare form on two. Whichever doc the next author happened to read would decide the next appender. The qualified form wins, on evidence rather than precedence. A slice directory and an MCP tool name are read from inside a BC, so the path around them already supplies the aggregate. A command class name does not stay there: `_COMMAND_NAME` is a flat label written into the event envelope, the OTel span name, and the idempotency cache key, where 138 labels share one namespace and nothing nearby says which BC produced it. The same test explains why the entry class went the other way and kept its bare noun, so the two rules stop looking like a coin flip. Records the two open renames rather than making them here, so the rule and its debt land separately. * Make a command prove it can still name its own event `RegisterAsset` emits `AssetRegistered`; move the verb to the end, past-participle it, keep every other token. 191 of 210 single-event command slices already did this. Nothing said so and nothing checked, which the naming audit demonstrated the hard way: renaming an event class and leaving its command behind passed all 29,741 architecture tests without a murmur. A reader who knows either name should know the other, and until now only luck was keeping that true. The nineteen exceptions are split across two allowlists on purpose. `_SANCTIONED_DEVIATIONS` holds the sixteen that are right as they stand: the entry-append slices whose only event is the lazy logbook marker, the preposition phrases, the pairs where the event must name the stream owner while the command names the object. `_KNOWN_DRIFT` holds the three the audit judged wrong, each carrying the rename it wants. One list would have let those three sit behind a plausible sentence and stop looking like work. Both lists fail when an entry starts deriving cleanly, so a rename prunes its own bookkeeping instead of leaving a dead line behind. * Return the Visit arrival pair to the sanctioned list The 2026-06-22 slice audit already tried this one. PR #318 renamed `arrive_visit` to `record_visit_arrival` because `arrive` was the single intransitive verb in the Visit lifecycle family, and kept `VisitArrived` deliberately: the event states the arrival fact, the command states the act of recording it, exactly as `StartRun` emits `RunStarted`. Filing it as drift would have sent a future reader to undo a decision that was made on purpose, which is the failure the two-list split exists to prevent. Known drift is down to two, both genuinely unadjudicated. * Let the Agent target plan be updated like every other field `AgentTargetPlanSet` was the corpus's only `Set` against ten `Updated` events, and its payload is the same shape as all of them: aggregate id, the new value, occurred_at. No reason string, no actor denorm, nothing that marks it as a different kind of act. It read as a synonym because it was one. Follows the `revise_agent_budget` to `update_agent_budget` rename from PR #318 exactly, down to the error class taking the verb with it: `AgentCannotSetTargetPlanError` becomes `AgentCannotUpdateTargetPlanError`, keeping the per-verb error shape. The two Agent field-updates now read as siblings rather than as two unrelated operations that happen to touch the same aggregate. REST URL `POST /agents/{agent_id}/target-plan` is unchanged: it was already noun-only, so the verb never appeared in the path. Docstrings and the MCP tool description move from "set" to "update" so the vocabulary does not fork between the code and what an operator or an agent reads. * Let the Allocation ceiling be updated like every other field `AllocationCeilingAmended` was the corpus's only `Amended`, and its payload is the ten-strong `Updated` family's exactly: aggregate id, the new value, occurred_at. PUT semantics, no reason, no actor denorm. "Amend" carried a suggestion of formality the event does not actually encode. Safety's `amend_clearance` keeps its verb and is untouched. That one earns it: amending a Clearance emits TWO events across two streams, registering a child and superseding the parent, which is a different act from replacing a number in place. The distinction is now visible in the vocabulary instead of being blurred by a shared word. `AllocationCannotAmendCeilingError` follows to `AllocationCannotUpdateCeilingError`, and the prose in the slice, the aggregate, the projection SQL constant, the sealer subscriber and the tests moves with it, so a grep for "amend" in Budget now returns nothing rather than half a vocabulary. REST URL `POST /allocations/{allocation_id}/ceiling` is unchanged; it was already noun-only. * Let the version field say which aggregates are templates Ten aggregates are versionable templates, and all ten carry a `version` field on the root while emitting the full Defined -> Versioned -> Deprecated triad. Ten out of ten, both directions, no exceptions. The field is already the machine-readable marker of the archetype; it just was not doing the job. This closes the gap `test_event_class_defined_vs_registered.py` left open, but not the way that file predicted. Its docstring defers a direction-enforcing companion "until a rule-of-three misclassification fires", expecting to need a hand-maintained semantic map. Checking the corpus, the rule of three has NOT fired: eight aggregates say `Defined` without being versionable and seven have solid reasons (LanguageModel is a vendor product we cannot version, Calibration versions through revisions, Role derives its id from its name so a bump would mint a new identity, and Conduit / Policy / Surface / Zone have no version axis at all). One arguable case is not three. So `Defined` keeps its broader meaning of "declared artifact", and the structural signal picks out the versionable kind with no map to maintain. Pins four things the genesis-suffix rule cannot see: the field and the event imply each other, a versionable template is Defined rather than Registered, it can also be Deprecated, and deprecating without a version axis needs an allowlist entry. LanguageModel is the only such entry and is guarded against going stale. * Make a deprecated Recipe template say whether prior data still stands Deprecation is the one terminal whose reason changes how a reader must treat data that ALREADY EXISTS. A Method retired because a better one landed leaves every prior Run standing. A Method retired because it was subtly wrong makes every prior Run suspect. Same event, same verb, opposite consequences, and until now the record could not tell them apart: seven of the ten versionable templates recorded nothing at all. `DeprecationReason` is closed rather than free text, following `CautionRetireReason`, whose docstring records the same rejection: operators already pick from a small mental list. A required prose field becomes "old" / "n/a" by its third use, which leaves a column that looks like signal and is not. The three values partition on the consequence for prior data, not on the operator's narrative, which is what makes "show me every Run whose Method was later found Defective" a query the record can actually answer. Capability and Recipe keep `replaced_by_<x>_id` beside the new field. The pointer answers "what do I use instead" and the reason answers "should I distrust what I already have"; they are different questions and the LOINC MAP_TO precedent behind the pointer is worth keeping. The event carries the primitive and the command carries the enum, so Pydantic rejects an unknown value at the edge with a 422 and the payload stays JSON-plain. Five slices here; the other six templates follow. * Retire the free-text deprecation reason in Equipment Family recorded nothing when deprecated. Assembly and Model recorded operator prose, Model behind a dedicated `ModelDeprecationReason` value object with its own bounded-text error class. Three aggregates in one BC, three answers to the same question. All three now carry `DeprecationReason`. The prose loses more than it looks: `ModelDeprecated`'s own docstring offered "vendor EOL 2026" as its worked example, and that string is no longer accepted. That detail was never the part a reader needed. What they needed was whether Assets already bound to the Model are still trustworthy, and prose could not be queried for it. Vendor specifics belong on the Model's own fields or a Caution, both of which are built to carry them. `ModelDeprecationReason` and `InvalidModelDeprecationReasonError` are gone, along with the error-handler registration and the decider tests and properties that exercised empty, whitespace-only, and over-long input. A closed enum makes all three impossible at the type level, so those tests asserted a guard that no longer exists rather than a behaviour that could regress. The 400-on-whitespace contract test becomes 422-on-unknown-value, the same intent under the new contract. * Put back the reasons a deprecation sweep overwrote The Equipment commit ran a find-and-replace over every test file that mentioned "deprecat" anywhere, which is a keyword, not a scope. It rewrote 61 files that have nothing to do with deprecation. Three kinds of damage, in rising order of how badly the suite hid them. Most had free-text reasons overwritten, so a Run was now aborted because it was "Superseded" and a Caution retired for the same reason. Five had deliberately-invalid fixtures made valid, which is how a 422-on-empty-reason test ends up asserting against a perfectly good reason. Four had a `reason` argument INJECTED into tools that take none, including `get_family`, which is a read-only query. Only fifteen of them failed. That is the part worth keeping: a free-text field accepts any string, so overwriting one is invisible to a green suite. The tests that broke were the ones checking that bad input is rejected, and they broke precisely because the sweep handed them good input. Restored from the commit before. Files that legitimately changed are untouched, verified mechanically rather than by eye: every remaining added `reason` line sits inside a call whose name contains "deprecat", and pyright catches any construction still missing the new field. * Require a deprecation reason on the last three templates Agent, LanguageModel and ClearanceTemplate close the set: all ten versionable templates now record why they stopped being recommended, plus LanguageModel, which is deprecated the same way without being versionable. `AgentDeprecated` was the family's only optional reason, so the fold changes with it: `Agent.deprecation_reason` was an `AgentDeprecationReason` value object and is now a plain string, and the two `get_agent` DTO mappings that unwrapped `.value` get simpler. Optional was the wrong shape here. A reader asking whether Runs debriefed by this Agent are still trustworthy cannot act on "the operator declined to say". Two neighbouring value objects survive on purpose. `LanguageModelReason` still serves `announce_language_model_retirement` and `retire_language_model`, where free text is right because a vendor can pull a model with no statement at all, and that event's docstring says so. `AgentDeprecationReason` still serves `suspend_agent`. Only the deprecation path moved. * Keep the next template from shipping a silent deprecation Eleven `<X>Deprecated` events now carry a required reason. The drift this guards against is not a rename an author would notice in review; it is the twelfth template shipping a `Deprecated` event with no reason at all, which is exactly how the first seven got there. Two things are pinned, and the second matters as much as the first. The field must exist and be `str`, and it must have no default. `AgentDeprecated` shipped as `str | None`, and optional is the wrong shape for this specific question: a reader asking whether data produced under this template is still trustworthy cannot act on "the operator declined to say". Where a cause genuinely may not exist the event is not a deprecation, and `LanguageModelRetired` keeps its optional free-text reason to prove the distinction is real. No allowlist, deliberately. A future `Deprecated` event that cannot name a reason is the first evidence the closed vocabulary is wrong, and that deserves a design conversation rather than a quiet exemption. Verified by breaking it: making `PlanDeprecated.reason` optional fails with the message pointing at the enum and the CautionRetired precedent. * Make an Asset say why it left service, like its four siblings Enclosure, Facility, Frame and Mount have carried a reason on decommission since they shipped. Asset, the biggest and most-used aggregate in the corpus, did not. So an operator could retire a mount and be made to explain, then retire the camera sitting on it and be asked nothing. Whether the record answers "why is this out of service" depended on which part of the floor you asked about. Free text here, not the closed `DeprecationReason` enum, and the difference is the point. Deprecation's reason decides whether data produced under a template is still trustworthy, which is a small closed question worth constraining. Why a physical unit left service is open-ended: broke, sold, obsolete, moved to another beamline. A three-value enum would force those into boxes that fit none of them. Matches what the four siblings already do. The parametrized evolver tests funnel every lifecycle transition through one `_extra_kwargs_for()` helper, so three files needed a line each rather than dozens of call sites. That helper exists because `AssetDecommissioned` was already the only transition carrying attribution, which is the same asymmetry that left it without a reason. * Make revoking an Agent's tool say why, like revoking a grant `AgentToolRevoked` recorded nothing. `PolicyGrantRevoked`, which does the same thing to a person, has always required a reason. Both remove a live capability from a principal; only one made anyone explain. The line this settles is not access-versus-structure, it is draft versus live, and the corpus already drew it. `EditionDatasetRemoved` stays silent and its docstring says why: pre-Sealed membership churn is exploratory editing, nothing has been claimed yet. `CampaignRunRemoved` requires a reason because ungrouping a live Campaign retracts a claim. Eleven structural removals (ports, wires, families, owners, identifiers) stay silent under that rule and should. Revoking a tool from an autonomous Agent mid-campaign does not: it narrows what that principal may do, while it is doing it. Free text, not the deprecation enum. What a template's retirement means for prior data is a small closed question; why an operator pulled a tool is not. Note for anyone reading the diff: `to_payload`'s arm for this event was written on one line where its neighbours span several, so the first attempt updated the class and `from_stored` and silently skipped serialization. That writes events whose payload has no reason and fails on every replay. The round-trip test caught it; the diff looked fine. * Act on the gate review: dead code, dead responses, dead claims A six-reviewer panel found three P0s and I verified each against the tree. Two were mine to fix here; the third was yours to decide and you chose to deviate on the record. `AgentDeprecationReason` had no constructors left in src, and its error class could not be raised yet was still mapped to a 400. I deleted Equipment's identical twin in this same branch and kept Agent's because I read a docstring cross-reference in suspend_agent as a call site. It was not one. `LanguageModelReason` genuinely does survive, with two live callers in the retirement slices, which is what made the asymmetry look plausible. Three endpoints published a 400 for "reason is empty / whitespace-only / over-cap after trim" long after the deciders that raised it were deleted. That reached the committed openapi.json, and from there SDK generators and the docs site. A closed enum cannot produce any of those conditions. Around fifteen docstrings asserted the opposite of their own code: "1-500 chars after trim" on enum fields, "no body at the API layer" on four commands that now require one, "trimmed at the VO" pointing at a VO this branch deleted. One MCP tool description told a calling agent to send prose that would 422. The schema-evolution deviation is now recorded where a reader will look for it, in both renamed aggregates' module docstrings: what modeling.md requires, that this deviates twice, why no compat arm exists (no such payload was ever written), and what spends the deviation. Also here, from the panel: the enum's stated axis contradicted its own extension gate and is rewritten; ClearanceTemplateWithdrawn now carries a reason, since removing a live safety template and freeing its code is the harder of that aggregate's two endings; the operationId that still said "set" after the rename now says "update"; and _UPDATE_CEILING_UPDATED_SQL loses the stutter its six *Updated siblings never had. Four integration tests were red at HEAD. I had been running three of five test tiers all session and calling it the full suite. * Put back the tests the enum did not justify deleting Deleting the free-text validation properties was right; the enum makes empty, whitespace-only and over-long impossible at the type level. I took seven more with them. `test_deprecate_model_decider_properties` went from six properties to one, losing not-found, cannot-deprecate and emits-exactly-one-event, none of which mention a reason. Agent and LanguageModel each lost their source-state TOTALITY property while their module docstrings kept promising it: "so a future status value cannot silently fall through". Nothing enforced that. `test_decider_changes_ require_paired_pbt` checks that the file exists, so a gutted file reads as covered. Restored with the reason drawn from the enum rather than hardcoded, so the properties exercise the closed vocabulary instead of one member. Twelve new contract tests: missing-reason and unknown-reason 422s on the six deprecate endpoints that had neither. Three slices already had the pair and the corpus precedent is `test_retire_caution_endpoint`, the existing closed-enum endpoint, which has both. Two degenerate tests are repurposed rather than deleted: one asserted 204 on a body it no longer sends, the other built an over-cap string out of an enum member. Fitness hardening, all from panel findings. `_derives` compared token multisets, so `AgentPlanTargetUpdated` derived as happily as `AgentTargetPlanUpdated` — order-blind, in a repo whose R3 rule is noun-LAST and which has been burned reading it backwards. Now positional, which immediately surfaced the two Visit phrasal verbs where the preposition genuinely moves; both sanctioned with that reason. The deprecation pin covered the event field but not the command annotation, where the vocabulary is actually closed, and had no vacuity guard: an empty collection would have collected zero cases and passed silently. * Catch a stored-name typo before it silently poisons a stream The write side stores `type(event).__name__`. The read side dispatches on a `case "<literal>":`. Nothing tied those two strings together, and a literal that drifts from its class is not a degraded read: the event writes fine, then every later fold of that stream raises at the wildcard arm and the whole stream stops loading. The gate review demonstrated this instead of arguing it. Injecting the divergence into `AllocationCeilingUpdated` left 155 allocation-touching tests and all three replay fitness functions green, because no test writes a ceiling update twice on one stream, so the event is never deserialized anywhere. The same class of bug already reached HEAD once in this branch: `AgentToolRevoked`'s `to_payload` arm was written on one line where its neighbours span several, so a rename updated the class and `from_stored` and silently skipped serialization. A round-trip test caught that one. Luck is not a control. The existing coverage fitness function cannot see this by design: it permits extra literals aliased to a union member, because that is the shape a legacy rename takes. So a stale literal satisfies it. This one asks the different question, per arm, and cannot be missing the way per-event round-trips can. Verified against the reviewer's exact injection: caught in 1.1 seconds. One legitimate divergence exists corpus-wide (`ActorRegisteredV2`), allowlisted with a staleness guard. AST helpers are inlined verbatim rather than imported, since pyright rejects cross-module private use and fitness tests here stand alone. Two smaller gaps from the same review. `Superseded` accounted for all 166 uses of the enum; `Defective`, the value the closed set exists to make findable, had never left a fixture. The Plan round-trip is now parametrized over all three members and one endpoint posts `Defective` end to end. And the read side kept leaking the event's primitive: state, fold and both `get_agent` DTOs now carry `DeprecationReason`, matching what `CautionRetired` has always done on both sides. * Make five green tests actually test what they claim Found while checking whether the branch was ready to ship, which is the point: all five passed. A suite going green says nothing about tests asserting the wrong thing. `test_handler_deprecates_a_versioned_agent` seeded a DEFINED agent. It was a byte-for-byte rerun of the test above it, and the name was the only thing suggesting otherwise. That mattered more than the others: deprecate's source set was widened to include Versioned and Suspended, and this was supposed to be the unit-tier proof that the widened arm works. The only real coverage was an integration test, and that one was among the four found red earlier today. Now seeds through Versioned and asserts the later stream version. `test_to_payload_serializes_agent_deprecated` was an exact duplicate of its `_with_reason` sibling, left over when the optional-reason case stopped existing. `test_decide_trims_reason_before_embedding_in_event` trims nothing. Its input is an enum member, which has no whitespace to strip, so it asserts what the test above it already asserts. The behaviour it named was deleted with the bounded-text VO; the test outlived it. And the two slices that gained a required free-text reason had no negative coverage at all: `decommission_asset` had neither a missing-reason nor an empty-reason test, and `revoke_tool_from_agent` had no empty-reason test. Their siblings all carry both. --------- Co-authored-by: xmap <16776958+xmap@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Follow-up to a feature-slice naming audit (~280 slices, 17 BCs). The audit found the corpus is overwhelmingly clean; this PR closes the few items worth acting on.
1. Lock the
suspend/holdpause-verb split (docs only)The reversible-pause verb is
suspendon grant-shaped aggregates (Agent, Permit) andholdon execution/container aggregates (Run, Campaign, Visit). Investigation of code semantics + the standards corpus (license/account suspension, X.509certificateHold, PackMLHeld, Bluesky pause) showed this tracks a real axis: the pause verb pairs with the aggregate's terminal verb (suspend+revoke/deprecate,hold+abort/stop/close). All five share theresumerecovery verb. Documented inconventions.mdso it reads as intentional, with a guard innaming-r3-reviewerso the coexistence is not re-flagged.2.
revise_agent_budget->update_agent_budgetrevisewas a lone synonym for the corpus's dominantupdate_<subject>_<field>field-mutation verb (same set/replace/clear PUT shape asupdate_asset_settings,update_method_launch_spec). Renamed slice, command, MCP tool, event (AgentBudgetUpdated, matching the family's*Updatedevents), and error (AgentCannotUpdateBudgetError). REST URL was already noun-based (POST /agents/{id}/budget), unchanged.3.
arrive_visit->record_visit_arrivalarrivewas the one intransitive verb in the Visit lifecycle slice family ("arrive a visit" breaks the transitive verb-subject skeleton of start/hold/resume/complete/cancel/abort/void). The action is an explicit operator gesture, so "record arrival" reads transitively. Renamed slice, command, MCP tool, route function, and URL (POST /visits/{visit_id}/record-arrival). The eventVisitArrived, statusArrived,arrived_atcolumn, andVisitCannotArriveErrorguard keep the arrival-fact vocabulary (command verb differs from event past-participle, as withStartRun/RunStarted).Validation
openapi.jsonregenerated for both renamed surfaces.🤖 Generated with Claude Code