Skip to content

fix(benchmark): make prompt cache instantiation atomic - #729

Draft
RapidPoseidon wants to merge 1 commit into
mainfrom
fix(benchmark)/atomic-prompt-cache-instantiation
Draft

fix(benchmark): make prompt cache instantiation atomic#729
RapidPoseidon wants to merge 1 commit into
mainfrom
fix(benchmark)/atomic-prompt-cache-instantiation

Conversation

@RapidPoseidon

@RapidPoseidon RapidPoseidon commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Context

Investigating a report that benchmark.prompts / .identifiers / .tags broke in SDK 3.17.2 with:

TypeError: Field 'tags' on GetPromptsByBenchmarkEndpointOutput has an unexpected type from the backend: Input should be a valid string

Primary root cause (already fixed on main, not this PR): backend PR RapidataAI/rapidata-backend#4879 feat(leaderboard): prompt tags and origin (merged 2026-07-27) deliberately changed GET /benchmark/{id}/prompts to return tags as {value, category} objects (plus a new origin field). The OpenAPI regen (#725) already updated the model to List[Tag] and __instantiate_prompts already extracts tag.value. That fix just needs a PyPI release — the latest published version (v3.17.2, 2026-07-22) predates the backend change. This PR does not address the release; it fixes an independent latent bug the report surfaced along the way.

What this PR fixes (report's "Secondary issue 1")

__instantiate_prompts reset the five prompt caches and appended to them in place while paginating. Any failure partway through — a 503 on a later page (the report saw intermittent 503s), or a parse error on a single item — left the caches partially populated. Because every cache getter (prompts, identifiers, tags, …) treats a non-empty list as a complete, authoritative set, the next access returned silently-truncated data instead of re-fetching or raising.

Observed in the report: .prompts raised, then .identifiers returned 1 entry rather than re-instantiating — which downstream surfaced as a misleading "All identifiers/prompts must be in the registered ... list" instead of the real fetch failure.

Fix: accumulate into local lists and assign to self only after every page has been fetched and parsed cleanly — the caches are now all-or-nothing.

Not in this PR (deliberately)

  • Release of the tags fix — needs a maintainer to dispatch Release and Publish (patch → 3.17.3). Publishing to PyPI is outside an agent's remit.
  • Report's open question — add_model(prompts=...) validates against the wrong list. Confirmed a real latent bug: when prompts= is passed, it is validated against self.identifiers (line ~619) and used as the upload identifier, but identifier and original_prompt are distinct fields (identifiers default to prompt text only when add_prompts was called without explicit identifiers). For a benchmark with distinct identifiers, valid prompts= input is spuriously rejected, and the complete fix must also translate prompt→identifier before upload (samples are matched server-side by identifier). That's a behavioral change with edge cases (duplicate/None prompt text) best decided by the owner, so it's left out of this focused PR — happy to follow up.

Testing

uv run black clean; uv run pyright src/rapidata/rapidata_client → 0 errors.

🔗 Session: https://poseidon.rapidata.internal/chat/session-f1ef50bc


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

__instantiate_prompts reset the five prompt caches and appended to them
in place while paginating. Any failure partway through — a 503 on a later
page, a schema-drift parse error on one item — left the caches partially
populated, and every cache getter treats a non-empty list as a complete,
authoritative set. The result was a silently truncated benchmark: e.g.
.prompts raising, then .identifiers returning a single entry instead of
re-fetching or raising, which later surfaced as a misleading "all
identifiers must be registered" error rather than the real fetch failure.

Accumulate into locals and assign to self only after every page has been
fetched and parsed, so the caches are all-or-nothing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: mads <37117211+Makuh17@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant