Commit graph

10 commits

Author SHA1 Message Date
Bryan Helmkamp
af38d68942
Delete fabro-validate and fabro-acp; validate on Petri's check
Petri judges a workflow at admission, so Fabro's lint rules go:
`fabro-validate` (its 35 rules and the `LintRule` trait) is deleted, and
with it `fabro-acp` (only a rule and two legacy executor tests used it),
the model-resolution transform, the legacy `create`, `compile_create_run`
and `materialize_create_run` stages, and `fabro-graphviz`'s `condition`
and `fidelity` modules. `Diagnostic`, `RelatedDiagnostic` and `Severity`
move to `fabro_types::diagnostic`, the one shape every diagnostic takes.

Validation is now the same question the create handler asks. A new
server module, `petri_check`, builds Petri's check request from a
workflow bundle and the run's settings (every workflow of the bundle at
its bundle-relative path, the inputs, the run variables, the launch),
runs the check, and maps the diagnostics; Fabro adds one rule of its
own, `fabro.model.no_ready_provider`, refusing a model node when no
provider is ready. Admission, the validate and preflight endpoints and
the offline `fabro validate` all go through it:

- `validate_prepared_manifest` runs Fabro's structural pass (parse and
  transform, whose diagnostics stay) and then Petri's check, on the
  blocking pool from the handlers;
- the offline `validate_manifest` checks with no model client and with
  an unbound input as a warning (`CheckRequest.unbound_is_warning`), so
  a workflow validates before its inputs exist; a collected workflow
  before upload checks with unbound inputs as errors, as before;
- preflight resolves each LLM node's selector against the ready
  providers and the catalog for its probe, as the deleted transform did,
  and no longer probes a model Petri refused;
- the graph render endpoint needs only the structural pass;
- a run manifest now carries its `[run.goal] file`, which Petri reads
  from the bundle as it does for a version.

The transforms keep the authored model selector (`sonnet` stays
`sonnet`): Petri pins the catalog model in the admitted graph, not in
the graph Fabro displays or in the settings snapshot. Tests assert that,
and the CLI's validate, preflight and graph snapshots carry Petri's
diagnostics (`attractor.no_start`, `attractor.undeclared_node`,
`attractor.bad_on_failure`, ...) in place of the lint rules' text.

Known gaps, Petri's side: a `workflow.toml` whose `[run.environment]`
names an environment the server catalog defines but the file does not
is refused (`unsupported.workflow_toml.run.environment`), as admission
already refused it; an unbound input inside an included template
partial is a render error rather than the unbound-input warning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-18 11:32:04 -04:00
Bryan Helmkamp
b5885b15dc
Merge pull request #686 from fabro-sh/fix/space-separated-node-classes
Fix space-separated node class parsing
2026-07-29 22:31:48 -04:00
Bryan Helmkamp
01c4a4a23b
test(graphviz): table-drive the class separator cases
One row per separator rule, so a regression names the input that broke
instead of pointing at a combined fixture string. Also record why
`add_class` keeps insertion order: `fidelity` falls back to the first class
for the thread ID.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 22:17:37 -04:00
Bryan Helmkamp
f932a0763b
refactor(graphviz): simplify stylesheet comment stripping
Replace the hand-rolled byte scanner in strip_css_comments with a
str::find loop over "/*" and "*/".

Drop the quote and backslash tracking. The stylesheet language has no
string literals: parse_declarations ends a value at the first ';' or
'}' with no quote awareness, and values flow into AttrValue::String
verbatim, so a quoted model name is just an unknown model. Tracking
quotes here also created a failure mode the simple scan does not have.
An unpaired apostrophe, as in `model: don't`, disabled comment
stripping for the rest of the input and then blamed a well-formed
comment for the parse error.

Also drop the Cow and its copied_through watermark. They avoided one
allocation on a graph attribute of a few hundred bytes, parsed once per
workflow load, in a function whose caller already clones the attribute
and whose parser allocates a String per property and per value.

Extract excerpt() for the error snippets. The two existing call sites
sliced raw bytes at index 20, which panics when a multi-byte character
straddles the cutoff; model_stylesheet is arbitrary user text, so that
was reachable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 22:16:36 -04:00
Bryan Helmkamp
85586151e5
refactor(graphviz): parse node classes in one place
Node classes were built in two places. The parser split the `class`
attribute on commas and whitespace, but the import transform re-split the
raw attribute on commas only. A space-separated class on an import
placeholder became a single class name, so stylesheet rules did not match.
That included the `class="fast shared"` example in the imports docs.

- add `Node::add_class`, replacing the duplicate append helpers in
  `SemanticState` and `ImportTransform`
- read `node.classes` in `placeholder_config` instead of re-parsing the raw
  attribute, so class splitting happens in exactly one place
- name the separator rule `split_class_attr`, splitting on commas and then
  whitespace so empty entries need no trimming
- drop the unused `Node::class` accessor that invited the re-parse
- keep the comma-compatibility note in the DOT attribute reference only

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 22:15:30 -04:00
Bryan Helmkamp
af9aa53088
fix(graphviz): parse whitespace-separated node classes 2026-07-29 21:51:57 -04:00
Bryan Helmkamp
692301d867
feat(workflow): support comments in model stylesheets 2026-07-29 17:25:04 -04:00
Bryan Helmkamp
04c931e9d3
Simplify edge-only node validation 2026-07-28 14:14:32 -04:00
Bryan Helmkamp
59b1c2e59f
Reject nodes referenced by an edge but never declared
The DOT parser created a node for every edge endpoint, and nothing
recorded whether a node came from a declaration or was synthesized from
an edge. The edge_target_exists rule only checked whether the node id
was present in the graph, which was always true by then, so a misspelled
endpoint became an attribute-free node that defaulted to shape=box — an
LLM stage. Validation emitted a prompt_on_llm_nodes warning and exited 0.

Node now carries `implicit`, set only when the parser synthesizes the
node from an edge endpoint. A declaration anywhere in the workflow
clears it, so order does not matter and subgraph declarations count.
Node::new leaves it false, so programmatic construction and graphs
deserialized from older checkpoints read as declared.

edge_target_exists treats an endpoint as valid only when it exists and
is declared, reporting each undeclared node once. The near-identical
missing-source and missing-target branches collapse into one path. The
import transform copies the flag onto spliced nodes so an edge-only node
inside an imported fragment is caught too.

parse_and_validate_human_gate had two edge-only nodes and now declares
them; it was an instance of the bug rather than a casualty of the fix.
No shipped workflow, docs example, or CLI fixture relied on the old
behavior.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 13:53:54 -04:00
Scott Werner
47bc772f7b refactor: organize crates into three layers 2026-07-23 17:59:34 -04:00