Skip to content

fix(conversations): align create and update request contracts - #625

Open
leseb wants to merge 6 commits into
praxis-proxy:mainfrom
leseb:leseb/issue-566-conversation-request-contracts
Open

fix(conversations): align create and update request contracts#625
leseb wants to merge 6 commits into
praxis-proxy:mainfrom
leseb:leseb/issue-566-conversation-request-contracts

Conversation

@leseb

@leseb leseb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Problem

The Conversations create and update request contracts diverged from verified
OpenAI live behavior: create treated items as a required Vec instead of
a nullable optional array, and update accepted absent or null metadata via
a three-variant MetadataUpdate enum instead of requiring a metadata object.

Changes

  • make CreateConversationRequest.items an Option<Vec<ConversationItem>>
    with a nullable bounded-array OpenAPI schema matching upstream
  • replace MetadataUpdate enum with a required Metadata field using a
    custom deserializer that rejects null, matching verified 400 behavior
  • add nullable_metadata_schema and nullable_initial_items_schema for
    precise OpenAPI generation
  • document verified create/update edge cases in conformance README
  • regenerate the conformance report (request drift reduced on create and
    update operations)

Validation

  • make lint (clippy, fmt, deps, docs, examples all clean)
  • cargo test -p praxis-ai-apis -- conversations
  • cargo xtask openai-conformance --area conversations (3/8 exact, 12 exceptions)
  • git diff --check origin/main...HEAD

Closes #566

@leseb
leseb requested review from a team and franciscojavierarceo July 31, 2026 06:06
@leseb leseb added the conformance-failure-acknowledged Acknowledges newly introduced OpenAI conformance failures recorded in the report label Jul 31, 2026
aslakknutsen
aslakknutsen requested changes Jul 31, 2026
View reviewed changes
Comment thread apis/src/openai/conversations/handlers.rs
praxis-bot
praxis-bot reviewed Jul 31, 2026
View reviewed changes

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Correct alignment of create and update contracts with verified OpenAI behavior. The MetadataUpdate enum removal, optional nullable items, and empty-body handling are all internally consistent. The macro syntax change ([none], [required ...], [optional ...]) cleanly disambiguates request body semantics. Conformance documentation with the verification date is a good addition.

CI passes. Two medium findings below on test coverage gaps for the new schema and deserialization paths.

Comment thread apis/src/openai/conversations/openapi.rs Outdated
Comment thread apis/src/openai/conversations/contracts.rs
@leseb
leseb force-pushed the leseb/issue-566-conversation-request-contracts branch from aef5501 to 63291ca Compare July 31, 2026 13:23
@leseb
leseb enabled auto-merge July 31, 2026 13:31
@leseb
leseb requested a review from aslakknutsen July 31, 2026 13:32
praxis-bot
praxis-bot reviewed Aug 6, 2026
View reviewed changes

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Clean alignment of create and update contracts with verified OpenAI behavior. The MetadataUpdate enum removal, nullable optional items, empty-body handling, and macro syntax disambiguation are all internally consistent. Good conformance documentation and test coverage for the new update-requires-metadata invariant.

CI passes. One medium finding on incorrect error messages.

Severity File Finding
Medium handlers.rs classify_update_error hardcodes "got null" for all non-object metadata types

Comment thread apis/src/openai/conversations/handlers.rs Outdated
praxis-bot
praxis-bot reviewed Aug 7, 2026
View reviewed changes

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Correct alignment of create and update contracts with verified OpenAI behavior. The MetadataUpdate enum removal, nullable optional items, empty-body defaulting, and macro syntax disambiguation ([none], [required ...], [optional ...]) are all clean and internally consistent. Good conformance documentation with verification dates.

CI passes. Two medium findings on error response consistency.

Severity File Finding
Medium handlers.rs:856 invalid_input_response now adds "code": null, "param": null to all 400 responses
Medium handlers.rs:178 validate_metadata errors all use "invalid_type" error code

