fix(ads): worker drains every approved spot per tick and Approve names the render window (gh-#745) - #764
Merged
Conversation
STORY-432/433 specs from /plan: the worker drains every approved spot per tick (bounded), and an approved spot's DTO/toast names the render window. Red until T456–T458 land.
…ject session The tracked hook command used "$CLAUDE_PROJECT_DIR" alone, which expands to /.claude/hooks/merge-guard.sh and fails open when the variable is unset. Fall back to $PWD so the guard still fires in sessions started from the project root. The README and git-workflow skill name the fallback. gh-#745 PR-A rider.
RenderOneIfDueAsync claimed one approved spot per tick, so approving three spots left two of them waiting a full interval each (gh-#745). RenderDueAsync now loops ClaimNextApprovedAsync until the queue is empty or MaxRendersPerTick (25) is hit, re-checking the on-air render gate before every claim; a per-spot failure marks that spot failed and the drain continues, a claim conflict or a cancelled render stops the tick. Logs the drained count once. Story391's one-per-tick fact pinned the old behaviour and is retired on purpose; Story432's failure scenario gains the InvokeDelegates=false arrange the render-service specs already use.
Add `renderWithinMinutes` to `AdSpotDto`: the configured `Ads:WorkerIntervalMinutes` for a spot in the Approved state, `null` for every other state. The admin UI uses it to tell the operator roughly when the station will pick the spot up (STORY-433 AC1-AC3, gh-#745). The TS `AdSpotDto` mirror and the spec fixtures gain the field so `typecheck:specs` stays green.
The row toast and both wizard approve buttons now read "Approved. The station will render it within N minutes." (singular at 1) from the approve response's `renderWithinMinutes`, falling back to "Approved." when the window does not apply. New `describeApproved` helper in ads-api.ts keeps both callers on one sentence. "Approve without a preview" no longer looks like nothing happened (STORY-433 AC4-AC7, gh-#745).
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #745 · STORY-432 (worker drains every approved spot per tick) · STORY-433 (Approve tells me when the station will render it) · PLAN T455–T459.
🐛 What was wrong
AdSpotWorkerrendered one approved spot per 10-minute tick. Approve three spots in the wizard and the second and third sat in Approved for 20–30 minutes with nothing on screen saying why. The toast just said "Spot approved."✅ What changed
MaxRendersPerTick = 25, still yields to on-air renders). Log line:Ad spot worker rendered {Count} approved spot(s) this tick.AdSpotDto.renderWithinMinutes=Ads:WorkerIntervalMinuteswhile a spot is Approved,nullin every other state.Approved. The station will render it within 10 minutes.(singular at 1) from both the row and the wizard's approve-without-a-preview step.describeApproved()inadmin-ui/lib/ads-api.tsis the single copy source.${CLAUDE_PROJECT_DIR:-$PWD}so it works outside a Claude project session (.claude/settings.example.json,.claude/README.md,git-workflowskill). This is the settings fix that was blocking the guard locally;settings.local.jsonitself is personal and gitignored.Specs:
tests/GenWave.Host.Tests/Specs/Story432_DrainApprovedSpots.cs,Story433_RenderWindow.cs,admin-ui/__specs__/approve-copy.spec.tsx(red at a287e17, green now).🔌 Wire evidence (T459, dev stack via
./launch.sh)Approved spots 18, 19, 20 from the Ads page inside one interval. One worker pass:
GET /api/ads/{18,19,20}→"state":"ready". Approve responses carried"renderWithinMinutes":10.Toast screenshot (Next dev server against a Release Kestrel with
Ads__WorkerIntervalMinutes=7): reads "Approved. The station will render it within 7 minutes." — sent to Dean directly; the repo does not carry screenshots.📝 Notes for review
$HOME-based hook path is the root cause of the guard misfire; T455 is the fix, not a workaround.[data-sonner-toaster]node never appears; reproduced on main's image747e5649686b). Filed as a follow-up issue. Approvals themselves succeed either way; the copy is verified on the dev server and in Jest.Do not merge without Dean.