Commit graph

9 commits

Author SHA1 Message Date
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