control: accurate HTTP status codes for model actions#20
Merged
Conversation
The download and delete handlers mapped every App error to 409 Conflict, including a malformed model id (a 400 Bad Request) and deleting a model that isn't present (a 404 Not Found). Add an ErrInvalidRepoID sentinel and a small mapper so the endpoints report 400 / 404 / 409 as appropriate; a genuine conflict (already downloading, busy serving) stays 409. Tests assert an invalid id is 400 and an absent-model delete is 404 (both were 409 before). Assisted-by: Claude:claude-fable-5
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.
Cleanup from the round-9 WebUI review. The
downloadanddeletehandlersmapped every
Apperror to409 Conflict— including a malformed model id(should be 400) and deleting a model that isn't in the registry (should be 404).
Adds an
app.ErrInvalidRepoIDsentinel and a smallmodelErrorStatusmapper:400 for a bad id, 404 for an absent model, 409 for a genuine conflict
(already-downloading, or busy serving a request). Tests assert the 400 and 404
cases (both returned 409 before).
Note on the other round-9 WebUI findings: on closer inspection the rest are
intentional or defensible, so I did not change them —
says cancel abandons the download outright rather than leaving a failed row);
Registry.Removenot broadcasting whenos.RemoveAllfails correctly keepsthe still-present model visible;
bytes(0)rendering "—" treats 0 as "unknown", which is reasonable.The one genuine UX gap — the Load button lacks a loading state — is frontend-only
with no test harness; happy to do it (with a browser check) if you want it.
Assisted-by: Claude:claude-fable-5