[COVAL-5823] Model the remaining dropped request fields - #126
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Closes the request-field gaps left after the metric, persona, review, and report work. UpdateAgentRequest gains attributes, customer_agent_id, language, tags, and workflows. CreateAgentRequest already modeled all five, so until now a field could be set when the agent was created and never changed again. The agent merge patch writes only supplied columns and normalizes an explicit null to an empty value, so null is how a caller clears one of these. language, attributes, workflows, and tags therefore take explicit_option; customer_agent_id does not, because the API documents it as non-nullable and rejects null. The six fields the struct already declared have the same clearing problem and are left for COVAL-5829 rather than widening this change. Test sets and run templates gain tags. Uploaded conversations gain tags on submit and metadata on patch. The API patches conversation audio and metadata separately so a rejected metadata key cannot leave audio half-attached, and accepts exactly one target per call, so the command enforces that locally rather than relaying a 400. Deliberately unmodeled: agent_id, persona_id, and test_set_id on the run-template requests. The published spec documents them, but the served model takes the plural arrays the CLI already sends and forbids extra fields, so modeling the documented names would break every call. Spec drift is COVAL-5825.
239fd62 to
9430c33
Compare
1 participant
Depends on #122 for the audit exceptions; the code here stands alone. Shares the
explicit_optionmove with #124 and #125 (identical content, so the later ones drop that hunk).Last of the four modelling PRs. With #123, #124, and #125 it closes all 74 real request-field gaps the audit reports.
Agents
UpdateAgentRequestgainsattributes,customer_agent_id,language,tags,workflows.CreateAgentRequestalready modeled all five, so until now a field could be set when the agent was created and never changed again.AgentUpdatePatch.database_field_updates()ismodel_dump(exclude_unset=True), andnormalize_cleared_json_fields/normalize_cleared_list_fieldsmap an explicit null to{}/[]. Null is therefore how a caller clears one of these, solanguage,attributes,workflows, andtagstakeexplicit_option.customer_agent_iddoes not: the API documents it non-nullable and rejects null.Known, deliberately out of scope: the six fields the struct already declared (
metadata,metric_ids,test_set_ids,phone_number,endpoint,prompt) have exactly the same clearing problem and are still plainOption<T>. Fixing them here would widen this PR past the audit's reported gaps, so they are COVAL-5829.Test sets and run templates
Both gain
tagson create and update. Both follow the "None means don't update;[]clears" contract, so plainOption.Uploaded conversations
submitgainstags(repeatable--tag).patchgainsmetadata, the additive customer-metadata path — the API rejects a key that already has a value.The API patches audio and metadata separately, deliberately, so a rejected metadata key can never leave audio half-attached, and it accepts exactly one target per call.
patchnow enforces that locally with a message naming the flags instead of relaying a 400.Deliberately not modeled
agent_id,persona_id,test_set_idon the run-template requests. The published spec documents them and marks them required on create, butStrictCreateRunTemplateRequest/StrictUpdateRunTemplateRequestforbid extra fields and the served model takes the plural arrays the CLI already sends. Modeling the documented names would break every call. That drift is COVAL-5825, on the API/docs side.Verification
Nine new tests, including the explicit-null clear, the "unset stays omitted" counterpart, and both conversation-patch guard rails.
Running the #122 audit against this tree moves modeled request fields from 280/360 to 291/360 and reports exactly these 11 entries as stale exceptions, with no new extras.
Minor version bump to 0.8.0.
On merge order
Cut from
main. After #122 merges, rebase and delete the 11[[known_field_gap]]entries for agents, test-sets, run-templates (tagsonly — keep the three singular-id entries), and uploaded conversations. Then regenerateapi-coverage-report.mdand re-apply the version bump.test_update_check_notifies_when_outdatedfails under parallelcargo testhere exactly as on cleanmain. Pre-existing, COVAL-5826.