Repository navigation
Separate Rust crate responsibilities and the browser boundary - #64
Merged
Merged
Conversation
Move the value vocabulary that the language and the check engine share into a new crate, crates/whirl-types: Value, ValueType, and quote from check/value.rs; Number from check/number.rs; StaticType, FilterKind, PredicateKind, and the keyword tables from check/types.rs; and the bytes-literal helpers is_bytes_literal_shape and bytes_literal from check/predicate.rs, now in their own bytes.rs module with their test. whirl depends on whirl-types. lang, check, and run import these items from whirl_types directly, so check.rs no longer re-exports them. The base64, chrono, serde_json, and proptest versions move to [workspace.dependencies] unchanged, because two crates now need them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The parser no longer checks literal regexes, `json:` queries, `xpath:` expressions, date formats, or charset labels. It keeps each literal and its span in the AST. `check::validate_literals` walks the parsed file and runs the same validators that build filters at run time, and reports the first invalid literal in source order as the parse diagnostic the parser produced before. The CLI calls it after every `parse_file` of an input or setup flow. The parser's tests for these errors move to `check/literals`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Move `crates/whirl/src/lang/` into the new `crates/whirl-lang` crate: the AST, parser, formatter, lint rules, EXTRACT schema rules, and command-line option resolver. It depends only on `whirl-types`, `serde_json`, and `thiserror`, with the pest grammar tests as dev-dependencies, so it builds with no native code. `ast` stays a public module; the other modules are private and `lib.rs` re-exports what `crates/whirl` uses. `ModelFacts` stays an enum: `lint_act` takes a closure, so the lint has no model dependency. `thiserror` moves to the workspace dependencies. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Move the check engine out of crates/whirl into crates/whirl-check: check.rs becomes lib.rs, and filter, json, literals, pattern, predicate, and xpath move with it. The crate depends on whirl-lang, whirl-types, and whirl-xpath, and owns regress, serde_json_path, encoding_rs, and the URL and HTML escaping crates. It re-exports the types the runner resolves into plus validate_literals; the modules stay private. crates/whirl now depends on whirl-check instead of whirl-xpath, and the runner and cli import from whirl_check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Move report.rs and report/ (model, console, json, junit, html, aggregate, metadata, the test fixture, and html.css) into crates/whirl-report. The fixture no longer builds a Masker; it writes its strings with the mask text already applied, so whirl-report depends on no other Whirl crate. The runner's private `impl EntryReport` in run/flow.rs became part of `setup_entry`, because an inherent impl cannot live outside the type's crate. anyhow, serde, and tempfile moved to workspace dependencies with their existing version specs; quick-junit left crates/whirl. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`StepCommand::Snapshot` carried a `#[serde(skip)]` `SnapshotReport` that never went to the shim; it only rode along so the flow could put it in the step report. The flow now builds the command and the report together in `build_snapshot` and keeps the report in a new `PreparedStep::Snapshot` variant. The wire frame and the report JSON are unchanged. This removes the shim module's only use of `whirl-report`, so the shim can move to its own crate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Move the browser shim boundary out of `crates/whirl/src/run/shim*` into `crates/whirl-shim`: the `ShimClient` process client and wire protocol (`lib.rs`), the AST-to-wire conversion (`wire.rs`), and the client and timeout unit tests. A new `bundle` module holds the installed bundle's layout (`data_dir`, `BUNDLE_*`, `SHIM_JS_ENV`, `WHIRL_VERSION`), which is what `install` needs, so `install` and `doctor` no longer reach into `run`. `whirl-shim` depends only on `whirl-lang` among the Whirl crates. `tokio`, `tracing`, and `url` move to `[workspace.dependencies]` with the same specs; `dirs` moves from `whirl` to `whirl-shim`. The unit tests keep using `crates/whirl/tests/fixtures/fake_shim.js`, which the acceptance tests share. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`Instruction::try_new` took the runner's `VarStore`. It now takes any `Variables` store: `resolve`, `mask`, and `secrets`, which are the three things it used, with the store's own error type. `VarStore` implements the trait in `run/vars.rs`, so the runner's call sites and their `VarError` handling are unchanged. The act module's unit tests use a small `TestVars` double instead of `VarStore`. This prepares the move of `run/act` into its own crate below the runner. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Move `run/act` out of the `whirl` crate into `crates/whirl-ai`: the planner and its Jev path, the model catalog and client, the prompts for ACT, GOAL, ai: targets, EXTRACT, and JUDGE, the AI snapshot parser, and the decision that turns a model answer into a Whirl action. The crate depends on `whirl-lang` and `whirl-shim` only, and is the one crate with `lithos-llm`. It re-exports `ModelError` and `ModelErrorKind` so the runner classifies a failed call without naming `lithos-llm`. Items the runner and the CLI use are `pub`; the rest stays crate-private. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Move `run.rs` and `run/` (artifacts, cache, extract, flow and its step modules, runner, vars) out of `crates/whirl` into the new `whirl-run` crate. The binary keeps `cli`, `doctor`, `install`, and `telemetry`. `whirl-run` exposes `run_files`, `RunSettings`, `RunnerError`, `FlowFlags`, `setup_path_for`, `dedup_flows`, `ArtifactsError`, `parse_var_flag`, `parse_variables_file`, `VarsFileError`, and a `pub mod cache` for the `whirl check` cache diagnostics. Every other module stays private. `CacheMode` loses its `clap::ValueEnum` derive so the runner does not depend on the argument parser; `cli.rs` parses `--cache` into a local `CacheModeArg` with the same documented values and converts it. The pinned `run --help` output is unchanged. `crates/whirl` drops `base64`, `chrono`, `tempfile`, `tracing`, `url`, and `whirl-types` from its normal dependencies (the acceptance tests keep `base64`, `chrono`, `tempfile` as dev-dependencies); `sha2` moves to `[workspace.dependencies]`. The telemetry doc comment now records that the `whirl` target prefix also matches the `whirl_*` crates. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Record the nine-crate workspace as a draft ADR: what each crate owns, the layer order with its down-only dependency rule, why whirl-types exists and must stay narrow, where literal validation happens, and which crate owns the shim protocol, the lithos-llm dependency, and the acceptance tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
List each crate and its role in CONTRIBUTING, say that unit tests live in the crate that owns the code, name the option resolver by its new path in the recording ADR, and fix the telemetry test's comment and filter to use the tracing targets the split crates emit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The telemetry test kept only lines from `whirl_run` and `whirl_shim` targets after the split. The binary crate still traces under `whirl::`, so its lines were no longer checked for paths and URLs. The filter now keeps all three. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
whirl-shim gains BrowserOptions, Recording, VideoOutput, and Features, and StartFlowParams::from_options maps them to the startFlow params. The browser-to-string, dialog and reduced-motion text, the base host added to allow-hosts (url_host), the Chromium-only screencast fps rule, and the video temp dir now live in whirl-shim's private context module with tests. ResolvedOptions in whirl-run keeps the core fields and holds a BrowserOptions for the rest; the runner only decides the artifact paths and the file's features. The wire frame is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`wire` is now a private module. The resolved wire values travel as typed newtypes: Locator (with `resolve` and `element_ref`), PageExpect, AssertSpec (`element_state`, `tab_closed`), ReadSubject (`resolve`, `url`, `element_value`, `element_attr`), MockPattern::parse, and a MockResponse enum. StepCommand fields take these instead of serde_json::Value; Scroll holds the AST ScrollMotion and Snapshot the AST MaxDiff, which step_frame encodes. GeneratedLocator decodes the shim's segments into an ast::Locator when the client parses the result (a segment a file cannot write is reported as Unstable). `independent_http_response` is `response_name_for_http` at the root. whirl-run and whirl-ai no longer import `wire` or build protocol JSON. Wire frames are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FlowFlags keeps trace, video, video_fps, update_snapshots, and cache,
and holds a new WebFlags { har, headed, load_state, save_state } as
`web`. The CLI fills both; the runner reads the web flags from
`flags.web` when it normalizes the state paths and builds the shim's
browser options and recording. Doc comments moved with their fields.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 5, 2026
brynary
added this pull request to stack #67
October 5, 2026 18:18
brynary
marked this pull request as ready for review
October 5, 2026 18:18
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
1 participant
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.
Split the Rust implementation by responsibility and move browser-context construction and wire details behind whirl-shim. Keep the crate-layout ADR at status: draft for explicit architectural review.
Recovers the intended changes from #50, #51, which were merged into the old development stack but did not reach
main. Preserves the newerlithos-llmpin from #54.Stage 8 of 10. Based on #63. Full recovery inventory and merge order: #57.
Merge the recovery PRs in order with merge commits. Once the preceding PR lands, retarget this PR to
main.Draft: the crate-layout ADR remains
status: draft; this PR does not accept it. Literal validation moves after parsing, so a syntax error can take precedence over an earlier invalid literal when a file has multiple errors.Validation:
CARGO_NET_OFFLINE=true mise run checkpassed on this branch. This runs formatting, Clippy, local Rust/browser tests, shim checks, eval tooling, and workflow checks. Paid model evals, the external BrowserSim round trip, and live Kernel checks were not rerun.