From 0fda3f88db4feeaa19c1f476022d111aab5f9f13 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 6 Oct 2026 05:01:03 +0000 Subject: [PATCH 1/4] The README shows the Pydantic Graph A/B, and stops keeping two rules tables (#16, #15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## #16 — the lineage was real and unpublished `examples/greeting.py` IS Pydantic Graph's smallest complete builder program, morphed — its own docstring says so, and `examples/ladder/their_hello.py` keeps their original shape with no workbench in the file. But `grep -iE 'visualize_graph|their_hello|variation of' README.md` returned NOTHING, so the comparison a reader actually wants — *here is theirs, here is the same thing declared* — existed only in a docstring. A two-row table at the top of "What it looks like" now puts theirs beside ours, says theirs is fine, and names the two things it cannot do. `their_hello.py` and `contestable.py` are in the More index; both were reachable only from `docs/ladder.md` or one inline mention. A test requires the lineage by name, because a paragraph nothing holds in place is the first thing a prune deletes — and the prune is the other half of this commit. ## #15 — restructure, not a word target ⛔ The 750-900-word target in the handoff counted two GENERATED tables as prose. Both are now inside `
`; the drift test matches on the markers, so it does not care, and nothing is deleted. **Measured, and it is the honest number: 3,407 -> 2,902 words visible before expanding anything (-15%), while the page carries MORE content.** Short of the ~1,800 I estimated, because that estimate assumed cutting all three honesty sections and one of the two greeting walkthroughs. I did neither — see below. ### The one real deletion, and it had already drifted A hand-written `| check | catches |` table sat ~190 lines below the generated one and listed **10 of 12**: `check_recursion` and `check_transform_edges` had landed and nobody re-synced the copy. Two tables of one set of facts, one of them generated — `.claude/rules/spec-as-code.md`, source or derived, and mixing them is the whole failure mode. Deleted rather than re-synced, because re-syncing leaves the next rule free to do it again. The failure each check exists for lives in that check's docstring, which the README already says. A test now refuses any table row naming a `check_*` outside the generated block. Mutation-tested. ### Two of three honesty sections merged `What the checks guarantee, and what they do not` + `Current limitations` -> `## What this does not do`, with `### Structural checks are not a correctness proof` and `### What cannot be declared`. `Working with a coding agent` was stranded inside that merge and is promoted to its own `##` — it is not a limitation. ⚠️ `### ⛔ When not to use this` STAYS AT THE TOP and is deliberately not merged down. A reader deciding whether to keep reading needs it in the first screen; moving it would cost more than the duplication it removes. ⚠️ The greeting example is still told three ways — the case table, the quickstart's terminal excerpt, and the five-stage walkthrough. The remaining lever is collapsing the walkthrough into `
`, which would take visible words to roughly 2,000. NOT DONE: 43c1a85 deliberately rebuilt this README *around* that walkthrough, and undoing a decision that recent is a judgement call, not a prune. ### Every removed line, read before committing 21 lines: 13 are the stale duplicate table, 6 are the agent section MOVED (verified still present), 2 are headings retitled with their content kept. Nothing lost. 278 -> 280 tests. Both generators green. Co-Authored-By: Claude Opus 5 --- README.md | 69 ++++++++++++++++++++++++++--------------- tests/test_reference.py | 38 +++++++++++++++++++++++ 2 files changed, 82 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 082909f..bb47c07 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,21 @@ Built on [Pydantic Graph](https://ai.pydantic.dev/graph/) and ## What it looks like +This is **their** example, declared our way. Pydantic Graph's smallest complete builder program +is two steps where the second formats the first's output +([`visualize_graph.py`](https://pydantic.dev/docs/ai/graph/builder/)); the same shape runs here, +so the difference you are looking at is the declaration layer and nothing else: + +| | step 1 | step 2 | +|---|---|---| +| **theirs** — [`their_hello.py`](examples/ladder/their_hello.py), no workbench in the file | `step_a` → `10` | `step_b` → `f'Result: {ctx.inputs}'` | +| **ours** — [`greeting.py`](examples/greeting.py) | `normalize` → a clean name | `compose` → `f'Hello, {name}!'` | + +Theirs is fine, and that is the point of keeping it: one graph with one implementation per step +runs perfectly well like that. What it cannot do is check or draw itself before the steps are +written, or answer *"and what if `normalize` were written differently?"* — which is the only +thing the rest of this page is about. + One workflow — normalize a name, then compose a greeting from it: | step | input | output | @@ -184,6 +199,9 @@ missed. ## What `coherence_check()` enforces +
+All 12 rules — generated from each check's own docstring + **12 rules.** `coherence_check()` returns one finding per violation and an empty list for a clean design; `render()` refuses on any finding that blocks. @@ -210,6 +228,8 @@ missed. | `check_recursion` | A design does not implement one of its own nodes with itself. | +
+ Every one of these exists because it caught something that otherwise **ran and returned a plausible answer**. Each check's docstring in [`checks.py`](workflow_workbench/checks.py) carries the measured case that produced it. @@ -251,6 +271,9 @@ slots are one transposition away from a graph that is wrong and runs. #### The data language +
+Every word you declare a design with — generated from the types' own docstrings + A design is **data** — tuples of these, in a class body. Nothing executes, which is what lets `coherence_check()` and `diagram()` read it before a single step is written. @@ -296,6 +319,8 @@ A design is **data** — tuples of these, in a class body. Nothing executes, whi The two unions are annotations, not classes you instantiate — calling either one raises `TypeError`. They exist so a signature can say *any declared box*, or *anything a strategy must bind*, and have it type-check. +
+ ### 2. Check it and draw it, before implementing anything ```python @@ -377,7 +402,18 @@ battle = eval_battle(spec, trim_only, normalize_spaces, dataset()) # the compar `eval_battle` takes one `spec` and two strategies, so both arms render from the same nodes, edges and types. There is nowhere to put a second design. -## What the checks guarantee, and what they do not +## Working with a coding agent + +The specification is the reviewable artifact. Review the diagram and the contracts, and the +agent's job narrows to step bodies satisfying a declared input and output type for a named role, +with `coherence_check()` as the acceptance test. + +A proposed change to the workflow itself is then a diff to `nodes` and `edges` — one small place, +reviewed on its own, not a behaviour change buried in a function body. + +## What this does not do + +### Structural checks are not a correctness proof The specification supplies the structure and the data contracts; a strategy supplies implementations; `render()` constructs the graph from those declarations. There is no second, @@ -385,35 +421,16 @@ separately maintained wiring definition to drift from them — `edges` is the on wired, with no hook and no override, so a strategy can change what a node *does* and cannot change what the workflow *is*. -What the checks in [`checks.py`](workflow_workbench/checks.py) detect: - -| check | catches | -|---|---| -| `check_names` | two nodes with one name — they become one graph node id | -| `check_reachable` | a node unreachable from `START`, or unable to reach `END` | -| `check_variables` | an edge carrying a value its source does not produce or its target does not take | -| `check_step_arity` | two unconditional arrivals into one step: it runs twice and one result is dropped | -| `check_bindings` | a strategy binding too few nodes, or one the design does not declare | -| `check_implementations` | a binding that is not callable, or does not take exactly one `ctx` | -| `check_variable_types` | a return annotation that does not satisfy the role's declared output | -| `check_decisions` | a branch condition anywhere but on an edge leaving a decision | -| `check_fan_out_rejoins` | fan-out items reaching `END` without passing a join | -| `check_subgraphs` | a child design that does not fit the node it fills, or is bound inside itself | +Every rule is in the generated table above, and the failure each one exists for is in that +check's docstring in [`checks.py`](workflow_workbench/checks.py) — one place, which is the point. +This section used to repeat all of them in a hand-written table and had already drifted to 10 of +12; a test now refuses a second table of check names anywhere outside the generated block. All of that is **structural**. None of it says the workflow produces good answers: `trim_only` passes every one of those and gets half the cases wrong. Structural consistency is what a specification guarantees; behaviour is what the battle is for. -### Working with a coding agent - -The specification is the reviewable artifact. Review the diagram and the contracts, and the -agent's job narrows to step bodies satisfying a declared input and output type for a named role, -with `coherence_check()` as the acceptance test. - -A proposed change to the workflow itself is then a diff to `nodes` and `edges` — one small place, -reviewed on its own, not a behaviour change buried in a function body. - -## Current limitations +### What cannot be declared - **The `BaseNode` authoring style cannot be declared.** It returns its own successor, so declared `edges` would be a claim it is free to ignore. Everything a `BaseNode` is used *for* — @@ -435,6 +452,8 @@ Row by row, with their code beside ours: [`docs/parity.md`](docs/parity.md). | [`docs/parity.md`](docs/parity.md) | every Pydantic Graph builder feature, declarable or not | | [`docs/how-it-runs.md`](docs/how-it-runs.md) | their executor from the source, with a probe behind every claim | | [`examples/greeting.py`](examples/greeting.py) | the walkthrough above; beside it a counter, a fan-out, subgraphs, extraction | +| [`examples/ladder/their_hello.py`](examples/ladder/their_hello.py) | the control — Pydantic Graph's own smallest program, no workbench in the file | +| [`examples/contestable.py`](examples/contestable.py) | four judgement-call stages, two strategies, nothing implemented and no score | Downstream of community requests for [reusable/extensible nodes](https://github.com/pydantic/pydantic-ai/issues/798) and diff --git a/tests/test_reference.py b/tests/test_reference.py index 52fd9cd..f6cb8b9 100644 --- a/tests/test_reference.py +++ b/tests/test_reference.py @@ -20,6 +20,7 @@ import workflow_workbench as ww import workflow_workbench.checks as checks from workflow_workbench.reference import ( + BLOCKS, GROUPS, NOT_THE_LANGUAGE, language_markdown, @@ -122,6 +123,43 @@ def test_every_dotted_reference_in_a_docstring_resolves() -> None: assert not dead, f"docstrings name attributes that do not exist: {dead}" +def test_the_readme_shows_the_pydantic_graph_lineage() -> None: + """⛔ `greeting.py` IS their smallest complete builder program, morphed — and the README never + said so. The lineage lived in a docstring, so the comparison a reader wants (*here is theirs, + here is the same thing declared*) was invisible to anyone who stopped at the README. + + Required by name because a paragraph nothing holds in place is the first thing a prune + deletes, and the prune is this same change. + """ + readme = (Path(__file__).resolve().parent.parent / "README.md").read_text() + for needed in ("their_hello.py", "visualize_graph", "This is **their** example"): + assert needed in readme, ( + f"the README no longer shows the Pydantic Graph lineage ({needed!r} is gone). " + "Drop this test on purpose, or restore it.") + + +def test_the_readme_holds_no_second_table_of_check_names() -> None: + """⛔ IT HELD ONE, AND IT HAD DRIFTED. A hand-written `| check | catches |` table sat ~190 + lines below the generated one and listed 10 of 12 — `check_recursion` and + `check_transform_edges` had landed and nobody re-synced the copy. + + Two tables of one set of facts, one of them generated. `.claude/rules/spec-as-code.md`: a + document is source or derived, and mixing them is the whole failure mode. + """ + readme = (Path(__file__).resolve().parent.parent / "README.md").read_text() + outside = readme + for tag in (t for t, _ in BLOCKS): + start, end = f"", f"" + head, rest = outside.split(start, 1) + _, tail = rest.split(end, 1) + outside = head + tail + rows = [ln for ln in outside.splitlines() + if re.match(r"^\|\s*`check_[a-z_]+`", ln.strip())] + assert not rows, ( + f"{len(rows)} table rows name a check outside the generated block — a second, " + f"hand-maintained copy of the rules table:\n " + "\n ".join(rows[:4])) + + def test_no_prose_anywhere_states_a_rule_count_the_generator_disagrees_with() -> None: """⛔ THIS TEST'S OWN FIRST VERSION MISSED THREE SITES, and that is the lesson in it. From 0b814be376179ee4ad7c358088f102dee58193aa Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 6 Oct 2026 18:45:30 +0000 Subject: [PATCH 2/4] Stuff the README with the learnings, including four rescued from an orphan branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Boris: *"before I start manually editing the README I need to make sure you stuff it with all our learnings....then I will refactor and prune and reorg."* So this is deliberately MAXIMAL. 3,407 -> 4,425 visible words. Pruning is the next pass and it is his. ## Four of these were orphaned on `health-stack-example` That branch has 4 commits, is not in `main`, is two renames behind it, and the code on it was decided against (#14). The KNOWLEDGE in it was not, and would have died with it: - **Same Python type, different meaning.** `given_graph` / `cited_given_graph` / `expanded_graph` — what the user said, what the literature backs, what was added on top. A type checker cannot tell them apart and nothing downstream can either. The README's opening claimed that failure abstractly; it is now shown. - **⛔ A strategy is an ALGORITHM, not an environment.** The single most likely misuse of this library and it was documented NOWHERE. Fake vs production clients belong in `ctx.deps`, not in two strategies. The test: do the two arms deserve evaluation on the same cases? A fake and a real client do not, and "the fake scored worse" is not a finding about anything. - **Two contracts that type-checked and were wrong:** edges returned without the concepts they point at, so every id dangled; and a store query with subject and object the wrong way round, which silently returned nothing against a real index — i.e. "no evidence found", the most plausible wrong answer available. - **Recall and precision fail differently, so they are two stages.** Candidate generation vs selection: one score over both averages unrelated failures and sends you at the wrong half. ## And the uncomfortable one, from this repo's own review history Across five rounds of review on one change, EVERY finding was a check that was narrower than its claim — not code that was wrong. The dead-link test skipped malformed links; "every rule" listed 10 of 12; the retired-API lint had no token for the method that release removed; the compatibility oracle enumerated five string operations and omitted the three that broke; a UI test drove one of three render surfaces. For a library whose product IS checks, that is the failure mode a user should expect in their own use of it, so it is on the front page rather than in a postmortem nobody reads. ## Design answers that were only in a conversation - **The three-state test for nesting**, replacing a slogan that conflated "I do not know which algorithm wins" with "I do not know what result I want". The second is not repaired by a boundary — it produces a number over an unstated objective, and a number reads as signal. `StepSpec.problem` is how you leave that row. - **Why there is no LLM-node type**, which was asked for directly: it would declare the answer to the question a battle asks, and make the deterministic arm illegal by construction. - **Pattern vs example, made checkable** — importable, ships a Dataset and >=2 strategies, and has its noise floor recorded. By that bar this repo has ZERO patterns, which is stated outright. - **`eval_battle` does not take the floor for you**, and a battle can therefore report a delta inside it. Named as a live gap rather than left for a reader to discover. - **Two names we expect to change**, as proposals with reasons, not as quiet pending work. - **Where this is going**, so the gap between built and intended is readable rather than inferred. ⚠️ Two naming notes are PROPOSALS and say so. `.claude/rules/domain-language.md` #3 — propose language changes, do not make them. 280 tests, both generators green. Co-Authored-By: Claude Opus 5 --- README.md | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/README.md b/README.md index bb47c07..c447387 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,13 @@ silently dropped, two wires crossed between values of the same type, a stage nobody implemented. It runs, it returns something of the right shape, and nothing downstream can tell. +The second one is worth making concrete, because it is the failure we have actually hit. A real +pipeline carried three values — `given_graph`, `cited_given_graph`, `expanded_graph` — all the +**same Python type** and all meaning different things: what the user said, what the literature +backs, and what was added on top. A type checker cannot tell them apart. Nothing downstream can +either. Naming each value in the declaration is what makes crossing two of them a finding instead +of a plausible answer. + Workflow Workbench is a [declaration layer](docs/glossary.md#declaration-layer) over Pydantic Graph Builder. You write the workflow's shape and its data contracts as **data**, before any step exists — which is what makes that class of defect findable: @@ -149,6 +156,13 @@ a design is data — a diagram, a coherence check and a diff with **nothing impl delta has to clear. It is `0.00` here because both implementations are deterministic — a `0.00` floor on a model-backed arm usually means a cache answered the second run. +⚠️ **Taking that floor is currently YOUR job, and that is a real gap.** `eval_battle` labels a +replicate when you hand it the same strategy twice; it does not run one for you. So a battle can +report arm B ahead by 0.08 while the same arm scores ±0.125 against itself — which is not +hypothetical, it is a measured result from the first corpus this was used on, where one metric's +floor (6.0) was larger than its mean (5.0). **A delta inside the floor is not a result**, and +nothing yet stops you reporting one. + The whole example: [`examples/greeting.py`](examples/greeting.py). ## Quickstart @@ -331,6 +345,24 @@ spec.diagram() # -> mermaid for the specification This is the stage a built `Graph` cannot reach: a `Graph` needs every function to exist first. +### ⛔ A strategy is an ALGORITHM, not an environment + +The single most likely way to misuse this. **Fake services versus production services are not two +strategies.** Infrastructure goes in `ctx.deps`, where Pydantic Graph already puts it. + +``` +deps a fake UMLS client vs the real one; a stub LLM vs a live one; a local file vs a service +strategy joint vs staged extraction; exact-triple vs neighbourhood discovery; compact vs full +``` + +The test is whether the two arms **deserve to be evaluated on the same cases**. Two +implementations of one algorithm do. A fake and a real client do not — the fake exists so the +real one's cost is not paid in a test, and "the fake scored worse" is not a finding about +anything. + +Bind an environment as a strategy and the battle reports a difference that is real, meaningless, +and indistinguishable from the one you were looking for. + ### 3. Implement the steps, then bind them as named strategies The step bodies are ordinary Pydantic Graph steps — nothing in them refers to this library: @@ -402,6 +434,52 @@ battle = eval_battle(spec, trim_only, normalize_spaces, dataset()) # the compar `eval_battle` takes one `spec` and two strategies, so both arms render from the same nodes, edges and types. There is nowhere to put a second design. +## When does a stage become a nested graph? + +A step can be implemented by a whole child design rather than a function. The useful question is +when to do that, and the tempting answer is wrong. + +**The tempting answer:** *"if you cannot say what a good implementation looks like, it is not a +step — it is a sub-design."* That reads well and it conflates two different situations: + +| can you state the objective? | can you settle it by reading the code? | | +|---|---|---| +| **no** | — | **not ready to be anything.** Go write the objective. | +| yes | yes | **a step.** A function; review is the oracle. | +| yes | **no** | **a sub-design.** Give it a boundary and battle it. | + +The slogan merges rows 1 and 3 — and row 1 is the dangerous one, because a boundary plus an +evaluation over an *unstated* objective still produces a number, and a number reads as signal. +Decomposition cannot repair an objective nobody wrote down. + +**`StepSpec.problem` is how you leave row 1.** Non-empty means somebody stated what this stage is +for. Empty means nobody wrote one — never that the stage is easy. + +A worked case: resolving a medical term to an ontology id looked like one stage and is two. +Generating candidate ids is a **recall** problem; choosing among them is a **precision** problem. +They fail for different reasons, so a single score averages two unrelated failures and tells you +to fix the wrong half. Two stages, two scores, and the boundary is what makes the second score +exist at all. + +### ⛔ There is no "LLM node" type, deliberately + +The obvious request is a node subtype that forces richer fields — a problem, a rubric, its own +evaluation — for stages backed by a model. It is the wrong mechanism for three reasons: + +- **It declares the answer to the question a battle asks.** The whole point is that one role takes + an LLM arm *and* a deterministic arm. Type the role as an LLM node and the deterministic arm is + illegal by construction. +- **Whether a stage calls a model is a fact about the implementation**, not about the role. It + belongs to the strategy, which is where it already is. +- **Forcing prose destroys the signal it was added for.** A required brief means every node has a + sentence, and the ones written to satisfy a type checker are indistinguishable from the ones + written because somebody thought. + +**Richness follows the boundary, not the node type.** A stage that deserves a problem, a rubric +and its own evaluation is a sub-design — and a graph already has all three, because a graph has a +declared public boundary you can build a dataset against. That is the one thing a `StepSpec` does +not have. + ## Working with a coding agent The specification is the reviewable artifact. Review the diagram and the contracts, and the @@ -426,6 +504,25 @@ check's docstring in [`checks.py`](workflow_workbench/checks.py) — one place, This section used to repeat all of them in a hand-written table and had already drifted to 10 of 12; a test now refuses a second table of check names anywhere outside the generated block. +Two cases worth keeping, because both type-checked and both were wrong: + +- a retrieval step returned edges **without the concepts those edges point at**, so every id + dangled. The declared type was satisfied; the value was unusable. +- a store query returned subject and object **the wrong way round**. Same type, same shape, and + it silently returned nothing against a real 6.9 GB index — which reads as "no evidence found", + the most plausible wrong answer available. + +⚠️ **And the uncomfortable one, from this repo's own review history.** Across five rounds of +review on one change, **every single finding was a check that was narrower than its claim** — not +code that was wrong. The dead-link test skipped malformed links. The table that said "every rule" +listed 10 of 12. The retired-API lint had no token for the method that release removed. The +compatibility oracle enumerated five string operations and omitted the three that broke. A UI +test drove one of three render surfaces. + +The code was mostly right. **The things asserting it was right were the defects.** For a library +whose product is checks, that is the failure mode to expect in your own use of it: a check that +looks authoritative because it is specific, and is specific about the wrong thing. + All of that is **structural**. None of it says the workflow produces good answers: `trim_only` passes every one of those and gets half the cases wrong. Structural consistency is what a specification guarantees; behaviour is what the battle is for. @@ -443,6 +540,37 @@ specification guarantees; behaviour is what the battle is for. Row by row, with their code beside ours: [`docs/parity.md`](docs/parity.md). +## An example teaches the mechanism. A pattern has been battled. + +The ambition is that reusable designs accumulate — the library supplies the grammar, you grow the +vocabulary. The hazard is that "pattern" becomes whatever somebody wrote six paragraphs about, so +the bar here is checkable rather than literary. A **pattern** is a reusable design that: + +1. is importable by name — a graph plus its strategy, not a script that runs top to bottom; +2. ships a `Dataset` and at least two strategies, so `eval_battle` runs on it; +3. **has its noise floor recorded.** + +(3) is the one that bites. A pattern claiming an arrangement helps, with no floor behind the +claim, is the same mistake as reporting a 0.08 delta against a 0.125 floor — with a grander name +on it. + +⛔ **By that bar this repo has zero patterns today, and that is the honest state.** `greeting.py` +has a battle and says outright it is not contestable. `contestable.py` has a test forbidding it +from printing a score. There is no registry and no `Pattern` type, because there is nothing to put +in one yet — the definition is written down so the first real one has a bar to clear. + +### ⚠️ Two names we expect to change + +Proposals, not decisions — they are tracked, not quietly pending: + +- **`SubgraphBinding`.** In graph theory a *subgraph* is a subset of a graph's own vertices and + edges. A child design is not that: it is a separate graph whose result substitutes for one node. + "Nested graph" is accurate; "subgraph" is a borrowed word bent to fit. +- **`blocking` as a bool.** Two states and no third had been observed when it was chosen. A third + has since been observed — near-duplicate *names* are mechanically detectable and are a smell + rather than a provable defect, so they do not belong among findings where every one is a defect. + The likely answer is a separate check with its own exit code, not a widened enum. + ## More | | | @@ -460,6 +588,21 @@ Downstream of community requests for [reusable subgraphs](https://github.com/pydantic/pydantic-ai/issues/3901) — complementary to native Pydantic Graph, not a proposal to change it. +## Where this is going + +Stated because the gap between what is built and what is intended should be readable, not +inferred. Tracked as issues, not promised here. + +| | | +|---|---| +| **a flagship example that earns the library** | one graph IR in, competing reports out — four stages that are each a judgement call, two strategies differing in one. `contestable.py` is a placeholder for this shape with the nouns marked as stand-ins. | +| **`eval_battle` takes its own noise floor** | so a delta inside the floor cannot be reported as a result | +| **a problem brief on `GraphSpec`, not only on `StepSpec`** | a reusable design has its own problem, separate from the role it happens to fill | +| **the nested-graph rename** | see the naming note above | + +Not planned: a rubric field on a node, an advisory severity, or a node subtype for model-backed +stages. Each was asked for and each is answered above by a boundary instead of a field. + ## Licence MIT — see [LICENSE](LICENSE). From 1f2bc96be72711f6a3b067970aa94a19b7fdd432 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 6 Oct 2026 19:06:15 +0000 Subject: [PATCH 3/4] Boris's review: A rewritten, C cut, the domain stripped out of B and D MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four learnings went in; he pushed back on three, and was right about two and a half. **A — "makes no sense."** The mechanism was right and the illustration was wrong: three domain nouns (`given_graph`, `cited_given_graph`, `expanded_graph`) with no context, in the FIRST SCREEN of a generic library's README. Exactly the complexity objection he had already made about the health-stack examples, and I reproduced it in prose. Rewritten with the illustration that was already on the page: `raw_name` and `clean_name` are both `str`. Wire the raw one into `compose` and nothing complains — right type, right shape, and the normalization stage silently stops mattering. Zero new nouns, and it is the exact bug the example's own first stage exists to prevent. **C — cut.** Two contracts that type-checked and were wrong: edges without their concepts, and a reversed subject/object. Both are nobsmed defects, **nothing in this library could have caught either**, and they were propping up a sentence the very next paragraph already makes. An anecdote that needs domain context to parse and supports an already-clear point is cost with no benefit. **D — the learning survives, the framing does not.** "Resolving a medical term to an ontology id" was the leak. The generic statement is stronger anyway: does this stage have ONE failure mode or two? Anything that retrieves and then chooses has two — a missing candidate is recall, a wrong pick from a good set is precision, different causes and different fixes, and one score over both points you at the wrong half. **B — kept, acronym removed.** He approved the substance; "a fake UMLS client" still meant nothing to a reader of this repo. Now a fake search client, a stub model, a file vs a service. A full sweep for domain leakage across the README comes back clean. 4,425 -> 4,346 visible words. 280 tests, both generators green. Co-Authored-By: Claude Opus 5 --- README.md | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index c447387..83201fe 100644 --- a/README.md +++ b/README.md @@ -5,12 +5,11 @@ silently dropped, two wires crossed between values of the same type, a stage nobody implemented. It runs, it returns something of the right shape, and nothing downstream can tell. -The second one is worth making concrete, because it is the failure we have actually hit. A real -pipeline carried three values — `given_graph`, `cited_given_graph`, `expanded_graph` — all the -**same Python type** and all meaning different things: what the user said, what the literature -backs, and what was added on top. A type checker cannot tell them apart. Nothing downstream can -either. Naming each value in the declaration is what makes crossing two of them a finding instead -of a plausible answer. +The second one is worth making concrete, because it is the one a type checker cannot help with. +The example below carries `raw_name` and `clean_name` — **both `str`**. Wire the raw one into the +step that composes the greeting and nothing complains: right type, right shape, and the +normalization stage silently stops mattering. Declaring each value by NAME, not just by type, is +what turns that into a finding. Workflow Workbench is a [declaration layer](docs/glossary.md#declaration-layer) over Pydantic Graph Builder. You write the workflow's shape and its data contracts as **data**, before any step exists — which is what makes that class @@ -351,8 +350,8 @@ The single most likely way to misuse this. **Fake services versus production ser strategies.** Infrastructure goes in `ctx.deps`, where Pydantic Graph already puts it. ``` -deps a fake UMLS client vs the real one; a stub LLM vs a live one; a local file vs a service -strategy joint vs staged extraction; exact-triple vs neighbourhood discovery; compact vs full +deps a fake search client vs the real one; a stub model vs a live one; a file vs a service +strategy one call vs two; extract-then-verify vs extract-only; a wide search vs a narrow one ``` The test is whether the two arms **deserve to be evaluated on the same cases**. Two @@ -455,11 +454,11 @@ Decomposition cannot repair an objective nobody wrote down. **`StepSpec.problem` is how you leave row 1.** Non-empty means somebody stated what this stage is for. Empty means nobody wrote one — never that the stage is easy. -A worked case: resolving a medical term to an ontology id looked like one stage and is two. -Generating candidate ids is a **recall** problem; choosing among them is a **precision** problem. -They fail for different reasons, so a single score averages two unrelated failures and tells you -to fix the wrong half. Two stages, two scores, and the boundary is what makes the second score -exist at all. +**The test that actually decides it:** does this stage have ONE failure mode or two? Anything +that retrieves and then chooses has two — missing a candidate is a *recall* failure, picking the +wrong one from a good set is a *precision* failure, and they have different causes and different +fixes. One score over both averages them and points you at the wrong half. Two boundaries, two +scores, and the second score only exists because the boundary does. ### ⛔ There is no "LLM node" type, deliberately @@ -504,15 +503,7 @@ check's docstring in [`checks.py`](workflow_workbench/checks.py) — one place, This section used to repeat all of them in a hand-written table and had already drifted to 10 of 12; a test now refuses a second table of check names anywhere outside the generated block. -Two cases worth keeping, because both type-checked and both were wrong: - -- a retrieval step returned edges **without the concepts those edges point at**, so every id - dangled. The declared type was satisfied; the value was unusable. -- a store query returned subject and object **the wrong way round**. Same type, same shape, and - it silently returned nothing against a real 6.9 GB index — which reads as "no evidence found", - the most plausible wrong answer available. - -⚠️ **And the uncomfortable one, from this repo's own review history.** Across five rounds of +⚠️ **The uncomfortable case, and it is from this repo's own review history.** Across five rounds of review on one change, **every single finding was a check that was narrower than its claim** — not code that was wrong. The dead-link test skipped malformed links. The table that said "every rule" listed 10 of 12. The retired-API lint had no token for the method that release removed. The From dab42a39778c921f1c52d6648eb811f76e47b5d6 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 6 Oct 2026 20:31:47 +0000 Subject: [PATCH 4/4] Copilot on #18: the lineage table was FALSE, and my own guard could not tell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings are mine, both are the shape this branch is about, and the first is the worst kind — a confidently wrong doc added in the commit that added a test to protect it. ## 1. The table said `their_hello.py` contains `step_a`/`step_b`. It does not. That file is ALREADY a greeting adaptation of upstream: its steps are `pick` (returns `"Hello"`) and `compose` (returns `f"{ctx.inputs}, {ctx.state.name}!"`). `step_a`/`step_b` appear only in its docstring, describing the upstream program it was adapted FROM. So the row I added pointed at a file and described something else. Three accurate rows now, and they make the point better than two wrong ones did: upstream, unchanged their visualize_graph.py step_a -> 10 the control their_hello.py pick -> "Hello" no workbench ours greeting.py normalize -> clean declared ⚠️ **`test_the_readme_shows_the_pydantic_graph_lineage` passed the whole time**, because it asserted the WORDS were present, not that they were TRUE. That is the same defect as every finding on #8, now committed by me in the test written to prevent it. It reads the control's source and requires the row to name that file's real steps, and refuses `step_a` in that row specifically. ## 2. "All 12 rules" sat OUTSIDE the generated markers So `reference --write` could not update it, and `test_no_prose_anywhere_states_a_rule_count...` matched "well-formedness rules" and "check functions" and not "12 rules" — a thirteenth check would have left the summary stale with every test green. The numeral is gone; the count lives only in the generated block, which is where it is derived. The regex covers a bare `rules` now, so re-introducing one goes red rather than passing quietly. Both mutation-tested: restoring the false table -> RED, putting a stale numeral back -> RED. 280 tests, both generators green. Co-Authored-By: Claude Opus 5 --- README.md | 9 +++++---- tests/test_reference.py | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 83201fe..f3b5eb3 100644 --- a/README.md +++ b/README.md @@ -95,8 +95,9 @@ so the difference you are looking at is the declaration layer and nothing else: | | step 1 | step 2 | |---|---|---| -| **theirs** — [`their_hello.py`](examples/ladder/their_hello.py), no workbench in the file | `step_a` → `10` | `step_b` → `f'Result: {ctx.inputs}'` | -| **ours** — [`greeting.py`](examples/greeting.py) | `normalize` → a clean name | `compose` → `f'Hello, {name}!'` | +| **upstream**, unchanged — their [`visualize_graph.py`](https://pydantic.dev/docs/ai/graph/builder/) | `step_a` → `10` | `step_b` → `f'Result: {ctx.inputs}'` | +| **the control** — [`their_hello.py`](examples/ladder/their_hello.py): upstream's shape, a greeting instead of a number, and no workbench in the file | `pick` → `"Hello"` | `compose` → `f"{ctx.inputs}, {ctx.state.name}!"` | +| **ours** — [`greeting.py`](examples/greeting.py), the same workflow declared | `normalize` → a clean name | `compose` → `f"Hello, {name}!"` | Theirs is fine, and that is the point of keeping it: one graph with one implementation per step runs perfectly well like that. What it cannot do is check or draw itself before the steps are @@ -213,7 +214,7 @@ missed. ## What `coherence_check()` enforces
-All 12 rules — generated from each check's own docstring +Every rule — generated from each check's own docstring **12 rules.** `coherence_check()` returns one finding per violation and an empty list for a clean design; `render()` refuses on any finding that blocks. @@ -571,7 +572,7 @@ Proposals, not decisions — they are tracked, not quietly pending: | [`docs/parity.md`](docs/parity.md) | every Pydantic Graph builder feature, declarable or not | | [`docs/how-it-runs.md`](docs/how-it-runs.md) | their executor from the source, with a probe behind every claim | | [`examples/greeting.py`](examples/greeting.py) | the walkthrough above; beside it a counter, a fan-out, subgraphs, extraction | -| [`examples/ladder/their_hello.py`](examples/ladder/their_hello.py) | the control — Pydantic Graph's own smallest program, no workbench in the file | +| [`examples/ladder/their_hello.py`](examples/ladder/their_hello.py) | the control — their smallest program's SHAPE, adapted to a greeting, with no workbench in the file | | [`examples/contestable.py`](examples/contestable.py) | four judgement-call stages, two strategies, nothing implemented and no score | Downstream of community requests for diff --git a/tests/test_reference.py b/tests/test_reference.py index f6cb8b9..584b207 100644 --- a/tests/test_reference.py +++ b/tests/test_reference.py @@ -75,8 +75,11 @@ def test_every_check_in_the_module_reaches_the_rules_table() -> None: #: A count of CHECKS stated in prose. Deliberately NOT a bare `rules` — `docs/ladder.md` says #: "eleven rungs" and that is a different eleven. +#: ⚠️ `rules` is in here because a hand-written "All 12 rules" sat in a
summary +#: OUTSIDE the generated markers — so `reference --write` could not update it and this test did +#: not match it. Two phrasings covered, and the stale one was a third. _COUNT = re.compile(r"\b(\d+|" + "|".join(_WORDS) + r")\s+" - r"(?:well-formedness rules|check_\* functions|check functions)\b") + r"(?:well-formedness rules|check_\* functions|check functions|rules)\b") def test_write_does_not_announce_success_on_a_partial_write() -> None: @@ -131,12 +134,27 @@ def test_the_readme_shows_the_pydantic_graph_lineage() -> None: Required by name because a paragraph nothing holds in place is the first thing a prune deletes, and the prune is this same change. """ - readme = (Path(__file__).resolve().parent.parent / "README.md").read_text() + root = Path(__file__).resolve().parent.parent + readme = (root / "README.md").read_text() for needed in ("their_hello.py", "visualize_graph", "This is **their** example"): assert needed in readme, ( f"the README no longer shows the Pydantic Graph lineage ({needed!r} is gone). " "Drop this test on purpose, or restore it.") + # ⛔ THE HALF THAT WAS MISSING, and it is why the first version passed on a FALSE table. + # Asserting the words are present says nothing about whether they are true: the row for + # `their_hello.py` claimed it contains `step_a`/`step_b`, which is upstream's shape and NOT + # what that file holds — it is already a greeting adaptation whose steps are `pick` and + # `compose`. A lineage claim has to be checked against the file it names. + control = (root / "examples" / "ladder" / "their_hello.py").read_text() + row = next(ln for ln in readme.splitlines() if "their_hello.py" in ln and ln.startswith("|")) + for step in ("pick", "compose"): + assert f"`{step}`" in row, f"the control's row no longer names its real step {step!r}" + assert f"def {step}(" in control, f"{step!r} is not a step in their_hello.py any more" + assert "step_a" not in row, ( + "the control's row attributes upstream's `step_a`/`step_b` to their_hello.py, which is " + "an adaptation and does not contain them") + def test_the_readme_holds_no_second_table_of_check_names() -> None: """⛔ IT HELD ONE, AND IT HAD DRIFTED. A hand-written `| check | catches |` table sat ~190