BE-585, BE-591: HashQL: Add MIR interpreter test suite and refactor diagnostic categories#8837
Conversation
chore: add new dependency chore: format feat: error module feat: introduce hashql_eval interner chore: checkpoint feat: checkpoint feat: checkpoint chore: remove old value module feat: checkpoint feat: checkpoint feat: checkpoint feat: checkpoint feat: checkpoint chore: checkpoint feat: move entity query into its own modul fix: query request feat: checkpoint (it compiles!) feat: checkpoint feat: checkpoint feat: checkpoint fix: issue around cached thunking feat: covariance for opaque inners fix: cfgattr serde chore: remove graph module
PR SummaryMedium Risk Overview Orchestrator diagnostics are split from interpreter-only types: bridge failures use On the MIR side, Smaller API/doc work: Reviewed by Cursor Bugbot for commit 5bd5e93. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## bm/be-583-hashql-diagnostics-docs-and-span-improvements #8837 +/- ##
===========================================================================================
+ Coverage 59.23% 59.56% +0.33%
===========================================================================================
Files 1346 1347 +1
Lines 130119 130913 +794
Branches 5884 5884
===========================================================================================
+ Hits 77072 77982 +910
+ Misses 52142 52027 -115
+ Partials 905 904 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR wires the MIR interpreter into the compiletest harness for end-to-end execution tests, and refactors diagnostic category/severity plumbing so orchestration-layer (“bridge”) errors are no longer conflated with interpreter diagnostics. It also tightens/clarifies interpreter value invariants (notably struct field ordering), adds an initial mir/interpret UI test, and expands docs/examples across interpreter value types.
Changes:
- Add a new
mir/interpretcompiletest suite that runs MIR through the interpreter and emits MIR as a secondary.miroutput. - Introduce an orchestrator-level diagnostic category (
OrchestratorDiagnosticCategory) that cleanly distinguishes interpreter errors from bridge (PostgreSQL suspension) failures. - Harden interpreter value APIs (e.g.,
Struct::new_uncheckedbecomesunsafe,Struct::newrejects unsorted fields;StructBuilder::finishtakes&InternSet<[Symbol]>) and expand doc-examples/inline hints.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/@local/hashql/mir/tests/ui/interpret/access-struct-through-opaque.stderr | Expected stderr for the first MIR interpreter UI test. |
| libs/@local/hashql/mir/tests/ui/interpret/access-struct-through-opaque.jsonc | New UI test case exercising interpreter end-to-end behavior. |
| libs/@local/hashql/mir/tests/ui/interpret/access-struct-through-opaque.aux.mir | Secondary output snapshot of MIR for the UI test. |
| libs/@local/hashql/mir/tests/ui/interpret/.spec.toml | Declares the new mir/interpret compiletest suite. |
| libs/@local/hashql/mir/src/reify/mod.rs | Re-exports reify diagnostics publicly from the MIR crate. |
| libs/@local/hashql/mir/src/reify/error.rs | Makes ReifyDiagnostic* type aliases public. |
| libs/@local/hashql/mir/src/interpret/value/tuple.rs | Adds doc-examples and small API/inline tweaks for tuples. |
| libs/@local/hashql/mir/src/interpret/value/struct.rs | Marks Struct::new_unchecked as unsafe, enforces sorted fields, refactors StructBuilder::finish signature, adds docs/examples. |
| libs/@local/hashql/mir/src/interpret/value/str.rs | Adds From<Symbol> + TryCloneIn for Str and improves docs. |
| libs/@local/hashql/mir/src/interpret/value/ptr.rs | Adds docs/examples and inline hints for function pointers. |
| libs/@local/hashql/mir/src/interpret/value/opaque.rs | Adds Opaque::into_value and expands docs/examples. |
| libs/@local/hashql/mir/src/interpret/value/num.rs | Expands docs/examples for total-order Num. |
| libs/@local/hashql/mir/src/interpret/value/mod.rs | Adds/updates docs and examples for Value operations (type name, subscript, projection). |
| libs/@local/hashql/mir/src/interpret/value/list.rs | Adds docs/examples for list behavior including negative indexing. |
| libs/@local/hashql/mir/src/interpret/value/dict.rs | Adds docs/examples for ordered dict behavior. |
| libs/@local/hashql/mir/src/interpret/tests.rs | Updates tests for unsafe struct construction and related safety comments. |
| libs/@local/hashql/mir/src/interpret/suspension/temporal.rs | Small inline annotations for timestamp conversions. |
| libs/@local/hashql/mir/src/interpret/runtime.rs | Minor inline change; runtime error-to-diagnostic wiring relies on updated severity/category APIs. |
| libs/@local/hashql/mir/src/interpret/locals.rs | Updates struct construction call site for unsafe Struct::new_unchecked with safety justification. |
| libs/@local/hashql/mir/src/interpret/inputs.rs | Adds inline on Default impl for inputs. |
| libs/@local/hashql/mir/src/interpret/error.rs | Refactors interpreter diagnostics: removes suspension category from interpreter, switches default severity kind to Critical, adds category-lifting APIs. |
| libs/@local/hashql/mir/src/error.rs | Adds a MIR diagnostic category variant for reify diagnostics. |
| libs/@local/hashql/eval/tests/orchestrator/execution.rs | Adjusts error conversion for orchestrator diagnostics. |
| libs/@local/hashql/eval/src/orchestrator/partial.rs | Updates call sites for StructBuilder::finish(&interner.symbols, ...). |
| libs/@local/hashql/eval/src/orchestrator/mod.rs | Updates public API to return OrchestratorDiagnostic and uses category-lifting from interpreter errors. |
| libs/@local/hashql/eval/src/orchestrator/error.rs | Introduces OrchestratorDiagnosticCategory with Interpret/Bridge variants and migrates bridge diagnostics to Critical. |
| libs/@local/hashql/compiletest/src/suite/mod.rs | Registers the new mir/interpret suite. |
| libs/@local/hashql/compiletest/src/suite/mir_interpret.rs | Implements the new mir/interpret suite runner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this PR will not alter performance
Comparing Footnotes
|
a58069f to
e6657ac
Compare
9ab066d to
5bd5e93
Compare

🌟 What is the purpose of this PR?
This PR wires up the MIR interpreter so it can be exercised end-to-end through the compiletest suite, and restructures the diagnostic category hierarchy so that interpreter errors and bridge (PostgreSQL suspension) errors are unified under a single
OrchestratorDiagnosticCategorytype rather than being conflated underInterpretDiagnosticCategory.Previously, bridge errors (query execution failures, row decoding, parameter encoding, etc.) were emitted as
InterpretDiagnosticvalues with aSuspensionsubcategory bolted ontoInterpretDiagnosticCategory. This made it impossible for downstream consumers to distinguish interpreter ICEs from orchestrator bridge failures. The newOrchestratorDiagnosticCategoryenum has two variants —Interpret(forwarding interpreter errors) andBridge(wrapping suspension failures) — giving callers a coherent, single diagnostic type from the orchestration layer.On the interpreter side,
RuntimeError::into_diagnosticis split intointo_diagnostic(same category,InterpretDiagnosticCategory) andinto_diagnostic_with(maps the category through a caller-supplied closure), enabling the orchestrator to embed interpreter diagnostics inside its own hierarchy without losing information.A new
mir/interpretcompiletest suite is added that runs the full pipeline through the MIR interpreter and captures the output, with an initial test demonstrating that field projection through an opaque wrapper is not yet supported.Additionally,
StructBuilder::finishnow accepts&InternSet<[Symbol]>directly instead of&Interner,Struct::new_uncheckedis markedunsafewith documented safety requirements, all interpreter diagnostic severities are migrated fromSeverity::Bug/Severity::ErrortoCritical::BUG/Critical::ERROR, and comprehensive doc-examples are added across the interpreter value types (Dict,List,Struct,Tuple,Opaque,Ptr,Str,Num).🔍 What does this change?
OrchestratorDiagnosticCategorywithInterpretandBridgevariants, replacing the previous use ofInterpretDiagnosticCategory::Suspensionfor bridge errors.BridgeDiagnosticCategoryas the subcategory for suspension-fulfillment failures, replacingSuspensionDiagnosticCategory.SuspensionDiagnosticCategoryand theSuspensionvariant fromInterpretDiagnosticCategory.RuntimeError::into_diagnosticintointo_diagnostic(identity category mapping) andinto_diagnostic_with(caller-supplied category mapping), allowing the orchestrator to lift interpreter diagnostics into its own hierarchy.InterpretDiagnosticdefault severity kind fromSeveritytoCritical, and migrates all diagnostic constructors fromSeverity::Bug/Severity::ErrortoCritical::BUG/Critical::ERROR.Struct::new_uncheckedunsafe, documenting that callers must ensure fields are sorted; updates all call sites withunsafeblocks and safety comments.Struct::newto also reject unsorted field slices.StructBuilder::finishto accept&InternSet<'heap, [Symbol<'heap>]>instead of&Interner, narrowing the dependency.mir/interpretcompiletest suite that runs the interpreter and captures MIR as a secondary output.access-struct-through-opaque) that documents the current limitation of projecting through opaque wrappers.ReifyDiagnosticCategory,ReifyDiagnostic, andReifyDiagnosticIssuespublicly and addsMirDiagnosticCategory::Reifyvariant.Opaque::into_value,From<Symbol> for Str, andTryCloneIn for Str.#[inline]annotations and comprehensive doc-examples across interpreter value types.🛡 What tests cover this?
mir/interpretcompiletest suite withaccess-struct-through-opaqueas the first test case, covering the interpreter pipeline end-to-end.StructBuilderunit tests updated to use the newfinishsignature.unsafeStruct::new_uncheckedcall sites.❓ How to test this?
mir/interpret.access-struct-through-opaquetest produces the expected stderr output showing theinterpret::type-invariantdiagnostic.