feat(api): retrieve interpretation runs via loopback GET-by-id - #438
Conversation
GAP-003A unique slice stacked on collection GET: loopback
GET /v1/interpretation-runs/{idempotency_key} returns one accepted
metric-free hypothetical identity without POST replay. Naruon and
LineageWeave refused. ADR 0071.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Coverage evidence needs follow-up
The repository requires 100% branch coverage. Added tests omit retrieval cursor refusal and several decoding branches, while the verification list provides no coverage run.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if idempotency_key.contains('/') || idempotency_key.contains('\0') { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| if idempotency_key.len() > INTERPRETATION_RUN_RETRIEVAL_ID_MAX_LEN { | ||
| return Err(OrchestratorLiveError::LimitExceeded); |
There was a problem hiding this comment.
🟡 Accepted runs become unretrievable
When POST accepts a key containing /, controls, or over 128 bytes, interpretation_run_retrieval_path_id rejects it. GET cannot retrieve the accepted run.
Prompt for agents
Unify idempotency-key validation across POST creation, collection rows/cursors, the retrieval exchange builder, and GET path decoding. Today InterpretationRunRequest accepts slash, control-character, and arbitrarily long keys, while interpretation_run_retrieval_path_id rejects them and the exchange builder applies a different subset. Either make every accepted POST key representable by GET-by-id or reject unsupported keys before accepting and storing a run. Add round-trip tests for all boundary characters and the 128-byte limit.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let payload = to_json(item)?; | ||
| refuse_metrics_on_interpretation_run_collection_payload(&payload)?; |
There was a problem hiding this comment.
🟡 Invalid identities gain authority
Callers can mutate public item fields before interpretation_run_retrieval_item_json serializes them. It emits non-hypothetical or scientifically authoritative identities without error.
Prompt for agents
Make interpretation_run_retrieval_item_json validate the supplied InterpretationRunCollectionItem before serialization. The fields are public, so construction through InterpretationRunCollectionItem::new is not sufficient. Reuse or expose the collection item's validation rather than duplicating claim-status and scientific-authority checks, and add tests that mutate claim_status and scientific_authority before calling the serializer.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let payload = to_json(item)?; | ||
| refuse_metrics_on_interpretation_run_collection_payload(&payload)?; |
There was a problem hiding this comment.
🟡 Reserved text blocks retrieval
When an accepted key contains tepp.scientific_acceptance.v1, refuse_metrics_on_interpretation_run_collection_payload rejects its serialized identity. GET returns 400 for an existing run.
Prompt for agents
Change the metric/scientific-acceptance filter to inspect JSON structure instead of rejecting the raw substring everywhere. Opaque identifier values may legitimately contain tepp.scientific_acceptance.v1. Reject that identifier only where it represents a forbidden schema/key, preserve opaque values, and add a POST-then-GET regression test using an idempotency key containing the text.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let accepted = self | ||
| .accepted_runs | ||
| .get(&idempotency_key) | ||
| .map(|(_, accepted)| accepted) | ||
| .ok_or(OrchestratorLiveError::InvalidWirePayload)?; |
There was a problem hiding this comment.
🔍 Missing-record status remains ambiguous
Unknown identities return HTTP 400 through InvalidWirePayload. The ADR requires refusal but defines no status; confirm whether clients require a distinct 404 response.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Folded into #439
Closed as
superseded_by_fold, not discarded. #439's head contains this PR as its direct ancestor and has been retargeted to this PR's former base, so interpretation-run GET-by-id implementation/tests and this review history remain intact while queue WIP is reduced. Stored-request vehicle #454 also retains this ancestry.Canonical retrieval landing vehicle: #439 (
feat(api): consolidate interpretation-run retrieval GET and CLI).Do not reopen unless a surviving folded head demonstrably loses unique behavior or evidence.