Full CI green — all checks pass, including the 30 integration tests (which exercise the real --query/--format #39
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue38"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Title: Plural wire encodings (json | bson) with capability-gated I/O
Implements the output + input design from #38 and docs/design/IO.md. Today every compiled binary is force-fed
JSON on stdout; this separates the envelope shape (fixed — count, exhausted, solutions, optional output) from
its encoding (plural), and lets a program declare which encodings its binary advertises. Two encodings cover the
space: json (the v1 text wire format, byte-identical) and bson (binary, dense, typed). The author picks the set;
the caller picks within it. Default behaviour — a [json] binary answering --query/--format json — is preserved
byte-for-byte.
What lands
EncoderDesc: encodings exposed as #[repr(C)] vtable statics (PLG_ENC_JSON, PLG_ENC_BSON) rather than a trait,
so codegen can emit a per-binary capability table and --gc-sections can strip unlisted encoders. The json
encoder is lifted verbatim from the old core::write_*_json; the bson encoder maps scalars to native bson types
and term values to BinData(0x00) carrying a copyterm::TermBuf — the single-sourced plg-shared::cell ABI,
lossless including cyclic terms. Also a bson reader for the one-field input request. core.rs shrinks to the
solve side.
already computed), which the bson encoder walks via copy_to_buf. json ignores it. All consumers updated.
parsed in the frontend, merged in the compiler, and emitted as @plg_caps — an array of EncoderDesc pointers —
alongside the atom table and registry. plg_rt_init's signature gains the table; entry.rs resolves --format by
scanning it (exit 2 if not advertised). Unknown encoder names are a build-time error.
text) and emits one self-delimiting bson document. An explicit flush() is required: Rust's stdout is a
LineWriter, and bson's no-newline bytes would sit buffered and never flush on exit — the json path only worked
because its \n triggered a line flush.
reads a one-field request document {"query":"...","limit"?:N} from stdin — the "honest cheat" (the engine
parses the inner string exactly as for argv, so there's no bson-term loader). Input and output encodings are
orthogonal; all four {in}×{out} pairings are legal. The capability table gates both directions; argv --query
(like the human text form) is always available, ungated. argv --limit overrides a doc limit when both are
present.
Load-bearing decisions
CLI and the WASM reactor build an Envelope and drive it through a chosen descriptor, so the shape can't drift
between transports.
entry.rs dispatches through pointers and never names an unused encoder statically, so --gc-sections drops it.
maximally dense, lossless for cyclic terms (the json path cuts X=f(X) to f(_N) for v1 byte-compat; bson
round-trips it). This asymmetry is stated honestly in IO.md rather than hidden.
Verification
byte-identical to v1); footprint gate (hello_world_binary_stays_lean, 1.3M ceiling) and ldd
standalone-contract check green with both encoders available.
input, all four in/out pairings, missing-query/unknown-encoder rejection); ~20 wire.rs unit tests (json
byte-shape, bson framing self-delimits, serialize_termbuf round-trips atoms/lists/cyclic-terms losslessly,
bson-request parsing).
write fns); a [bson]-only binary is the inverse. A [json]-only binary pays zero bson cost.
as BinData, X=f(X) round-trips with the cycle intact while json cuts it).
Out of scope (deliberate, separate PR)
change, so it gets its own PR, ideally with a deprecation shim rather than a hard cutover. The current names
work coherently today.
--format). Native binaries are correct (the reactor isn't linked, so dead-stripping works); for --target
worker a [bson]-only binary still emits JSON from its reactor. Not a regression (the reactor always emitted
JSON), just a loose end worth a one-line note in IO.md on whether/how io_format should apply to the worker
target.
wire path end-to-end through entry.rs), the binary-size footprint gate, and the ldd standalone-contract check. Here's what landed for this first step: crates/runtime/src/wire.rs (new) — the seam. Defines: - Envelope<'a> (the fixed shape: count, exhausted, solutions, program_output) with a from_machine constructor that reads m.solutions and m.captured_output() (so it serves both the streaming CLI and the capture-mode reactor from one constructor). - WireError { Parse, Runtime }. - Encoder trait (write_envelope, write_error, can_stream). - encoder_for(name) registry — currently only "json", with "bson" deliberately returning None (lands in a later PR). - Json encoder impl — byte-identical to the old core::write_solutions_json / write_error_json (5 tests pin the shape). crates/runtime/src/core.rs — shrunk to the solve side: QueryResult, run_query, exhausted. The two JSON writers and their tests moved out; the exhausted test stays. Module doc rewritten to point at wire for the shape/encoding. crates/runtime/src/entry.rs & reactor.rs — both now build an Envelope and drive it through the Json encoder via the Encoder trait, instead of calling core::write_*_json. Behavior unchanged. Deliberately deferred (per the staged plan in IO.md, not this PR): the Request input type, --input-format, the capability table / io_format/1 directive, the Bson encoder, and the CLI flag rename (json→text, human form → --pretty). The seam is in place; everything else hangs off it. Next logical step is the Bson encoder + TermBuf-in-BinData term encoding, but that's a self-contained addition to wire.rs once you're ready. Want me to proceed to that, or pause here?crates/runtime/src/wire.rs — added the Bson encoder: - A hand-rolled bson emitter (bson_doc_begin/end, bson_cstring, element helpers) building into a Vec<u8> (bson is length-prefixed and dyn Write isn't seekable — buffer then flush). - Scalar envelope fields map to native bson types (count→int32, exhausted→bool, output→string) so generic tooling reads them. - Term values are BinData(0x00) carrying serialize_termbuf output: version byte + cell count + root word + cells — the plg-shared::cell ABI verbatim, single-sourced via copyterm::copy_to_buf. - can_stream() == false (binary can't coexist with interleaved text), which forces capture mode. - Registered in encoder_for("bson"). - Tests: bson framing self-delimits; serialize_termbuf round-trips atoms/lists/cyclic-terms losslessly. crates/runtime/src/render.rs — RenderedSolution's binding tuple (name, json, text) became a Binding struct with an added word: Word field (the dereferenced root term, free at capture time). The bson encoder walks it via copy_to_buf; json ignores it. Updated all consumers (control.rs, solve.rs, entry.rs). crates/runtime/src/entry.rs — --format bson now accepted; switches the machine to OutputSink::Capture before running (so write/1 bytes ride in the envelope's output field rather than corrupting the binary stream); routes solutions and errors through the Bson encoder. Added an explicit flush() — the real bug found in validation: Rust's stdout is a LineWriter, and bson's binary output (no trailing newline) sat buffered and never flushed on exit. The json path worked only because its \n triggered an immediate line flush. Flagged in a comment so it doesn't bite again. crates/runtime/src/reactor.rs — write_envelope now takes &Machine (the trait signature change), passed through. Validation done Built a real binary and decoded its bson output with a dependency-free Python struct decoder: - Spec-compliant (pure-Python parser reads it cleanly). - count/exhausted/output are native bson types. - X=bob carries as a 0-cell TermBuf with root word 0xa1 (ATOM tag, id 20) — matches (id<<3)|1. - X=point(3,4) carries as 3 cells (functor header + 2 args). - X=f(X) round-trips with the cycle preserved (cell[1] → STR index 0), while the same term via --format json is cut to f(_1) — proving the IO.md flag #1 losslessness asymmetry through the actual binary. Still deferred (per the staged plan) Capability-table gating (io_format/1 → codegen .rodata, so a [text]-only binary dead-strips the bson encoder — currently every binary links it), --input-format/bson input, and the CLI flag rename (json→text, human form → --pretty).What landed crates/runtime/src/wire.rs — added the bson reader: - ParsedRequest { query, limit } and parse_bson_request(buf) -> Result<ParsedRequest, String>. Parses a one-field request document {query: string, limit?: int32|int64}, the IO.md "honest cheat" — the engine parses the inner query string exactly as for argv --query, so there's no bson-term loader. - Robust: skips unknown fields for forward-compat (via skip_value over the standard bson element types), errors on truncated/malformed documents, ignores trailing bytes (stdin may carry them), requires query to be present and a string. - 7 new unit tests (query-only, int32/int64 limit, unknown-field skipping, missing-query error, truncation, trailing bytes). crates/runtime/src/entry.rs — --input-format text|bson (default text): - parse_args gained the flag; --query is now Option (required only in text-input mode, enforced in plg_rt_main). - plg_rt_main resolves the query by input mode: text → argv --query (required); bson → reads stdin via read_to_end, parses the request, applies the doc's limit (argv --limit overrides if present). Output --format is orthogonal and still works normally — so all four {in}×{out} pairings are legal. - The capability table gates both directions: --input-format bson requires bson advertised (a [json]-only binary rejects it, exit 2), mirroring how argv --query is always available (like the human text display form). - Updated entry tests for the new flag and the now-optional --query. crates/compiler/tests/ — harness gained run_with_stdin (argv + stdin bytes → raw stdout); capabilities.rs gained 6 bson-input integration tests (bson-in/json-out, limit honored via exhausted:false, bson-in/bson-out, capability rejection, missing-query rejection, argv --query still works). 15 capability tests total now. Docs — ARCHITECTURE.md wire contract now describes --input-format and the orthogonality; IO.md status updated to "output and input implemented." Verified end-to-end Through the real compiled binary: {"query":"parent(tom, X)","limit":1} via stdin → {"count":1,"exhausted":false,...} (limit correctly hit); bson-in/bson-out produces valid self-delimiting bson; a [json]-only binary rejects bson input; malformed/missing-query requests exit 2 cleanly.Reviewed the full diff —
wire.rs, theentry.rs/core.rs/render.rs/machine.rs/reactor.rschanges, the frontendio_formatdirective parsing, the codegen capability table, and thecapabilities.rsintegration tests.Overall
Clean and well-factored. Both flags raised on #38 are addressed head-on in the IO.md diff: cyclic-term losslessness is explicitly scoped to bson-only (with the text carve-out documented as deliberate v1 byte-compat), and the input/output orthogonality matrix is spelled out with the
--input-format/--formatseparation and its rationale ("avoid the overload trap of absent--query⇒ read bson"). The descriptor/vtable design over a trait-object is the right call for--gc-sectionsdead-stripping, and thenm-based dead-strip test proves it end-to-end. The LineWriter/flush subtlety is correctly handled. The cyclic-term round-trip test directly exercises the losslessness claim.Three things worth attention before merge, ranked by substance:
1.
WireError::Runtimeis dead, and runtime errors are mislabeled on the wireWireError::Runtimeis never constructed in production code. Both callers hardcodeParse:entry.rsoutput_resultwraps every message inWireError::Parse(...), including theQueryResult::RuntimeErrorarm.reactor.rs— same, for bothParseErrorandRuntimeErrorarms.So a runtime error (exit 3) is serialized as a
Parse-flavored error on the wire, and theRuntimevariant only exists in a unit test. The encoders collapse both variants to the same message anyway (WireError::Parse(m) | WireError::Runtime(m) => m), so today it's invisible — but the enum advertises a distinction the implementation doesn't honor. Either thread the actual class through (output_resultshould take the class and construct the right variant), or collapseWireErrorto a single struct. As-is it's a latent footgun: the moment a bson error document wants akindfield, every runtime error is misreported as a parse error.2. Pre-solve errors escape the wire encoding
There's a seam in error routing. Post-solve errors go through
output_result(enc, ...)and are encoded in the chosen format (bson error document on stdout). But every pre-solve exit-2 path bypasses the encoder and writes plaintext to stderr:--format→eprintln!--input-format bson→eprintln!eprintln!parse_bson_requestfailure →eprintln!("bson request parse error: {e}")So a caller doing
--input-format bson --format bsonthat sends a malformed request document gets a plaintext message on stderr, not a bson error document on stdout — breaking the "I only speak bson" contract at exactly the framing layer where a bson caller lives. Notably, a query-string parse error (exit 2) is encoded, but a bson-request parse error (also exit 2) is not. That asymmetry isn't documented. It may be a deliberate choice ("malformed framing = caller bug, text is fine"), but if so it should be a one-liner in IO.md; if not, the bson-request parse path should route through the chosen output encoder like query-parse errors do.3. The reactor/worker note promised in the PR description isn't in IO.md
The PR description says the Tier-2 reactor ignoring the capability table (a
[bson]-only--target workerbinary still emits JSON) is "not a regression… just a loose end worth a one-line note in IO.md on whether/howio_formatshould apply to the worker target." Grepping the merged IO.md, that note was not added (the onlyreactormention is the pre-existing structure section). Either add the note or file it as a follow-up issue; as-is the promise in the PR body is the sole record of the loose end.Minor nits (don't block)
io_formatdedups in the parser (collect_io_format_specs) and again in codegen (emit_capabilities), whileparse_sourcesmerges across files without dedup. Works, but the responsibility is smeared — pick one layer to own dedup.:- io_format([])silently becomes[json].emit_capabilitiestreats an emptydeclaredslice as "use the default," so an explicit empty declaration is indistinguishable from no declaration. A binary with zero encodings is useless so defaulting is reasonable, but the parser could either reject[]or the defaulting rule deserves a sentence in IO.md.:- io_format([json | bson])parses as[json, bson]because theTerm::Atomarm accepts thebsontail. Harmless, but a malformed spec silently succeeding is the kind of thing that bites later. An explicit "tail must be[]" check would be tidier.can_stream: fn() -> bool. Aboolfield would be simpler and equally const-constructible; the indirection through a function pointer reads like premature extensibility. Not wrong.--format bsonasserting a valid bson error document on stdout).bson_error_document_is_validtests the encoder in isolation; the routing throughentry.rsis unexercised for bson — which is exactly how #1 above could hide.My read: #1 and #3 are merge-blockable (the mislabeling is a real correctness bug if
WireErrorever gains meaning; the missing note is a stated commitment unfulfilled), and #2 is a documented-design-decision call.