fix: wrong-shaped API error raise (#522) and a proposal count that disagreed with its own page (#524) #629

Merged
claude-bot merged 2 commits from fix/522-524-small-correctness into main 2026-09-12 19:31:58 +00:00
Collaborator

Two small, independent correctness fixes, one commit each.

#522 — QuestBoardApiError accepted anything as its response

__init__ read getattr(response, "status_code", 0), so a caller passing a string where the response belongs got status_code == 0 and an empty detail — which user_message_for renders as "Could not reach Quest Board right now. Please try again shortly." The bot announcing an outage because the bot got its own arguments wrong, three layers from the mistake.

VotingClosedError was raised that way during the #373 rebase and was caught only because a test happened to cover that path. A missing status_code is now a TypeError. The message names the type that was actually raised, so a subclass does not report its parent's name — with a test for that, since a guard written on the base class is the obvious place to get it wrong.

#524 — the proposal count did not match the proposal page

count_bot_proposals was the one bot-proposal surface without the deleted_at IS NULL predicate that list_bot_proposals directly above it, and attention_service's "Needs you" query, both carry. Every soft-deleted proposal widened the gap.

Worth noting it is not only the cosmetic disagreement the issue describes: this count is the X-Total-Count the proposals page paginates on, so a large enough gap offered a page of suggestions that came back empty.

The regression test went into test_lore_soft_delete.py, whose header says there is deliberately a test per surface rather than one test for the flag — the count was the surface it was missing.

One thing found alongside

LoreEntry.deleted_at's comment told the reader "every read filters on it; see live_lore_entries in lore_service for the one helper they all go through." No such helper exists, or ever has. A reader trusting that comment would conclude the filtering was centralised, when in fact every query carries its own predicate — which is precisely how this one went missing for four releases. Comment corrected to say what the code actually does. (Squarely the shape of thing #442/#444 are about.)

Deliberately not changed

  • campaigns.py:3454, the approve-a-proposal lookup, also omits the filter, so a stale browser tab could approve an already-trashed proposal and resurrect it. That is a write path with different consequences, not a count, so it wants its own issue rather than a quiet ride along here.
  • The two delete(LoreEntry) statements that clear stale proposals omit it on purpose — hard-deleting a trashed proposal is the intended outcome there.

Tests

  • Bot: 442 passed
  • Backend: 3430 passed, 13 skipped
  • ruff check / ruff format --check clean under CI's pinned 0.4.4

Both new tests were confirmed to fail against the pre-fix source.

Closes #522
Closes #524

🤖 Generated with Claude Code

Two small, independent correctness fixes, one commit each. ## #522 — `QuestBoardApiError` accepted anything as its response `__init__` read `getattr(response, "status_code", 0)`, so a caller passing a string where the response belongs got `status_code == 0` and an empty detail — which `user_message_for` renders as *"Could not reach Quest Board right now. Please try again shortly."* The bot announcing an outage because the bot got its own arguments wrong, three layers from the mistake. `VotingClosedError` was raised that way during the #373 rebase and was caught only because a test happened to cover that path. A missing `status_code` is now a `TypeError`. The message names the type that was actually raised, so a subclass does not report its parent's name — with a test for that, since a guard written on the base class is the obvious place to get it wrong. ## #524 — the proposal count did not match the proposal page `count_bot_proposals` was the one bot-proposal surface without the `deleted_at IS NULL` predicate that `list_bot_proposals` directly above it, and `attention_service`'s "Needs you" query, both carry. Every soft-deleted proposal widened the gap. Worth noting it is **not only** the cosmetic disagreement the issue describes: this count is the `X-Total-Count` the proposals page paginates on, so a large enough gap offered a page of suggestions that came back empty. The regression test went into `test_lore_soft_delete.py`, whose header says there is deliberately a test per surface rather than one test for the flag — the count was the surface it was missing. ### One thing found alongside `LoreEntry.deleted_at`'s comment told the reader "every read filters on it; see `live_lore_entries` in lore_service for the one helper they all go through." **No such helper exists, or ever has.** A reader trusting that comment would conclude the filtering was centralised, when in fact every query carries its own predicate — which is precisely how this one went missing for four releases. Comment corrected to say what the code actually does. (Squarely the shape of thing #442/#444 are about.) ### Deliberately not changed - `campaigns.py:3454`, the approve-a-proposal lookup, also omits the filter, so a stale browser tab could approve an already-trashed proposal and resurrect it. That is a write path with different consequences, not a count, so it wants its own issue rather than a quiet ride along here. - The two `delete(LoreEntry)` statements that clear stale proposals omit it on purpose — hard-deleting a trashed proposal is the intended outcome there. ## Tests - Bot: `442 passed` - Backend: `3430 passed, 13 skipped` - `ruff check` / `ruff format --check` clean under CI's pinned 0.4.4 Both new tests were confirmed to fail against the pre-fix source. Closes #522 Closes #524 🤖 Generated with [Claude Code](https://claude.com/claude-code)
`__init__` read `getattr(response, "status_code", 0)`, so a caller passing a
string where the response belongs got `status_code == 0` and an empty detail —
which `user_message_for` renders as "Could not reach Quest Board right now.
Please try again shortly." The bot announcing an outage because the bot got its
own arguments wrong, three layers from the mistake.

`VotingClosedError` was raised that way during the #373 rebase and was caught
only because a test happened to cover that path. A missing `status_code` is now
a `TypeError`, naming the type that was raised so a subclass does not report
its parent's name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(webapp): exclude trashed proposals from count_bot_proposals (#524)
All checks were successful
CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 1m0s
CI / Backend lint (ruff) (pull_request) Successful in 36s
CI / Bot/backend version sync (pull_request) Successful in 27s
CI / Docker image build (pull_request) Successful in 18s
CI / Frontend tests, audit, and build (pull_request) Successful in 2m45s
CI / Bot tests and audit (pull_request) Successful in 2m15s
CI / Backend migration, tests, and audit (pull_request) Successful in 8m25s
CI / Synthetic session harness (no GPU, no LLM) (pull_request) Successful in 18m12s
13a5b93028
`count_bot_proposals` was the one bot-proposal surface without the
`deleted_at IS NULL` predicate that `list_bot_proposals` beside it, and
`attention_service`'s "Needs you" query, both carry. Every soft-deleted
proposal widened the gap between the two.

It is not only a cosmetic disagreement with the dashboard: the count is the
`X-Total-Count` the proposals page paginates on, so it promised a page of
suggestions that came back empty.

Also corrects the soft-delete note on `LoreEntry.deleted_at`, which told the
reader that every read filters through a `live_lore_entries` helper. No such
helper exists, and never has — each query carries its own predicate, which is
exactly why one could go missing for four releases without anyone noticing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-bot deleted branch fix/522-524-small-correctness 2026-09-12 19:31:59 +00:00
Sign in to join this conversation.
No description provided.