fix: answer a rejected id as a client error, not a server fault

[dmccoystephenson]

Summary

• An id path variable that violates the @Min(1) bound its controller declares is now answered 400
  with an ErrorResponse naming the rejected parameter, rather than 500 with the message masked to
  "An unexpected error occurred". jakarta.validation.ConstraintViolationException — the type Spring's
  method-validation proxy raises for a method-parameter constraint, because every controller carries
  @Validated — had no handler in GlobalExceptionHandler and reached handleGenericException.
• The rejection is now logged at WARN as a client error, in place of an ERROR with a stack trace.
• Only the last node of the violation's property path is quoted back. The validator reports it as
  method.parameter (getEntityById.id), and which handler method received the request is not the
  caller's concern.
• docs/openapi/viron-api.json documents the 400 on the 27 routes that can now produce one, through
  a shared #/components/responses/ValidationError, and expresses the @Min bound itself as
  minimum: 1 on the parameter schemas.
• OpenApiSpecDriftTest gains two checks that read the controllers' annotations directly, so neither
  the 400 nor the bound can fall out of the contract again.

A spec bug the new guard found

Four location routes referenced a shared parameter component whose name does not match their path
template, which the route-level comparison in the existing drift test cannot see:

Route	Declared	Path template
GET /api/v1/locations/{id}	locationId	{id}
GET /api/v1/locations/environment/{environmentId}	id	{environmentId}
GET /api/v1/locations/grid/{gridId}	id	{gridId}
GET /api/v1/locations/grid/{gridId}/unoccupied	id	{gridId}

A client generated from the spec would have built the wrong URL for each. They are declared inline
now, matching how GET /api/v1/grids/environment/{environmentId} already declared its own. The
Postman collection was checked and was already correct, so it is unchanged.

Test plan

• mvn -B test — 454 tests, 0 failures (443 before this change, 11 added)
• PathVariableValidationTest reverted against the unfixed handler: all 6 original cases fail with
  expected:<400> but was:<500>, and pass with the handler in place
• boundedPathVariablesDeclareTheirBoundInTheSpec failed on the four mismatched parameters above
  before they were corrected, and passes now
• python3 -m pytest (pytest 7.1.3) and pytest (pytest 9.0.3) — 108 tests each, 0 failures
• docs/openapi/viron-api.json parses, and the spec's route set is unchanged

Notes on scope

• The Python client needs no mirrored change: it sends ids it holds rather than caller-supplied ones,
  and none of its methods branch on 500 as distinct from any other non-2xx answer. Its suite was
  run regardless and is green.
• No controller signature changed, so no endpoint's path, verb, or body moved; the contract gains
  documented answers rather than new ones.
• Both debug routes and GET /api/v1/environments/name/{name} are untouched: neither validates an
  input, so neither can raise the exception this handler answers.

Deferred this cycle

The open backlog was reviewed and left alone, with reasons:

• One entity per location is enforced only by the move endpoint, so a concurrent placement still walks around it #213 (one entity per location) and The Python client cannot be installed or imported as a package; its modules import through the repository path #209 (Python client packaging) each state in the issue
  body that a product decision is required before any implementation is correct, so neither is work
  this loop should pick.
• Nothing bounds how long a request waits for a row lock, and the pool is not sized against it #212 (row-lock timeout and pool sizing) is implementable but sets production values against an
  expected concurrency that is not recorded anywhere in the repository; it also wants a considered
  pair of settings rather than the single handler this PR adds.
• Wire OpenAPI-first codegen / spec-drift guard (rebuild-plan hygiene gap) #130 (OpenAPI-first codegen) is a build-tooling change well past a polish-sized PR. The guard
  added here covers a further slice of its second gap, a spec-drift check in CI, without wiring a
  generator.
• The README's feature list understates the implemented API surface #215 was filed during this cycle's documentation sweep — the README's feature list understates
  the implemented surface. It is unrelated to this fix and was kept out to avoid unrelated edits in a
  bug-fix PR.

Closes #214

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric, scored against the diff and command output rather than impression.

