Repository navigation
Unified error struct - #190
Closed
vinniefalco wants to merge 23 commits into
Closed
vinniefalco wants to merge 23 commits into
vinniefalco wants to merge 23 commits into
Conversation
This change gives each prompt parse rule one implementation and makes the doc comments state what the parser does. The alias grammar moves to `runvil_types::names`, the H1 and the sections share one compile path, and list detection uses one pass. Accepted prompts and error messages do not change. - `runvil_types::names::is_alias` is now the only alias grammar, and the `ContractKey` check, `Alias::parse`, and `tool_schema_new` call it but keep their own messages. The accept and reject cases move to `is_alias_follows_the_alias_grammar`, so the grammar cannot drift between callers while each caller's tests pass. - `SectionWalker` holds the values that the parse passes down, and its `compile_blocks` compiles both the H1 blocks and the section blocks, so `split_h1` is gone. - `split_section_blocks` gives each `RawBlock::Lua` a `LuaRole` once, and `lua_block_location` takes that role in place of four position arguments. - `#[non_exhaustive]` drops from `Prompt`, `ParseError`, `Section`, `Frontmatter`, `FileDecl`, `ArgDecl`, `ArgsDecl`, and `RoleRequirements`, whose fields are all private or crate-private, so it restricted nothing. `public-api.txt` changes to match. - `is_list` on `Section` replaces the test-only `is_list_only` as a public method, and the worker check in `tasks-spawn.rs` calls it. - `list::bullet_items` replaces `is_all_list_markers` and `parse_bullet_items` with one pass that returns `None` for prose and reports an empty item only after the full scan. `an_empty_marker_before_a_prose_line_stays_prose` pins that order, and the two unreachable errors and their tests are gone. - `level as u8` replaces `level_num`, so the heading level now comes from the `HeadingLevel` discriminant. - `max_tool_iterations` now validates in one chain, and `max_tool_iterations_reports_the_guides_messages` pins both messages, also for a value above the `u32` range. - No new test covers the Lua block locations or the shared compile path. They rely on the existing parse tests. - The Language Guide does not change in this commit.
The desktop app spawned its sibling Gateway without checking whether that file was the compile-only stub the fetch script writes, so Windows answered a text file with an image-format error that named no file. Boot now reads the stub's prefix and fails with the path and the command that stages the real Gateway. - sidecar_is_stub reads the .pinned marker with a substring test and keeps �uild.rs free of a JSON dependency; a malformed marker reads as not-a-stub, so an unreadable marker cannot silence the check. - cargo:rerun-if-changed watches the marker, which only the fetch script writes, so re-staging a release over a stub re-runs the build script and clears the warning. - The stub test is a byte-prefix compare against the exact text --stub writes, so an empty or missing file reads as not-a-stub and falls through to the spawn.
The alias is named `vibelight`, and cargo resolves an alias before an installed subcommand of the same name and warns that the alias shadows it. That warning says cargo plans to make shadowing a hard error. `cargo install vibelight` now installs a `cargo-vibelight` shim, so the alias would shadow it on every machine that has it. The alias becomes `outline`, which no subcommand name can collide with. It still builds the sibling checkout in release mode, so `cargo outline` is the moving copy and `cargo vibelight` is the installed one.
The new `error` module gives one record that can describe a failure from any Engine crate. `Error` holds an `ErrorCode`, a `message` for logs, a `prose` text for the model, an optional `Location`, and a `facts` map from `FactKey` to `serde_json::Value`. - `ErrorCode` has 65 unit variants with explicit `#[repr(u16)]` numbers grouped in family hundreds, and `number` returns the number. - The four types have public fields and derive no serde traits. - `Location::new` returns `None` when the end comes before the start. `Location::line` covers one whole line, and the range is empty on line `u32::MAX`. - No code outside the module uses it yet. - The tests check that each code is in `ALL` once and that each number is in its `family` hundred. `ALL` is a hand-written list.
The universal error gains the codes and the fact key that harness failures need, and a default model-facing text for each code. `prose` returns one sentence per code from the new `error-prose.rs`, and the sentences for failures in the prompt itself speak to a model that fixes the prompt. - `RecorderFailed`, `PluginLoadFailed`, and `NoOutput` open the 8xx family, and the new `Run` fact holds a recorder run id. - The docs of the `Path` fact and of `InvalidValue` now cover file paths and missing values. - `prose` is one `match` with one arm per code, under an `#[expect]` of `clippy::too_many_lines`. - A new test checks that every default text is a whole sentence, and `ALL` now lists 68 codes.
Harness and Plugin crates cannot depend on `runvil-types`, so the universal error now reaches them through the two public crates. The `runvil` facade gains an `error` module, and `runvil-plugin` re-exports the same four types at its root. - The facade module holds `Error`, `ErrorCode`, `FactKey`, and `Location`, one `pub use` line each. - `public-api.txt` now lists the four types, every `ErrorCode` variant with its number, and every `FactKey` variant. - No code uses the re-exports yet.
`store_error` gives the one mapping from a `VfsError` to the universal error, for the places where a store failure leaves the VFS. `VfsError` stays the VFS's own error, so the std-only VFS crate does not change. - The code is named after the variant. A variant added to `VfsError` later becomes `Backend` until it is named. - The facts reuse `store_error_fields`, so they hold the same `path`, `valid_up_to`, `rule`, `anchor`, and `count` values a Lua error table holds. - The message is the failure's display text, the prose is the code's default, and the error has no location. - The `runvil` facade re-exports the function as `runvil::vfs::store_error`. No code calls it yet. - New tests cover all 21 variants and the facts each one keeps.
Every id type and both catalogs now reject an id with an `InvalidValue` error instead of the bespoke `IdError`. The message text stays the same, and the rejected text becomes the `Name` fact. - `invalid_id` in `names.rs` builds the error with the code's default prose and no location. It is crate-private. - `FromStr` for `GlobalName`, `ChainId`, `TaskId`, `PackId`, and `ToolId` now has `type Err = Error`. The `parse` functions, `ModelId::new`, `ModelId::gateway`, `ToolCatalog::new`, and `ModelCatalog::new` return the same `Error`. - `runvil::ids` and `runvil-plugin` no longer re-export `IdError`, and the run-log fidelity test names `runvil::error::Error`. - New tests pin the code, the `Name` fact, the message, and the prose for every id type and for a repeated id in each catalog.
`Role::from_str` now rejects unknown text with an `InvalidValue` error instead of the bespoke `ParseRoleError`. The error keeps the old text, names the input, and lists every role. - The message and the prose are both the old `Display` text, because the list of roles tells the reader what to use instead. - The rejected text is the `Name` fact, which replaces the `input` accessor. - `runvil::model` and `client` no longer re-export `ParseRoleError`. - The role test now pins the code, the `Name` fact, the message, the prose, and the missing location.
`ErrorKind::from_str` now rejects text that is no error-kind tag with an `InvalidValue` error instead of the bespoke `ParseErrorKindError`. The error keeps the old text, names the input, and lists every tag. - The message and the prose are both the old text, because the list of tags tells the reader what to use instead. - The rejected text is the `Name` fact, which replaces the test-only `input` accessor. - `runvil-lua` no longer re-exports `ParseErrorKindError`. The shim's unknown-kind refusal still renders the same text, because it formats the error's `Display`. - The kind test now pins the code, the `Name` fact, and the prose.
`Temperature::new` and `CompletionOptions::with_temperature` now reject a temperature with an `InvalidValue` error instead of the bespoke `TemperatureError`. The message text stays the same. - The error names the `temperature` setting in its `Name` fact, and its `Reason` fact is `not_finite` or `out_of_range` in place of the two variants. - The prose gives the valid range. It does not repeat the message. - `models.use` in the Lua crate still picks its finite-number refusal for a value that is not finite, now through `is_finite` instead of a match on the variant. - `runvil::model` and `model` no longer re-export `TemperatureError`. - The temperature test now pins the code, the `Name` fact, both reasons, the message, and the prose.
`tool_schema_new` now refuses a tool schema with a `ToolSchemaInvalid` error instead of the bespoke `ToolSchemaError`. The message text stays the same. - The error names the tool in its `Name` fact. Its `Reason` fact is `invalid_name` or `non_object_schema` in place of the two variants. - The prose is the code's default, which speaks to whoever fixes the tool. - The engine's `BindSchema` variant still boxes the refusal as its cause, now as the universal `Error`, and its test downcasts to that type and checks the code. - `client` no longer re-exports `ToolSchemaError`, and the engine's `model` module doc no longer names it. - The schema test now pins the code, the `Name` fact, both reasons, the prose, and the tool name in the message.
`SecretString::new` now refuses an empty credential with an `InvalidValue` error instead of the bespoke `SecretError`. The message stays `secret must not be empty`. - The `Reason` fact is `empty_secret`. The error holds no part of the secret. - The prose tells the reader to supply a non-empty secret. - `From<SecretError> for GatewayConfigError` is removed. No production code used it, because the environment loader folds the refusal into `None` with `Result::ok`. - The test that pinned the `SecretError` cause of a config error is removed with the conversion, and the source check on the empty key is removed from the leak test. - The empty-credential test now pins the code, the `Reason` fact, the message, and the prose.
The gateway client's setup errors are now `InvalidValue` errors instead of the bespoke `GatewayConfigError`. The environment and endpoint messages keep their text, and every message still omits any key or URL. - `missing_env` and `invalid_env` name the variable in the `Name` fact. Their `Reason` facts are `missing_env` and `invalid_env`, and an endpoint refusal has `invalid_config`. - The prose of a setup error is its message, because the message names the variable or the rule to fix. - An unparsable URL's message now ends with the `url::ParseError` text instead of keeping it as the source. That text names the rule, never the input. - `GatewayChat::from_env`, `GatewayEndpoint::new`, and its `TryFrom` return the universal `Error`. The crate no longer exports `GatewayConfigError`. - The tests check the reason, the variable, and the prose. The source-chain checks are replaced by a check on the parse text in the message, and the no-leak loop stays.
`HostServices::provide` now refuses a provider with an `InvalidValue` error instead of the bespoke `ServiceError`. The refusal messages keep their text. - The error names the id literal in its `Name` fact. Its `Reason` fact is `invalid_id` or `duplicate_id` in place of the two variants. - The prose is the message, which names the id and the rule. - The map still stays as it was on either refusal, and `runvil-plugin` no longer exports `ServiceError`. - The service tests now check the code, the reason, the literal, and the prose through one helper, in place of the matches on the variants.
`PackRegistry::insert` and every Plugin's `install` now refuse a Pack with an `InvalidValue` error instead of the bespoke `InstallError`. The refusal messages keep their text. - The error names the FQN in its `Name` fact. Its `Reason` fact is `invalid_fqn`, `duplicate_pack`, `reserved_pack_id`, or `reserved_name` in place of the four variants. - The prose is the message, which names the Pack and the rule. - The `install` functions of `plugin-std`, `plugin-mcp`, `plugin-user-input`, and `plugin-web` now return `Result<(), Error>`, and their docs name the `duplicate_pack` refusal. - `runvil-plugin` and the `harness` facade no longer export `InstallError`, and `runvil-plugin` drops its `thiserror` dependency. - The registry tests check the code, the reason, the FQN, and the prose through one helper, and the refusal-message test builds each refusal through `insert`. The two Plugin tests check the `duplicate_pack` refusal the same way.
`Error::with_cause` appends a cause, and each cause under it, to the message after `: `, and leaves the prose as it was. A conversion that kept a typed source now keeps its text in the message, where a log reads it, while the model reads only the prose. - A cause whose text is empty, or already in the message, is skipped. - The facade listing in `public-api.txt` gains the method. - Two new tests pin the appended chain, the unchanged prose, and the skip.
A tool call now fails with the universal `Error` instead of the bespoke `ToolError`, and its four kinds become the codes `ToolInvalidArguments`, `ToolBackend`, `ToolTransport`, and `ToolOther`. `tool_error` builds a failure whose message and prose are both the tool's model-safe text, and `with_cause` puts a cause in the message only. - The `Pack` trait, `PackInfo` construct, every Plugin, the Harness performers, the Engine's effect answer, and the Lua dispatch now carry `Error`. `runvil`, `runvil-plugin`, and `runvil-types` export `tool_error` in place of the two types. - The Lua crate's `Error::tool` and the Engine's failure record take the tool's `prose`, so a cause stays out of the text the model reads and out of the stored record. - `tool_failure_tag` keeps the tags a run log stored before: `invalid_arguments`, `backend`, `transport`, and `other` for any other code. - Tests that pinned the typed source or the kind tags are removed. Tests that hid a source now check the cause text in the message and its absence from the prose. A new test pins every tool code's record tag, and one new types test pins `tool_error`. - The two store-answer tests move unchanged to `effect-tests-store.rs`, which keeps `effect-tests.rs` under the 500-line limit.
A model round and a catalog fetch now fail with the universal `Error` instead of the bespoke `CompletionError`, and the 13 `CompletionErrorKind` kinds become model codes of the same names. `completion_error`, `empty_reply`, and `context_overflow` build the failures, and `phrase` gives each model code its fixed phrase. - `ErrorCode` gains `tag`, which gives each code its name in snake_case. The store and model codes keep the tags a run log already holds, and the chat `FailureRecord` uses `code.tag()`. - The prompt token count and the window move to the `Count` and `Limit` facts, and the finish reason moves to the `FinishReason` fact. - A cause goes through `with_cause`, and the Gateway client's `with_detail` appends the escaped provider body. Both go in the message only, so the prose stays free of provider text. - The Lua `ErrorValue` trait gains `table_message`, and the Engine returns the prose of a model failure through it. The Lua table's `message` stays free of provider text and causes. - The chat `FailureRecord` and the `ChatOutcome::Empty` text take the prose. The display text of the Engine's `Error::Completion` now includes the cause and the provider body. - `runvil` exports the four functions in place of the two types, and `harness-gateway-client` drops its re-exports. - Tests that pinned a typed source or the `detail()` accessor are removed. Tests that hid a source or a body now check its text in the message and its absence from the prose. New tests pin every tag, every phrase, and each builder.
A Gateway web search now fails with the universal `Error` instead of the bespoke `GatewaySearchError`. A transport failure is `ToolTransport` and a backend failure is `ToolBackend`, and `tool_error` builds each one with the existing text as its prose. - `search_error` gives the `SearchError` the prose as its text and the universal `Error` as its source, so the text of the search failure does not change. - A send, read, decode, or parse cause goes in the message through `with_cause` and stays out of the prose. - `harness-gateway-client` no longer exports `GatewaySearchError` and `GatewaySearchErrorKind`, and it drops its `thiserror` dependency. - The response tests check the code, the prose, and the cause in the message through `assert_caused`, and the provider tests check the universal cause through `assert_gateway_cause`. The source downcasts to `reqwest::Error` and `FromUtf8Error` are removed.
A recorder now fails with the universal `Error` under `RecorderFailed` instead of the bespoke `RecorderError`, and the in-memory recorder drops its private `MemoryError`. `recorder_error` builds the failure from an error or a message, so each `map_err` call site keeps its shape. - The message is `the run recorder failed` followed by the cause chain, and the prose is the code's default. `display_chain` renders the same text as before. - The memory recorder's refusals add the `Reason` fact `unknown_run` or `run_ended` and the run id as the `Run` fact. - `HarnessError::Recorder` and `PrepareError::Recorder` hold the error in a `Box`, which keeps both `Err` types under the `result_large_err` limit. - `harness-runner` and the `harness` facade export `recorder_error` in place of `RecorderError`. The workshop's `TursoRecorder` and the tee test build their failures through it. - The source downcast tests are removed. The runner tests check the code, the message, the prose, and the memory recorder's facts through `assert_refused`, and the run-log tests compare the message after the prefix with the `LogError` text.
When a cause is itself a universal `Error`, its message starts with its prose, and the outer message often already ends with that same text. `with_cause` now finds the longest head of the cause's text, cut at a `: `, that the message ends with, and appends only what follows it. - A cause with no such head still goes after `: ` whole, and a cause the message already holds is still skipped. - The search is from the right through `rmatch_indices`, so the longest repeated head wins, and an empty head never matches. - A new test pins a universal cause that adds only its own cause, and an unrelated cause with `: ` in its text that goes in whole.
A Plugin that does not open or load now fails with the universal `Error` under `PluginLoadFailed` instead of the public `PluginFailure` and the private `LoadError`. `load_error` builds each failure with the old message text and the `Reason` fact `open`, `missing_entry`, `refused`, `misuse`, `status`, `undescribed`, or `duplicate`. - The prose is the code's default, because the message holds operating system text and library paths. - `with_path` puts a library's path in the `Path` fact, and a repeat puts the repeated Plugin name in the `Name` fact. A failure of a linked Plugin has no `Path` fact. - `insert_plugin` returns `Result<Arc<Plugin>, Error>`, and `PluginReport::failures` returns `&[Error]`. `harness-plugin-loader` and the `harness` facade no longer export `PluginFailure`, and the loader drops its `thiserror` dependency. - The tests read `message` in place of `reason()`. New assertions check the code, the default prose, the `Reason` fact of each failure kind through the `reason` helper, the absent `Path` of a linked Plugin, and the `Path` and `Name` facts of a library failure.
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
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.