feat!: serve the System One wire shape; move to reqwest 0.12 / http 1 (0.2.0) - #6
Merged
Merged
Conversation
OAG, an LLM gateway on reqwest 0.12 / http 1 / axum 0.8, has to serve `POST /v1/systemone` and `GET /v1/models` and also call a Jev upstream with this SDK. 0.1 blocked both. It pulled in a second HTTP stack (reqwest 0.11 / http 0.2), so `http_client` could not take the gateway's client and share its pool. Its response types were decode-only: Question had no Deserialize, the answers had no Serialize, and SystemOneResponse had private fields and no constructor. A server could not read a request or write a response with them. Dependencies: reqwest 0.12 (json + rustls-tls), http 1, wiremock 0.6. The library compiled unchanged against the http 1 HeaderMap. Only the contract test's header helper needed wiremock 0.6's API. MSRV stays 1.85. The library and all tests pass on 1.85.0 with a lockfile resolved for 1.85. wiremock 0.6.5 and yoke-derive 0.8.3 need newer compilers without declaring it, and the CHANGELOG says how to pin around them. New `wire` module: SystemOneRequest, ModelsResponse, the two endpoint paths, and re-exports of Question, SystemOneResponse, Answer and the answer types. All have serde impls that match the wire, plus constructors. Question deserializes to a typed variant only when that variant holds the object exactly, and to Raw otherwise, so nothing is dropped. SystemOneResponse deserializes through the client's own decoder. One implementation means the same leniency: an unknown answer type is skipped with a warning. The client now builds its body as a SystemOneRequest. Its Serialize applies `extra` keys in place, last write wins, as the old Map merge did, so the bytes on the wire are unchanged. request_body_bytes_are_stable pins them. It was recorded against the 0.1.2 client before the refactor, and it fails on a pure key reorder, which JSON-value equality would miss. The docs claimed serde_json was pinned to =1.0.134. Cargo.toml requires 1.0.134 as a minimum, and the docs now say that. BREAKING CHANGE: the public API uses the http 1 HeaderMap (ClientBuilder::headers, SystemOneOpts/ModelsOpts::extra_headers, ApiError::headers, blocking::Builder::headers). ClientBuilder::http_client takes a reqwest 0.12 Client. NoulCriteria serializes with the wire keys true/false. The built-in client no longer negotiates HTTP/2 or reads OS proxy settings, because reqwest 0.12 moved both behind the http2 and system-proxy features.
reqwest 0.12 moved them behind features; enabling only json and rustls-tls dropped HTTP/2 and the macOS/Windows proxy settings that 0.1.x users had. Enable http2, system-proxy and charset alongside them, still on rustls.
Owner
Author
|
Restored |
6385577 turned on reqwest's http2 and system-proxy features, but the README still said the default client speaks HTTP/1.1, ignores OS proxy settings, and that callers should enable those features themselves. A reader would have added features the crate already enables, or built a custom client to get behaviour they already had. Say what the default client does now: rustls, HTTP/2 when an https server offers it, proxies from HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY with the macOS or Windows settings as the fallback. Passing your own client stays the way to change any of it. AGENTS.md and docs/ had no such claim.
Review of #6: Usage and ModelMetadata had two decoders that disagreed. serde used the derives, while Client used the hand-written decode_usage and decode_model. A token count above i64::MAX failed through serde and wrapped in the client. A repeated key failed in one and took the last value in the other. The error messages named different paths. A server built on the wire types could accept or reject a body differently from the client it proxies for. The models listing had the same split: wire::ModelsResponse against Client's ListModelsResponse. The answer structs NoulAnswer, ChoiceAnswer and ScoreAnswer had it too. SystemOneResponse already did this right, by deserializing through the client's decoder. Every response type now does the same. The client's field decoders take the object (noul_fields, choice_fields, score_fields, usage_fields, model_fields, decode_model_list). decode_answer, decode_fields and decode_models call them with the paths the client has always reported. Each Deserialize impl calls the same function, so there is one behaviour. It is the client's, lenient behaviour included. Client error messages and field paths do not change. ListModelsResponse is now the one models type, as SystemOneResponse is the one response type. It has Serialize, a Deserialize through decode_model_list, and new(). A gateway can re-serve what client.models() returned. ModelsResponse, which was new in this unreleased version, is gone. tests/wire.rs serves edge-case bodies to the real Client over wiremock. They cover null and absent fields, counts outside i64, repeated and unknown keys, unknown answer types, and a bad value at each level. The tests require serde to give the same value or name the same field, for whole responses and listings and for Usage, Answer, each answer struct and ModelMetadata on their own. Put the old derives back on Usage and ModelMetadata and both tests fail on the cases above. BREAKING CHANGE: Deserialize for Usage, ModelMetadata, NoulAnswer and ChoiceAnswer (derived in 0.1.2) now runs the client's decoder. A repeated key takes its last value, a count above i64::MAX wraps, error text names the field, and the input format must be self-describing. wire::ModelsResponse is replaced by ListModelsResponse. It was never released.
Review of #6: the docs promised round trips they did not deliver. Question::typed() accepted a variant whenever it held the same keys and values. So {"instructions":"x","type":"noul"} became Noul and serialized back as {"type":"noul","instructions":"x"}. Noul criteria written {"false":..,"true":..} came back in the other order. A gateway that deserializes a request and forwards it changed the bytes, even though the Question docs said a typed variant serializes back to the same keys and values. typed() now serializes its candidate and compares it with the input, key order included, all the way down. Anything that would come back different stays Raw, which keeps the input as it was. ScoreAnswer.legend and .probabilities were BTreeMap<u32, _>, so a wire {"0","10","2"} came back {"0","2","10"} through serde and through the client. They are IndexMap<u32, _> now, in wire order, both in the decoder and in ScoreAnswer::new. Keys stay u32, so a non-canonical "01" still reads as 1 and writes as "1". The type's docs say so, and they warn that IndexMap's legend[1] is positional. Lookups by key (legend[&0], get(&2)) compile unchanged. The response types stay lenient. A null usage count reads as None, and unknown keys on the response, answers, usage, listing and model entries are ignored. So those do not survive a round trip either. wire.rs now has a Round trips section listing exactly what serializes back byte for byte (a question always, a request whose keys start state, model, questions) and what does not. CHANGELOG, README and AGENTS.md match it. AGENTS.md no longer says a forwarded request is always byte-identical: without a model, the client sends its default. Tests: reordered questions stay Raw and round-trip. Score maps keep wire order through serde and through the client. The response and request normalisations above are pinned input to output. Letting typed() skip the comparison, or sorting the score maps, fails three of them. BREAKING CHANGE: ScoreAnswer::legend and ScoreAnswer::probabilities are IndexMap<u32, _> instead of BTreeMap<u32, _>. Iteration follows wire order, not index order.
…s not breaking Review of #6. Deserializing a SystemOneRequest is stricter than a map. The CHANGELOG said only that it "rejects any question the client would refuse to send", and the wire module docs said nothing. A server author could not tell which bodies get a 4xx without reading question.rs. Both now list every rejection. The list: a missing or non-string/object/array state, a model that is neither a string nor null, an empty questions object, a question without a nonempty string type, choice or score without criteria, and score with an empty criteria array. They also note that the client runs the same checks before it sends, except where extra_body replaces a field. The existing rejection test now covers the number state, number model and criteria-less score cases the list names. The CHANGELOG listed the wiremock 0.5 -> 0.6 move under Changed without saying whether it breaks anyone. It does not. The mock feature only enables the optional wiremock dependency for the examples, and src/ never names a wiremock type, so no wiremock type is in this crate's API. The entry now says so, and points at the MSRV pin that mock needs on 1.85.
Owner
Author
|
Changes in response to the review:
59 tests pass. They also pass on 1.85.0 with the documented pins. clippy, fmt, examples, bench |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
OAG, an LLM gateway on reqwest 0.12 / http 1 / axum 0.8, will serve the System One contract (
POST {base}/v1/systemone,GET {base}/v1/models) and call a Jev upstream with this SDK. 0.1 blocked both:ClientBuilder::http_clientcould not take the gateway's 0.12 client and share its pool.Questionhad noDeserialize. The answer types had noSerialize.SystemOneResponsehad private fields and no constructor. A server could not read a request or write a response with them.This is release 0.2.0. The version is bumped here. Nothing is published, tagged, or released. That is the owner's call.
Dependency change
default-features = false, featuresjson,rustls-tls,http2,system-proxy,charsetmock)reqwest 0.12 moved HTTP/2, the macOS/Windows system proxy, and charset decoding behind features. They are enabled explicitly, so the built-in client keeps what 0.1.x had. It negotiates HTTP/2 with
httpsservers that offer it and falls back to HTTP/1.1. It readsHTTP_PROXY,HTTPS_PROXY,ALL_PROXY, andNO_PROXY, and on macOS and Windows falls back to the OS proxy settings. TLS is still rustls, not the native TLS that reqwest's own defaults pull in. To change any of that, pass your own client tohttp_client.src/compiled unchanged against the http 1HeaderMap. Only the contract test's header helper needed wiremock 0.6's API. These versions match OAG's lock (reqwest 0.12.28, http 1.5.0, hyper 1.11).cargo tree -dshows no duplicate reqwest, http, hyper, or h2. Two unrelated duplicates remain.base640.22/0.23 is split inside the reqwest 0.12.28 / hyper-util 0.1.21 stack itself.syn2/3 comes from criterion (dev) against url/icu.serde_jsonstays at"1.0.134"withpreserve_order. README and AGENTS.md wrongly called it an exact=1.0.134pin. They now describe the minimum and the feature unification.New
wiremoduletypesafe_sdk::wireholds the same types the client uses, with serde impls that match the wire JSON:SystemOneRequest { state: JsonContent, model: Option<String>, questions: IndexMap<String, Question>, #[serde(flatten)] extra: Map<String, Value> }. Serialize writesstate,model,questions, then theextrakeys. Anextrakey namedstate,model, orquestionsreplaces that field in place (last write wins), so the output never repeats a key.Question: newDeserialize. An object becomesNoul,Choice, orScoreonly when that variant serializes back to the same bytes, key order included. Anything else, such as an unknown type, an extra key, orinstructionsbeforetype, staysRaw. So a deserialized question always serializes back unchanged.Serializeis byte-identical to 0.1.SystemOneResponse,ListModelsResponse,Answer,NoulAnswer,ChoiceAnswer,ScoreAnswer,Usage(aNonecount is left out), andModelMetadata. They getSerialize, and aDeserializethat runs the client's own decoder. For each type there is one decoding function, called by bothClientand serde. They accept the same input, give the same values, and name the same field on error (Invalid response data at 'usage.input_tokens'.). An unknown answer type inside a response is skipped with a warning, as the client does.ListModelsResponseis the one models type, asSystemOneResponseis the one response type.Client::modelsreturns it. A server builds it withListModelsResponse::newand serializes it. A gateway can re-serve whatclient.models()returned. Built or deserialized values have norequest_id()and an emptyraw_body().SystemOneResponse::new(model, usage, answers),ListModelsResponse::new,NoulAnswer::new,ChoiceAnswer::new,ScoreAnswer::new,Usage::new,ModelMetadata::new.SYSTEM_ONE_PATHandMODELS_PATH, for mounting handlers.SystemOneRequestis also re-exported at the crate root. Everything else was already there.Deserializing a request rejects:
state, or one that is not a string, object, or arraymodelthat is neither a string nornullquestionsobjecttypechoiceorscorequestion withoutcriteriascorequestion with an emptycriteriaarrayThose are the checks the client runs before it sends.
Round trips. The
wiremodule docs have a section on what deserialize-then-serialize keeps. Serializing writes compact JSON. Beyond that:A
Questionalways round-trips.A
SystemOneRequestround-trips when its keys start withstate, thenmodelif present, thenquestions. Otherwise the known keys move to the front and anullmodelis left out.Response types keep only what the client decodes, so these do not survive a round trip:
nulltoken countsf64field written1comes back as1.0"01"comes back as"1"i64::MAX: they wrap to a negative number, as the client always hasA missing
usageoranswerscomes back as{}. Each of these is pinned intests/wire.rs.The client now builds its body as a
SystemOneRequest. It serializes straight to bytes rather than through an intermediateValue, sostateis not deep-copied.API breaks (all in CHANGELOG.md)
HeaderMap(ClientBuilder::headers,blocking::Builder::headers,SystemOneOpts::extra_headers,ModelsOpts::extra_headers,ApiError::headers). It is the same type asreqwest::header::HeaderMapin 0.12.ClientBuilder::http_clienttakes a reqwest 0.12Client.NoulCriteriaserializes with the wire keystrue/falseinstead oftrue_meaning/false_meaning.Questionoutput is unchanged.ScoreAnswer::legendandScoreAnswer::probabilitiesareIndexMap<u32, _>, notBTreeMap<u32, _>. They iterate and serialize in wire order, not index order. Key lookups (legend[&0],probabilities.get(&2)) compile unchanged.legend[0]now also compiles, asIndexMap's positional index. The type docs warn about it.DeserializeforUsage,ModelMetadata,NoulAnswer, andChoiceAnswerwas derived in 0.1.2. It now runs the client's decoder. A repeated key takes its last value, a count abovei64::MAXwraps, errors name the field, and the input must be a self-describing format such as JSON.Not breaking: the
mockfeature moves to wiremock 0.6, but it only enables the optional dependency for the examples. No wiremock type is in the crate's API.Everything else in the 0.1 client API is source-compatible.
MSRV
Unchanged at 1.85. reqwest 0.12 does not force a bump. With
yoke-derivepinned to 0.8.2 andwiremockto 0.6.4,cargo +1.85.0 test --all-featurespasses all 59 tests at the head of this branch. Those two crates need newer Rust without declaring arust-version:yoke-derive0.8.3 (via url) needs 1.87, andmainhas the same problem today.wiremock0.6.5 needs 1.88 forletchains. The CHANGELOG has the pin commands.Verification
cargo test --all-features: 59 passed, 0 failed, 1 ignored (the existingrust,ignorecrate example). That is 11 unit, 16 contract, 10 example integration, 21tests/wire.rs, and 1 doctest insrc/wire.rs.request_body_bytes_are_stable(tests/contract.rs) asserts the exact request bytes for typed questions,Rawquestions, andextra_bodyoverrides ofstate,model, andquestions. It was recorded against the 0.1.2 client before the refactor, and it fails on a pure key reorder.tests/wire.rssuite covers the following:serde_reads_responses_exactly_as_the_client_doesandserde_reads_models_exactly_as_the_client_doesserve edge-case bodies to the realClientover wiremock. The cases include absent andnullfields, counts outsidei64, repeated and unknown keys, unknown answer types, and a bad value at each level. serde must produce the same value or name the same field. That holds for whole responses and listings, and forUsage,Answer, each answer struct, andModelMetadataalone.reordered_questions_stay_raw,score_maps_keep_wire_order,response_round_trips_keep_only_what_the_client_decodes, andrequest_round_trips_put_the_known_keys_firstpin the round-trip rules above.Usage, and listings.DeserializeonUsageandModelMetadatafails both equivalence tests, on theu64wrap and the repeated key.typed(), or sorting the score maps, fails three tests.cargo clippy --all-targets --all-features -- -D warnings,cargo fmt --check,cargo build --examples --features mock,cargo bench --no-run, andRUSTDOCFLAGS='-D warnings' cargo doc --no-deps --all-featuresall pass.