fix(runner): cap HuggingFace Retry-After backoff at 30s - #50
Conversation
Honor Retry-After integer seconds and HTTP-date values only up to the existing 30s quadratic fallback ceiling. A 429 with Retry-After: 3600 could block clawscan benchmark for an hour per attempt when the client context is Background. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. Reviewed September 5, 2026, 9:59 AM ET / 13:59 UTC. ClawSweeper reviewWhat this changesCaps Hugging Face retry delays from numeric and HTTP-date Retry-After headers at 30 seconds and adds tests for both formats. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked before merge - 5 items remain Keep open: current main and v0.1.7 do not implement this limit. The prior cooldown-recovery finding remains unresolved, and the supplied helper-test transcript does not establish real HTTP recovery. Priority: P2 Review scores
Verification
How this fits togetherClawScan’s benchmark client downloads datasets for scanner evaluation and submission validation. Its HTTP retry loop converts server responses into delays, then returns dataset rows or a fetch error. flowchart TD
A[Benchmark or submission validation] --> B[Dataset HTTP request]
B --> C{Response retriable?}
C -->|No| F[Return rows or error]
C -->|Yes| D[Calculate retry delay]
D --> E{Canceled or attempts exhausted?}
E -->|Yes| F
E -->|No, after waiting| B
Decision needed
Why: The current implementation explicitly honors server delays; changing that tradeoff affects unattended benchmark and submission workflows and needs maintainer intent. Before merge
Findings
Agent review detailsSecurityNone. Review metricsNone. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve server cooldowns by default and use a maintainer-approved cancellation or wait-budget policy to bound latency without premature retries. Do we have a high-confidence way to reproduce the issue? Yes, from source: a 3600-second header produces an hour-long wait on main, while fast persistent throttling exhausts the patched client’s six attempts in roughly 150 seconds. No runtime reproduction was executed. Is this the best way to solve the issue? No. A per-wait cap defeats server-directed recovery within the fixed retry budget; cancellation or an explicitly approved waiting budget provides a safer boundary. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6432c40f2340. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (29 earlier review cycles; latest 8 shown)
|
What Problem This Solves
clawscan benchmark clawhub-security-signalsfetches HuggingFace dataset rows and retries on 429/5xx.huggingFaceRowsBackoffhonorsRetry-Afteras integer seconds or an HTTP-date with no ceiling, while the quadratic fallback is already capped at 30s.The public CLI builds
HuggingFaceBenchmarkClientwith a nil Context, sorequestContext()iscontext.Background()and the retryselectnever expires. A 429 withRetry-After: 3600blocks the process for one hour per attempt (up to five waits). This change capsRetry-Afterat the same 30s ceiling. It does not add a process-wide signal context.The uncapped header path was introduced in #3 (2026-06-25). #46 made the wait cancelable when a context is set; it did not cap the delay.
Evidence
Red (unfixed
huggingFaceRowsBackoff):Green (after the 30s cap):
Retry-After: 3600and an HTTP-date two hours ahead both return 30s. The quadratic fallback path is unchanged.Real behavior proof
Behavior or issue addressed: HuggingFace row-fetch
Retry-Afterdelays (integer seconds and HTTP-date) are capped at 30s, matching the existing fallback ceiling, so a 429 cannot stallclawscan benchmarkfor an hour per attempt.Real environment tested: macOS 26.6.2 (Darwin 25.6.0 arm64), go1.27.0 darwin/arm64, checkout
/tmp/oc-pr-clawscan-F003onfix/f003-huggingface-retry-after-cap.Exact steps or command run after this patch:
go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/Evidence after fix: terminal output from the patched tree:
Observed result after fix:
Retry-After: 3600is 30s (was 1h). An HTTP-date two hours ahead is also 30s. The helper no longer returns a multi-hour wait.What was not tested: A live HuggingFace 429 with a long
Retry-Afterheader. Network fetch ofdatasets-server.huggingface.coduring this change.Command:
go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/Observed: unfixed delay was
1h0m0sforRetry-After: 3600; patched delay is30sfor both3600and a future HTTP-date.Expected: both header forms cap at 30s.
Time: 11:07:34 PDT (2026-08-29 18:07:34 UTC)
Date: 2026-08-29
Environment: macOS 26.6.2, Darwin 25.6.0 arm64, go1.27.0 darwin/arm64