Fix two issues - #66
Merged
Merged
Conversation
Replayed assistant tool calls now reach the model endpoint in the same function-call shape the engine already parses back, so a strict OpenAI-protocol upstream accepts the follow-up turn that carries tool results instead of rejecting it. The projection rendered those calls in a neutral shape the endpoint's schema rejects; aligning it to the inbound contract closes the dialect gap, and the test gateway now enforces that contract so a future regression fails loudly.
- `crates/promptforge/lua/src/projection.rs` — replayed calls now render as `{id, type, function: {name, arguments}}`, with `arguments` a JSON-encoded string produced by the infallible `Value::to_string` so a serialization miss cannot silently blank it.
- `assert_openai_tool_calls` — a pure validator mirroring the inbound parser; the scripted gateway runs it on every recorded body and answers 400 with a diagnostic naming the offending path, so a shape regression fails the suite instead of reaching a real endpoint.
- `replayed_tool_calls_reach_the_mock_gateway_in_the_openai_shape` — a tool round plus its follow-up now completes through the schema-checking gateway, and the test pins the replayed call's id, discriminator, name, and re-decoded arguments on the second request.
- `crates/promptforge/lua/src/protocol/request.rs` — the neutral stored record is unchanged; only its documentation and the wire rendering move, so replay and determinism are unaffected.
Design: new pure-function @ crates/promptforge-api-runtime/src/execute/tests.rs::assert_openai_tool_calls deps: Value
Repairs: OpenAI function-call wire shape for replayed assistant tool_calls @ crates/promptforge/lua/src/projection.rs::wire_message - replayed tool calls rendered as the neutral {id, name, arguments} triple were rejected with 400 by strict OpenAI-protocol endpoints
Plan: vibe/2026-09-22-1-replayed-tool-calls.md
A non-success upstream response now emits one structured log line carrying the status and the upstream error code, type, and message, at warn level for a server error and debug level for a client error. The raw response body never reaches the log, because an upstream error can echo prompt content or credentials. Each diagnostic field is bounded and control-escaped, so a crafted message cannot forge log lines, inject terminal controls, or flood the log, and a body outside the expected error envelope yields no diagnostics at all. - `UpstreamErrorDiagnostics` decodes only the upstream error envelope in `from_body`, yielding empty fields for any other shape so the raw body has no path into the log even as a fallback. - `bounded_error_field` renders a missing or non-string field as the empty string rather than a fallback that could leak the body. - `escape_control` escapes newlines, carriage returns, tabs, and other control characters, keeping at most `MAX_ERROR_MESSAGE_CHARS` input characters before escaping can expand them. - `capture_logs` takes the subscriber level and `capture_warnings` delegates to it, so the new tests assert both the warn and the debug paths. - `crates/gateway/protocol/src/upstream.rs` logs one structured event per failed response, at warn for a server error and debug for a client error, with `status`, `code`, `type`, and the escaped `error.message`. Design: new encapsulated-invariant @ crates/gateway/protocol/src/upstream.rs::UpstreamErrorDiagnostics Design: new pure-function @ crates/gateway/protocol/src/upstream.rs::bounded_error_field deps: Option<&serde_json::Value> Design: new pure-function @ crates/gateway/protocol/src/upstream.rs::escape_control deps: &str, usize Plan: vibe/2026-09-22-1-replayed-tool-calls.md
Plan: vibe/2026-09-22-1-replayed-tool-calls.md
An inference round emitted a separate infer_reply event whose payload was identical to assistant_reply, so every consumer had to learn a second kind and any wildcard match silently dropped it. Fold the distinction into the existing reply event as a typed origin, so one event covers both and a host that cares branches on origin. - Event::AssistantReply gains origin: ReplyOrigin (Chat | Infer), serde defaulted to Chat, so an old reader ignores it and a new reader parses old logs. - Emitter::assistant_reply takes the origin; Emitter::infer_reply is gone. - support::report_model_turn passes the origin; the chat path emits Chat and the infer path emits Infer, through one shared reporting sequence. - The session sink settles and reply-stamps the single reply variant. Plan: none
The agent guide and the papergate migration record described a separate inference reply kind that no longer exists. Record the reply origin instead, and mark the emit-infer-reply plan as superseded by the merged design. - guide/src/agent/05-the-event-log.md documents origin (chat or infer) on a model round's reply; the generated guide is regenerated. - vibe/papergate-harness-migration.md reads assistant_reply with origin infer. - vibe/2026-09-22-2-emit-infer-reply.md aligns to the merged design and marks its distinct-kind step record as superseded. Plan: none
gregjkal
approved these changes
Sep 22, 2026
| "upstream returned a server error" | ||
| ); | ||
| } else { | ||
| tracing::debug!( |
Contributor
There was a problem hiding this comment.
The gateway's default filter (DEFAULT_LOG_FILTER in app/src/main.rs) runs the gateway crates at info, so an upstream 4xx still logs nothing unless the operator sets RUST_LOG=debug. I think this should be info (or warn, matching the 5xx arm).
Contributor
|
Verified both fixes manually against an OpenAI-protocol upstream (OpenRouter, gpt-4o-mini) through a gateway built from this branch, driving papergate built against the branch's harness-api. The tool-calling run now completes through the replay turn that previously returned 400 (#64), and the tool-less models.infer prompt emits assistant_reply and returns its text (#65). |
Raise the level of the upstream 4xx log line in `OpenAiUpstream` from `tracing::debug!` to `tracing::info!` so client errors appear in default log output. Also move the 36 September plan files from `vibe/` into a `vibe/2026-09/` subdirectory with no content changes. - The 5xx branch stays at `tracing::warn!`; only the client error branch changes level. - No tests touch the log level.
Enabling exact float parsing on the workspace JSON dependency makes a payload read back as the identical value its stored text represents, so replay can compare re-executed results against stored records without a silent loss of precision. The generated workspace feature set is regenerated to carry the same setting, and a build-only dependency that the regenerated graph no longer requires is dropped. - `Cargo.toml` enables exact parsing once on the workspace JSON dependency rather than at each call site, so every consumer inherits the guarantee. - `crates/workspace-hack/Cargo.toml` is regenerated to carry the same feature, and the regenerated set no longer includes `time-macros`. Plan: vibe/2026-09-22-3-replay-json-fidelity.md
The model client now rounds each measured timing to a whole microsecond where it is computed, so the text a run log stores is short and parses back exactly on replay. A timing that carried nanosecond noise used to be written as a long decimal expansion that replay did not need. - `round_to_microsecond` is a new pure helper that scales a fractional-millisecond value by a thousand, rounds it, and scales back; `duration_ms` now routes every duration through it. - `mean_itl_ms` is rounded after the division by the interval count, so a repeating quotient is trimmed before it reaches the log. - `ClientTiming` keeps its fields and public shape; only the values it carries are rounded. Design: new pure-function @ crates/promptforge/model-client/src/client/read.rs::round_to_microsecond deps: f64 Design: extends pure-function @ crates/promptforge/model-client/src/client/read.rs::duration_ms deps: Duration Repairs: logged timing text is short and round-trips exactly @ crates/promptforge/model-client/src/client/read.rs::duration_ms - a timing carrying nanosecond noise serialized as a long decimal expansion Plan: vibe/2026-09-22-3-replay-json-fidelity.md
The run log writes each record's payload as JSON text and parses it back on read, and replay compares the parsed value against the one that produced it, so a value that does not survive the round trip would make a replayed run differ from the run that ran. This adds integration tests that hold a payload to that standard across awkward floating-point values, objects built with keys in non-sorted order, arrays of mixed numbers, and a realistic assistant reply carrying its timing metrics. The stored text is checked to stay canonical as well as the value to stay identical, and the guarantee is now recorded beside the crate's other invariants. - `crates/harness/log/tests/it/fidelity.rs` is a new integration suite, wired in by the `mod fidelity;` line added to `crates/harness/log/tests/it/main.rs`. - `promptforge-api-types` is added as a dev-dependency so the suite can build a real `Event::AssistantReply` carrying `CallMetrics`. - `round_trips` appends one event to a fresh in-memory run and asserts both that the returned value equals the original and that their serialized text matches. - `AWKWARD` names a double whose shortest decimal text a plain parser reads back as a neighbouring value, so the float cases fail unless parsing is exact. - `crates/harness/log/src/lib.rs` gains a documented invariant only; the writer, reader, and stored payload shape are unchanged. Plan: vibe/2026-09-22-3-replay-json-fidelity.md
Move the deterministic ordering of a Lua table's hash keys into its own crate-internal module, so one classifier and one comparator define that order in a single place. The classifier reports why a key cannot be ordered, and each caller turns that reason into its own message. The enumeration path now reuses the shared helper. - `crates/promptforge/lua/src/collection-order.rs` is a new crate-internal module; `collection.rs` declares it, re-exports `SortKey`, `sort_key`, and `KeyError`, and the ordering tests move to a sibling test file. - `compare_integer_float` moves into the new module unchanged, keeping an integer ordered exactly against a nearby float without rounding the integer. - `sort_key` classifies one Lua value into a sort position and a label, and keeps an integer key an integer rather than a float. - `KeyError` names three failures, a non-finite number, a non-UTF-8 string, and an unsortable type, and `collection_members` maps each onto fanout's own message. - `crates/promptforge/lua/src/collection.rs` keeps every extracted item crate-internal, so no public API is added. Design: new pure-function @ crates/promptforge/lua/src/collection-order.rs::sort_key deps: Value Design: replaces pure-function @ crates/promptforge/lua/src/collection-order.rs::compare_integer_float deps: f64,i64 was: crates/promptforge/lua/src/collection.rs::compare_integer_float Plan: vibe/2026-09-22-3-replay-json-fidelity.md
Lua leaves a table's hash traversal order unspecified, so two runs can visit the same table differently. This change replaces a section runtime's iterator and pair-traversal functions with one deterministic walk: the array part in index order, then the remaining keys by a fixed cross-type order, with a metamethod still taking precedence. A walk that loses its current key partway through resumes at the next live key instead of stopping or repeating. - `ordered_keys` fixes the walk order: the array part (`1..=#t`) in index order, then scalar hash keys by `SortKey`, then any non-scalar keys. - `install_deterministic_iteration` replaces the `next` and `pairs` globals on the supplied VM and surfaces an error when either global cannot be set. - `pairs` invokes a table's `__pairs` metamethod when one is present, as stock Lua does, and otherwise returns the installed `next`, the table, and a nil cursor. - `next` is stateless: `ordered_keys` rebuilds the live key sequence on every call, so a key cleared to nil between steps is skipped and the walk continues. - `next_index` resumes at the first key sorting strictly after a previous key that is no longer present, so clearing the current key does not end the traversal. - `iteration` has no production caller in this change; `lib.rs` gates it with `#[cfg_attr(not(test), expect(dead_code))]`, so a section VM still iterates with the stock functions. Design: new pure-function @ crates/promptforge/lua/src/iteration.rs::ordered_keys deps: Table Design: new pure-function @ crates/promptforge/lua/src/iteration.rs::next_index deps: Value Plan: vibe/2026-09-22-3-replay-json-fidelity.md
Every section virtual machine now installs a deterministic table walk as it is constructed, so two runs over the same table visit its keys in the same order instead of following Lua's unspecified hash order. The installation replaces the machine's iterator and pair-traversal functions during construction, positioned after the sandbox is applied. A new test proves that two freshly built machines over one table produce the identical key sequence, which is the stability replay comparison depends on. - `install_deterministic_iteration` is now called from the section constructor after `harden` and before `install_untrusted`, so every section virtual machine walks tables in the sorted order rather than Lua's hash order. - `crates/promptforge/lua/src/lib.rs` removes the temporary dead-code expectation that stood in for this caller and re-exports the installer at the crate root. - `two_fresh_section_vms_yield_the_same_key_order` builds two fresh machines over one five-key table and asserts both return the same sorted key sequence. Design: new surface-growth @ crates/promptforge/lua/src/vm.rs::SectionVm::new Design: extends facade @ crates/promptforge/lua/src/lib.rs Repairs: deterministic iteration order @ crates/promptforge/lua/src/vm.rs::SectionVm::new - two fresh section virtual machines visited one table in different key orders Plan: vibe/2026-09-22-3-replay-json-fidelity.md
Replay comparison needs every payload to survive the log round trip exactly, but a non-finite floating point value has no JSON number form and is written as null, silently changing the value that was stored. Object keys need the same guarantee, because if insertion order ever reached the log two equal payloads could print differently. These tests pin both facts against the serializer so a later change that enables insertion-order preservation is caught. - `a_non_finite_f64_serializes_to_null` asserts that NaN, infinity, and negative infinity each serialize to the JSON literal null, and that a metrics record carrying a non-finite `mean_itl_ms` reaches the log as null instead of the value that went in. - `object_keys_serialize_in_canonical_order` asserts that a serialized object emits its keys sorted rather than in insertion order, since the value type is a sorted map and no crate enables insertion-order preservation. Plan: vibe/2026-09-22-3-replay-json-fidelity.md
The audit reviews every Rust-side walk over a Lua table that builds an ordered array rather than a value. Only the tool-parameter schema builder was leaving its output in the runtime's unspecified hash order, so it now sorts the declared names before emitting the schema. The other walks either already sort their output or only assign into a table or map, where the walk order cannot affect the result. That makes a stored tool definition's schema text identical in every process, so replay can compare it. - `required.sort()` imposes a bytewise order on the schema's required names, matching the shared `SortKey::Text` order the other ordered walks already use. - `add_local_params_schema` is the only ordered pair walk that needed the sort; `ordered_keys` and `collection_members` already sort their output, and `properties` sits in an order-stable map. - `argv.rs`, `coro.rs`, `error-value.rs`, `prose.rs`, and `sys.rs` gained only a recorded justification in each copy walk, because every assignment lands in a table or map whose contents are independent of walk order. Design: new pure-function @ crates/promptforge/lua/src/tools/decode.rs::add_local_params_schema deps: Table Repairs: the schema required array is emitted in a deterministic sorted order @ crates/promptforge/lua/src/tools/decode.rs::add_local_params_schema - the array followed the runtime's unspecified hash order, so the same parameters produced different schema text in two processes Plan: vibe/2026-09-22-3-replay-json-fidelity.md
Record the repository rule that JSON reaching the run log or a replay comparison round-trips exactly, with canonical sorted object keys, finite numbers, and floats rounded to their meaningful precision at the source. Document that table iteration visits keys in a fixed sorted order, so every process observes the same sequence for the same table. - `AGENTS.md` states the round-trip rule: value identity across `to_value`, `to_string`, and `from_str`, canonical sorted object keys, finite numbers, floats rounded to meaningful precision at the source, and `preserve_order` never enabled. - `pairs` and `next` visit keys in a fixed sorted order - the array part first in index order, then booleans, numbers, and strings - recorded in both the language guide source and its `guide/src` chapter. Plan: vibe/2026-09-22-3-replay-json-fidelity.md
A gateway test expected a caller-side client error to be logged at debug, but the code records those errors at info, so the assertion could not pass. The test now checks the info level and keeps asserting that no warning is emitted, leaving the warn stream reserved for server-side failures. - `client_error_logs_at_info_not_warn` asserts that a 4xx is logged at info and never at warn. Plan: vibe/2026-09-22-3-replay-json-fidelity.md
Plan: vibe/2026-09-22-3-replay-json-fidelity.md
The deterministic pairs/next walker could revisit keys or end the walk early when a key was cleared during traversal: its ordered key list and its resumption comparator disagreed, and resumption required the previous key to be sortable. Share one walk order between the list and the resumption, and let pairs advance a captured snapshot, so a cleared key of any type is skipped and the walk stays strictly forward. - iteration.rs: one array < scalar-hash < non-scalar walk order; next is strictly forward for present keys with a best-effort fallback. - pairs returns (iterator, table, nil) and advances the snapshot by index. - iteration-tests.rs: the boolean-clear and non-scalar-clear repros plus next-path coverage; all fail against the pre-fix walker. Plan: none
Two documentation claims no longer matched the code: the replay-fidelity plan's audit line called the decode.rs walk canonical even though its required array was unsorted and is now sorted, and the Engineering rule implied every logged float is rounded at the source when only derived client timings are. State each accurately. - vibe/2026-09-22-3-replay-json-fidelity.md: the decode.rs audit line. - AGENTS.md: separate the exact-parsing guarantee from source rounding. Plan: none
This branch was successfully deployed
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
2 participants
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.
No description provided.