[Workbench] Two legacy name-generation endpoints bypass generation_results and write to Redis instead #288

New issue

Closed

opened 2026-08-05 20:34:06 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-08-05 20:34:06 +00:00

Contributor

Found in the LLM trigger audit that produced #278 and #279.

The inconsistency

generation_service is the one genuinely well-factored part of the LLM layer: a data-driven ToolDef registry (generation_service.py:1530-1663), one generic executor (run_tool, :301), one endpoint (campaigns.py:5235), and a persistence layer in generation_results that the Workbench polls (added in #138).

But two endpoints call run_tool directly and never persist the result:

campaigns.py:4302 — name-cache miss path. Calls generation_service.run_tool(...), takes result.output["options"], and writes it to Redis under name_cache:{campaign}:{category} with a TTL.
sessions.py:290 — the legacy session-scoped canonical-name endpoint. Same pattern.

Both produce real LLM output that costs real tokens and then drops it outside the system of record.

Why it matters

No history. Every other Workbench generation is queryable, re-viewable, and attributable via generation_results. Name generations vanish when the Redis TTL expires.
No token accounting. generation_results.token_usage is how spend is tracked. These two paths spend tokens invisibly.
Two sources of truth. A GM looking at Workbench history sees an incomplete picture with no indication anything is missing.
Divergent failure handling. These paths 503 on a missing LLM config; the persisted path records a failed row the UI can show.

Counting them, generation_service has six enqueue/execute paths hanging off it (sync inline, async Celery, cold-pool inline, and three prefetch refills) plus these two legacy bypasses. The registry is clean; the ways into it are not.

Fix direction

Route both through the same persistence layer the rest of the Workbench uses, keeping the Redis cache as a cache in front of generation_results rather than as the only place the output exists. The name-cache TTL behaviour can stay — it is a reasonable latency optimisation — but a cache miss should persist as well as cache.

Worth checking whether the session-scoped endpoint at sessions.py:290 is still used by the frontend at all, or whether it was superseded by the campaign-scoped one and can simply be removed.

Acceptance

Name generations appear in generation_results with token usage recorded
The Redis cache still short-circuits repeat requests
sessions.py:290 either routed through persistence or removed if dead
Failure handling consistent with the rest of the Workbench

Labels: backend

Found in the LLM trigger audit that produced #278 and #279. ## The inconsistency `generation_service` is the one genuinely well-factored part of the LLM layer: a data-driven `ToolDef` registry (`generation_service.py:1530-1663`), one generic executor (`run_tool`, `:301`), one endpoint (`campaigns.py:5235`), and a persistence layer in `generation_results` that the Workbench polls (added in #138). But **two endpoints call `run_tool` directly and never persist the result**: - `campaigns.py:4302` — name-cache miss path. Calls `generation_service.run_tool(...)`, takes `result.output["options"]`, and writes it to **Redis** under `name_cache:{campaign}:{category}` with a TTL. - `sessions.py:290` — the legacy session-scoped canonical-name endpoint. Same pattern. Both produce real LLM output that costs real tokens and then drops it outside the system of record. ## Why it matters - **No history.** Every other Workbench generation is queryable, re-viewable, and attributable via `generation_results`. Name generations vanish when the Redis TTL expires. - **No token accounting.** `generation_results.token_usage` is how spend is tracked. These two paths spend tokens invisibly. - **Two sources of truth.** A GM looking at Workbench history sees an incomplete picture with no indication anything is missing. - **Divergent failure handling.** These paths 503 on a missing LLM config; the persisted path records a `failed` row the UI can show. Counting them, `generation_service` has **six** enqueue/execute paths hanging off it (sync inline, async Celery, cold-pool inline, and three prefetch refills) plus these two legacy bypasses. The registry is clean; the ways into it are not. ## Fix direction Route both through the same persistence layer the rest of the Workbench uses, keeping the Redis cache as a *cache* in front of `generation_results` rather than as the only place the output exists. The name-cache TTL behaviour can stay — it is a reasonable latency optimisation — but a cache miss should persist as well as cache. Worth checking whether the session-scoped endpoint at `sessions.py:290` is still used by the frontend at all, or whether it was superseded by the campaign-scoped one and can simply be removed. ## Acceptance - [ ] Name generations appear in `generation_results` with token usage recorded - [ ] The Redis cache still short-circuits repeat requests - [ ] `sessions.py:290` either routed through persistence or removed if dead - [ ] Failure handling consistent with the rest of the Workbench Labels: backend

claude-bot added the

backend

label

2026-08-05 20:34:30 +00:00