• Scope: PASS, with one judgment call flagged below — four files, each traceable to An out-of-range id path variable is answered 500 rather than 400, because ConstraintViolationException has no handler #214: the handler is the fix, PathVariableValidationTest is its coverage, OpenApiSpecDriftTest is the guard against the documentation half recurring, and the spec is the contract for the behaviour that changed. No formatting or comment churn; git diff --stat origin/main reports 235 insertions / 24 deletions across 4 files, of which ~92 are non-test.
• Tests-new: PASS — handleConstraintViolationException is the only public method added, and nine cases exercise it: the four id-bearing controllers, a PATCH whose body is valid while its id is not, a request with both ids out of range, a negative id, and an assertion that the handler method's name is absent from the message.
• Tests-fix: PASS — confirmed empirically, not by reasoning. With GlobalExceptionHandler.java reverted to origin/main, all 9 cases of PathVariableValidationTest fail (expected:<400> but was:<500>); restored, all 9 pass. With docs/openapi/viron-api.json reverted, the two new OpenApiSpecDriftTest checks fail naming 27 and 29 routes respectively; restored, both pass. The message assertions use the exact rendered string, so a handler that answered 400 with a different message would still fail.
• Sibling structure: PASS — PathVariableValidationTest follows LocationControllerTest: @SpringBootTest + @AutoConfigureMockMvc + @WithMockUser, repositories and DbInteractions supplied as @MockBean, DbConfig left real.
• Sibling renames: PASS (nothing renamed) — no identifier in a parallel pair or series was renamed; the new handler is added alongside the existing ones rather than replacing one.
• Docs: PASS — docs/openapi/viron-api.json is updated in this PR and its accuracy is now machine-checked. docs/MVP.md needs nothing: no route, verb, or DTO moved. The Postman collection was checked route by route against the controllers and was already correct — it was the spec, not Postman, that had the four mismatched parameter names. README.md and docs/PLANNING.md describe nothing this change alters.
• Issue resolution: PASS — each of An out-of-range id path variable is answered 500 rather than 400, because ConstraintViolationException has no handler #214's four acceptance points is met: 400 naming the rejected parameter, WARN in place of ERROR with a stack trace, the 400 and the bound both expressed in the spec, and coverage that fails without the fix.
• CI: PASS — build, python-client (3.8), and python-client (3.12) all green on the PR head. CI here does cover the Python client, so its green is real signal for both surfaces rather than the Java one only.
• DTO boundary: PASS — no controller method was touched; the handler returns ErrorResponse, which is a DTO, and no internal model appears in the diff.
• Spec alignment: PASS — no endpoint's path, verb, params, or body changed. The spec gains documented answers for behaviour that already existed and was previously undocumented, in the same PR.
• Java/Python parallelism: PASS, by argument rather than by edit — nothing in src/main/python mirrors this change: the client sends ids it already holds rather than caller-supplied ones, and no client method distinguishes a 500 from any other non-2xx answer, so a status changing from 500 to 400 alters none of its branches. Its suite was run anyway — 108 tests, green on pytest 7.1.3 and 9.0.3.
• Override correctness: PASS (nothing to check) — no @Override was added.

Judgment calls left for a reviewer

docs/openapi/viron-api.json:280,301,321,341 — the four location routes whose parameter names were corrected are drift that predates this PR and is not caused by it. They are here because the guard this PR adds fails without them, so splitting them into a follow-up would land a PR whose own test is red. A reviewer who would rather see them separately should say so, and the guard can be narrowed to the 400 check and the parameter check filed on its own.

src/main/java/preponderous/viron/exceptions/GlobalExceptionHandler.java:88 — the violation messages are sorted before being joined, which the sibling handleValidationException does not do to its field errors. The divergence is deliberate: ConstraintViolationException.getConstraintViolations() returns a Set with no defined iteration order, so an unsorted join would give a request with two rejected ids a nondeterministic message. handleValidationException reads from an ordered list and has no such problem. The alternative reading — that the two should simply match — is a reasonable one to take.

src/main/java/preponderous/viron/exceptions/GlobalExceptionHandler.java:87 — the handler answers 400 to any ConstraintViolationException, on the strength of @Validated currently appearing only on the four controllers (verified by grep across src/main/java). Were a service or repository bean to become @Validated later, a constraint violated by internal code would be reported to the caller as its own bad request. Narrowing the handler by inspecting the violation's root bean type would guard against that, but it is speculative today and was left out.

Observed outside the diff, not acted on

tickets.md:244 describes the error response format as "JSON with message, status, timestamp". ErrorResponse carries status and message only, and never has carried a timestamp. This predates the PR and is unrelated to it, so it was not changed here; it is noted so it is not lost.

README.md — the feature list understates the implemented surface. Filed separately as #215 rather than folded in.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Autonomous merge is being held on this PR, and the reason is a path match rather than anything about the
change's readiness.

docs/openapi/viron-api.json is on this repository's do-not-auto-merge list, on the grounds that the spec
is the wire boundary and a change to it warrants human review. That hold is satisfied only by a codeowner
authorising the merge after being shown which protected path matched, which is what this comment is for.
The general merge pre-authorization this session was dispatched with is not treated as covering it, since
it was granted without sight of the diff.

Everything else the merge gate asks for is done:

• CI is green on the head commit — build, python-client (3.8), and python-client (3.12).
• The regression gate is satisfied empirically, not by inspection: with the handler reverted, all 9 cases
  of PathVariableValidationTest fail; with the spec reverted, both new OpenApiSpecDriftTest checks fail.
• The Phase 7 documentation pass was completed against every source of truth and found nothing further to
  correct. tickets.md Ticket 8's definition of done — "Validation errors return 400 with clear messages" —
  is satisfied by this change where it previously was not.
• The self-review rubric is posted above, including three judgment calls that were deliberately left open
  rather than decided unilaterally.

No further work is queued against this branch. Merging it needs only a codeowner's word.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson