feat: per-request interpreter architecture (DD-006)#17
feat: per-request interpreter architecture (DD-006)#17larimonious wants to merge 11 commits intomainfrom
Conversation
… — DD-006 Phase 4
…— DD-006 Phase 5a
…ase 5b stubs — DD-006 Phase 5b
There was a problem hiding this comment.
Pull request overview
Implements DD-006’s per-request interpreter HTTP execution model, replacing the prior single-interpreter/channel bridge so requests can execute in parallel and hot-reload can swap server state atomically.
Changes:
- Introduces
SharedState+StoredHandler+Value::FlatFunctionto store Send-safe handler snapshots and rehydrate per request. - Reworks the async HTTP server to execute handlers via
spawn_blockingwith per-thread interpreter caching and adds a background hot-reload watcher. - Updates Intent Studio + intent checking to run tests via
execute_request()directly (no subprocess/server spin-up), plus docs and new tuning env vars.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/typechecker.rs | Updates Request struct comment to reference the new async server source of request fields. |
| src/stdlib/sqlite.rs | Adds regression test for DB handles across per-request interpreter boundaries via global registry. |
| src/stdlib/mod.rs | Removes http_bridge module export. |
| src/stdlib/http_server_async.rs | Implements per-request request handling, interpreter caching, hot-reload watcher, and rebuild helpers. |
| src/stdlib/http_server.rs | Extracts SharedState, switches route/middleware storage to StoredHandler, and adds type-context carryover. |
| src/stdlib/http_bridge.rs | Deletes the old channel-based interpreter bridge implementation. |
| src/main.rs | Extends runtime docs generation with new env vars and examples. |
| src/interpreter.rs | Adds FlatFunction/flattening, StoredHandler, per-request interpreter construction/reset, and per-request request execution APIs. |
| src/intent_studio_server.rs | Adds SharedState-based “direct execution” test path for Intent Studio. |
| src/intent.rs | Adds SharedState build + direct execute_request-backed test runner; refactors live test execution core. |
| docs/runtime.toml | Documents new per-request tuning env vars. |
| docs/RUNTIME_REFERENCE.md | Documents new per-request tuning env vars + examples. |
| docs/AI_AGENT_GUIDE.md | Adds per-request interpreter architecture and migration guidance. |
| benches/interpreter_construction.rs | Adds Criterion benchmarks for interpreter construction/snapshot overheads. |
| Cargo.toml | Adds Criterion dev-dependency and bench target. |
| Cargo.lock | Updates lockfile for Criterion and transitive deps. |
| CLAUDE.md | Mirrors per-request architecture + env var docs. |
| ARCHITECTURE.md | Updates architecture description to per-request model (but currently references a deleted file). |
| .github/copilot-instructions.md | Mirrors per-request architecture + env var docs. |
| fn flatten_value_visited(v: Value, visited: &mut std::collections::HashSet<String>) -> Value { | ||
| match v { | ||
| Value::Function { | ||
| name, | ||
| params, | ||
| body, | ||
| closure, | ||
| contract, | ||
| type_params, | ||
| } => { | ||
| if !visited.insert(name.clone()) { | ||
| // Cycle detected — emit a no-op FlatFunction stub | ||
| return Value::FlatFunction { | ||
| name, | ||
| params, | ||
| body: Block { statements: vec![] }, | ||
| contract: None, | ||
| type_params: vec![], | ||
| closure_snapshot: HashMap::new(), | ||
| mutable_names: std::collections::HashSet::new(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
flatten_value_visited() uses a visited: HashSet<String> keyed only by function name. This will incorrectly treat distinct functions with the same name (notably multiple Expression::Lambda values named "") as a cycle and replace later ones with the no-op stub, breaking captured closures/handlers. Cycle detection should be based on a unique function identity (e.g., add a generated function id, or track by pointer/allocated id), and ideally have a regression test that flattens two different lambdas and preserves both bodies/snapshots.
| /// Build a SharedState from source code by parsing and evaluating the .tnt file. | ||
| /// | ||
| /// Uses test_mode so listen() returns immediately without starting a server. | ||
| /// Copies type context from interpreter into the resulting SharedState. | ||
| fn build_shared_state_from_source( | ||
| source: &str, | ||
| ntnt_path: &Path, | ||
| ) -> Result<crate::stdlib::http_server::SharedState, IntentError> { | ||
| let mut interpreter = Interpreter::new(); | ||
| interpreter.set_main_source_file(&ntnt_path.to_string_lossy()); | ||
|
|
||
| // Set test_mode so listen() returns immediately | ||
| let shutdown_flag = Arc::new(AtomicBool::new(false)); | ||
| interpreter.set_test_mode(0, 0, shutdown_flag); | ||
|
|
||
| let lexer = Lexer::new(source); | ||
| let tokens: Vec<_> = lexer.collect(); | ||
| let mut parser = IntentParser::new(tokens); | ||
| let ast = parser.parse()?; | ||
|
|
||
| // Eval registers routes/middleware. listen() returns immediately in test_mode. | ||
| let _ = interpreter.eval(&ast); | ||
|
|
There was a problem hiding this comment.
build_shared_state_from_source() sets test_mode and comments that listen() returns immediately, but should_skip_server_call() only checks execution_mode (not test_mode). As written, evaluating a typical server file that calls listen() will still start the server and block indefinitely here. Consider introducing a dedicated capture mode (or extend should_skip_server_call / listen() handling) so listen() becomes a no-op while still allowing route/static/middleware/shutdown registration to populate SharedState.
| let mut interp = Interpreter::new(); | ||
| interp.set_main_source_file(source_file); | ||
|
|
||
| // Set test_mode so listen() captures SharedState instead of starting a server | ||
| let shutdown_flag = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); | ||
| interp.set_test_mode(0, 0, shutdown_flag); | ||
|
|
||
| let lexer = Lexer::new(&source); | ||
| let tokens: Vec<_> = lexer.collect(); | ||
| let mut parser = Parser::new(tokens); | ||
| let ast = parser | ||
| .parse() | ||
| .map_err(|e| IntentError::RuntimeError(format!("Parse error during hot-reload: {}", e)))?; | ||
|
|
||
| // Eval registers routes/middleware/etc. into server_state. | ||
| // listen() in test_mode returns immediately without starting a server. | ||
| let _ = interp.eval(&ast); |
There was a problem hiding this comment.
rebuild_shared_state() relies on set_test_mode() to make listen() return immediately, but the interpreter skips server calls based on execution_mode, not test_mode. If the source file invokes listen(), interp.eval(&ast) will still start the server and block, preventing hot-reload from ever completing. Please adjust the capture/reload path so listen() is skipped (without skipping route/static/middleware/shutdown registration) when rebuilding SharedState.
| let decoded = | ||
| urlencoding::decode(relative).unwrap_or_else(|_| relative.into()); | ||
| if !decoded.contains("..") { | ||
| let file_path = std::path::PathBuf::from(fs_path).join(relative); |
There was a problem hiding this comment.
Static file handling decodes the URL path into decoded but then joins/canonicalizes using the undecoded relative. This makes percent-encoded paths (spaces, unicode, etc.) fail to resolve to the on-disk file even when they should. Use the decoded path segment (after traversal checks) for building file_path/canonicalization, and keep the undecoded value only for logging if needed.
| let file_path = std::path::PathBuf::from(fs_path).join(relative); | |
| let file_path = | |
| std::path::PathBuf::from(fs_path).join(decoded.as_ref()); |
| ├── http_server_async.rs # Axum server, per-request execution, hot-reload watcher | ||
| ├── http_bridge.rs # Send-safe HTTP types (BridgeRequest/BridgeResponse) |
There was a problem hiding this comment.
ARCHITECTURE.md still lists stdlib/http_bridge.rs, but that file is removed in this PR and BridgeRequest/BridgeResponse now live in http_server_async.rs. Update the source tree and “Key files” sections to reflect the current file layout (or re-add a dedicated module if that’s the intent).
| ├── http_server_async.rs # Axum server, per-request execution, hot-reload watcher | |
| ├── http_bridge.rs # Send-safe HTTP types (BridgeRequest/BridgeResponse) | |
| ├── http_server_async.rs # Axum server, per-request execution, hot-reload watcher, BridgeRequest/BridgeResponse |
| - `http_server_async.rs` - Axum runner, `execute_request()`, static files, graceful shutdown, hot-reload watcher | ||
| - `http_bridge.rs` - Bridge types (`BridgeRequest`/`BridgeResponse`) for `Send`-safe HTTP representation |
There was a problem hiding this comment.
This “Key files” list references http_bridge.rs, but the PR deletes that module. Please update this list to point at the actual location of BridgeRequest/BridgeResponse (currently http_server_async.rs) so the architecture doc matches the codebase.
| - `http_server_async.rs` - Axum runner, `execute_request()`, static files, graceful shutdown, hot-reload watcher | |
| - `http_bridge.rs` - Bridge types (`BridgeRequest`/`BridgeResponse`) for `Send`-safe HTTP representation | |
| - `http_server_async.rs` - Axum runner, `execute_request()`, bridge types (`BridgeRequest`/`BridgeResponse`) for `Send`-safe HTTP representation, static files, graceful shutdown, hot-reload watcher |
| let mut interpreter = Interpreter::new(); | ||
| interpreter.set_main_source_file(&ntnt_path.to_string_lossy()); | ||
|
|
There was a problem hiding this comment.
build_shared_state_from_source() sets main_source_file but not current_file. Several server-related helpers (e.g., serve_static() path resolution and routes() directory resolution) use current_file to resolve relative paths; leaving it as None can make intent checks run against the wrong filesystem locations (cwd-dependent). Consider calling set_current_file() (ideally with a canonicalized ntnt_path) before eval() to match runtime behavior.
|
|
||
| let mut interp = Interpreter::new(); | ||
| interp.set_main_source_file(source_file); | ||
|
|
There was a problem hiding this comment.
rebuild_shared_state() sets main_source_file but does not set current_file. Since route/static path resolution relies on current_file for relative paths, hot-reload rebuilds may resolve routes() / serve_static() differently than the initial startup (cwd-dependent). Consider setting interp.set_current_file(source_file) (preferably canonicalized) before eval() so rebuild behavior matches normal execution.
| // Ensure current_file is set so relative routes()/serve_static() paths | |
| // resolve the same during hot-reload as during initial startup. | |
| let canonical_path = std::path::Path::new(source_file) | |
| .canonicalize() | |
| .unwrap_or_else(|_| std::path::PathBuf::from(source_file)); | |
| interp.set_current_file(canonical_path.to_string_lossy().to_string()); |
DD-006: Per-Request Interpreter Architecture
Replaces the single-threaded bridge with per-request interpreter instances. True parallel HTTP request handling — no more serialized queue.
Commits
5728788Phase 2:Value::FlatFunction,StoredHandler,flatten_value()065c9a0Phase 3:SharedStateextraction with type contextf83a609Phase 4:execute_request(),spawn_blocking05ed172Phase 6: docs, migration guide, new env vars81f8bf0Phase 5a: deletehttp_bridge.rs, remove sync servera1b088cPhase 5b: hot-reload atomic swap, test_mode shutdownf8448e6Phase 4 DoD: DB connection across per-request boundary testResult
http_bridge.rsdeletedlisten()entrypoint deletedSee
plans/006-ntnt-per-request-interpreter.mdfor full design doc.