Comment thread apis/src/openai/conversations/handlers.rs Outdated
Comment thread apis/src/openai/conversations/handlers.rs Outdated
@leseb
leseb force-pushed the leseb/issue-566-conversation-request-contracts branch from 63291ca to ffe1315 Compare August 10, 2026 14:13
@leseb

leseb commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@aslakknutsen ptal again :)

praxis-bot
praxis-bot reviewed Aug 11, 2026
View reviewed changes

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review (Round 4)

CI passes. Two new medium findings on conformance gap and schema test coverage.

Severity File Finding
Medium handlers.rs:169 Empty-body update produces generic 400, not missing_required_parameter per conformance README
Medium openapi.rs:156 generated_create_request_contract_matches_runtime does not verify maxItems survived schema restructuring

Comment thread apis/src/openai/conversations/handlers.rs
Comment thread apis/src/openai/conversations/openapi.rs
praxis-bot
praxis-bot reviewed Aug 13, 2026
View reviewed changes

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review (Round 5)

Good conformance alignment. The MetadataUpdate enum removal, nullable optional items, empty-body create handling, and macro syntax disambiguation ([none], [required ...], [optional ...]) are clean and internally consistent. Prior round findings about classify_update_error message accuracy and blanket invalid_type usage have been addressed.

CI passes. One medium finding on error classification robustness.

Severity File Finding
Medium handlers.rs:176 validate_metadata error gate relies on fragile string matching

Comment thread apis/src/openai/conversations/handlers.rs Outdated
leseb added 6 commits August 13, 2026 17:25
@leseb
fix(conversations): align create and update request contracts
5cc9a9e 
Align the locally owned Conversation create and update request bodies with confirmed OpenAI behavior. Create requests accept an absent body and nullable optional fields, while update requests require a non-null metadata object.

Generate the matching OpenAPI schemas from the shared runtime contracts, retain the 20-item limit, cover absent, null, empty, valid, and invalid body shapes, and document the verified live behavior where it contradicts the pinned upstream specification.

Closes praxis-proxy#566

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
docs(conformance): remove unnecessary probe cleanup detail
6cb5e9f 
Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
test(conversations): expect 400 for update without metadata
7f0e6b6 
Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
fix(conversations): return OpenAI error codes on update validation
7f1ad63 
Map update-conversation parse and validation errors to the OpenAI error
contract with code (missing_required_parameter, invalid_type) and param
fields. Add inner anyOf assertions for the two-layer nullable metadata
schema and an array-type rejection test.

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
fix(conversations): narrow error code/param scope in update responses
b0d64fb 
- Remove hardcoded "but got null" from classify_update_error message
  since the error applies to all non-object types, not just null
- Restore invalid_input_response to not include code/param fields,
  keeping those only for update-specific errors via
  invalid_input_response_with
- Use invalid_type code only for actual type violations from
  validate_metadata, not for constraint violations (key/value length,
  key count)

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
fix(conversations): address praxis-bot review findings
9767694 
- Replace string-matching heuristic (msg.contains("must be")) with
  MetadataError enum that structurally distinguishes type errors from
  constraint violations
- Add maxItems assertion to OpenAPI schema test ensuring the 20-item
  bound is preserved in the generated document
- Handle empty update body with missing_required_parameter error code
  matching the {} case, as documented in conformance README

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
leseb force-pushed the leseb/issue-566-conversation-request-contracts branch from ffe1315 to 9767694 Compare August 13, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@praxis-bot praxis-bot praxis-bot left review comments
@franciscojavierarceo franciscojavierarceo Awaiting requested review from franciscojavierarceo franciscojavierarceo was automatically assigned from praxis-proxy/ai-reviewers
@aslakknutsen aslakknutsen Awaiting requested review from aslakknutsen

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

conformance-failure-acknowledged Acknowledges newly introduced OpenAI conformance failures recorded in the report

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Align Conversations create and update request contracts with OpenAI

3 participants

@leseb @aslakknutsen @praxis-bot