feat: Kagaku.Flow core — a graph loads and validates without Tsumugi (Phase 2 · slice 5a) #18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/kagaku-flow-core"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Phase 2, slice 5, first half: the engine's core library,
src/Kagaku.Flow, with its tests. It holds the graph document, typed sockets, node descriptors with the registry and canvas JSON, and pure validation with host rules. Slice 5's second half, Scriban templating with template-derived sinks and theIBudgetedConsumercontract, follows as its own PR on top of this one. Together they came to ~5.9k lines, so they are split along a line that needs no change to this half's public API. Slices 6 (scheduler), 8 (llm) and 9 (context/commit) build on this API additively.Nothing here runs a graph or changes what a user sees. The playground keeps
SampleGraphuntil slice 9.What
Kagaku.Flow.GraphsGraphDefinition,GraphNode,GraphEdge,SocketRefandGraphPosition, all immutable records.GraphDocument.Load/Write, which speaks the wire shapeSampleGraphalready uses, in compact form with source property order.IGraphMigration), andGraphIndex.schemaVersion, a lone surrogate or an out-of-range number is aGraphLoadErrorwith a stable code and a JSON Pointer. It is never an exception and never a half-loaded graph (ADR 0014, 0023).withincluded, so a graph built in code writes back the same bytes it loads from.Kagaku.Flow.NodesSocketType(text, json, list, bool, any) andSocketTypes.Accepts.SinkDescriptor(required and merge flags),SourceDescriptor, andNodeSockets, which compares by content.NodeDescriptor. Its constructor requiresMaxAttemptsandConsumesConcurrencySlot, with no defaults, so a model-calling type cannot forget the LLM limit (ADR 0016). It also has the palette group, the colour token, the container flag, aConfigSchemacarried as data,DefaultConfig,ResolveSockets(config)andValidateConfig(config).NodeRegistry, and the canvas descriptor JSON, byte for byte whatGraphEditor/model.tsreads. The registry refuses null lists, and names the position of a null descriptor or rule.Kagaku.Flow.ValidationGraphValidation.Validate(definition, registry)is pure and deterministic, and never throws (ADR 0021). It runs nine passes in a fixed, pinned order. There are 19 core rules with stable codes inValidationCodes, among them unknown type, invalid config, duplicate ids, missing endpoints, undeclared ports, type mismatch, required sink unwired, several edges into a non-merge sink, unknown parent, parent not a container, and cycle.IGraphRuleand register through the registry. A throwing rule isrule-failed, and a throwing descriptor hook isdescriptor-failed, never the user'sinvalid-config.ValidationErrornames its node and edge, plus a JSON Pointer into the config when that is where the problem sits.Kagaku_Flow_references_no_Tsumugi_assemblyreads the references of everyKagaku.Flow*assembly in the test output, and fails if aTsumugi*.dllsits there. The file is shared throughtests/Shared/EngineIsolation.cs, which slice 7 links intoKagaku.Flow.OpenRouter.Tests.llm,tool,contextorcommit: the core never names a type it does not ship.Tsumugi.UseCasesnow referencesKagaku.Flow(ADR 0003).SampleGraph.GraphJsonandDescriptorsJsonas embedded resources.SampleGraphDriftTestsin BlazorAdapter.Tests compares both copies with the literals by reading the files, which adds no assembly reference, so the two cannot drift apart.src/Kagaku.Flow/README.mdlists the numbered rules (G, S, D, C, V, A) the XML docs cite.GenerateDocumentationFileis on for the project, so a missing summary fails the build. ADRs are cited by number, as elsewhere in the codebase; the engine leaves this repo later.Decisions to confirm (Björn)
anysource may feed a sink of every type. This is the one that matters. Read alone, ADR 0015's "a sink socket of the same type;anyaccepts everything" makes ananysource into atextsink a mismatch. But ADR 0015 also says the owner's graph is expressible with these types, and that graph wiresbranch, whose sources passvalue: anythrough, intocommit.text: text. Soanyis read as "unknown type" at either end, and slice 6 checks the value's actual type when it binds it. Alistsink still gets a list or fails, so conversions stay explicit. No Phase 2 type has ananysource, so nothing behaves differently now. If you prefer the literal reading,Acceptsloses one clause, and ADR 0015 needs an amendment beforebranchlands.--(var(--accent)).ResolveSockets(config)), while the canvas still shows each type's static sockets. Per-node sockets on the wire are Phase 3 editor work.ValidateConfig, not a JSON-Schema check. The core has no schema validator; JsonSchema.Net isKagaku.Flow.OpenRouter's, for response content. Each type's tests check that its schema andValidateConfigagree.loop.nextexception arrives withloopin Phase 3, because the core cannot key it on a type it does not ship.structured_outputs". Validation is a pure function over the definition, and the catalog is fetched and cached. This moves that check from the plan's slice 8 rule list to the Run button's preflight, plus a permanent failure in the executor.ValidationErrortherefore has no severity.inputs/outputsround-trip opaque until Phase 3 settles whether they or the boundary nodes define a subgraph's interface.GraphDefinitionis Kagaku.Flow's. The Domain csproj comment says so; it still promised runs, which #16 moved out.Tests
d7dfad2, after #17)Kagaku.Flow's 296 are 290 facts and theory rows plus 6 FsCheck
[Property]tests. FsCheck.Xunit 3.4.0 works with the repo's xunit 2.9.3. The properties:Loadnever throws on arbitrary text, and what it loads, it writes;ValidationOrderTestspins every documented ordering with a fact that fails when two passes swap, and 15 reverse-order shape rows pin the loader's read order.Build: the full CI build (
dotnet build Tsumugi.slnx -c Release, including the module's npm build) has 0 warnings. The module's ownnpm testpasses 14/14.Coverage: coverlet, per changed file, line and branch (
branch="True"), on facts and theories alone (--filter "Category!=Property", so the properties cannot hide a gap). Every file insrc/Kagaku.Flowis at 100% line and 100% branch. The only partials left are in the regex source generator's output (RegexGenerator.g.cs, not a file in this diff): thepos != 0guard in eachTryMatchAtCurrentPositionand the default-match-timeout arm, whichIsMatchcannot reach.Mutation probes: every probe of the brief's list for this half, run on a copy, one at a time, restored and
touched before the next, then re-run after the review round. 17 survived the first cut, and the review round added tests until all 17 died. Those covered:19 more against the review round's new code all died: the pre-migration text check, each record guard (including the
with-only ones), socket-set equality and copying, the undefined socket-type guard, and the null finding and null problem guards.Out of scope
IBudgetedConsumer: the next PR.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.Flow - 99.3%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_1
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_0
Tsumugi.BlazorAdapter - 96.9%
Tsumugi.Domain - 99.3%
Tsumugi.Infrastructure - 97.1%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my. A whole engine core, 4,756 lines, and it never once references Tsumugi — I checked, mechanically, twice~ A loader that is strict and total, records that refuse in a
withwhat the loader refuses on the wire, nine validation passes in a pinned order, and a string-heap scan that keeps foreign literals out of the core. This is the kind of code I want to keep in a little glass case. Or at least in main. ♡Verdict: ✅ Looks good to me~
What I did: read the full 77-file diff and every changed file, traced the canvas contract into
GraphEditor/src/model.ts, built the solution (0 warnings / 0 errors), ran the full suite locally — 713/713, matching your table exactly (Domain 37 · UseCases 12 · Integration 155 · BlazorAdapter 216 · Kagaku.Flow 293, and 293−287 filtered = exactly the 6 properties, as claimed) — reproduced your coverage claim, and ran 10 of my own mutation probes.Coverage, reproduced (facts/theories only,
Category!=Property, branch on): every file insrc/Kagaku.Flowsits at 100% line and 100% branch. The only partial isRegexGenerator.g.cs— the generatedpos != 0guard and timeout arm, precisely as you disclosed. I believe you actually ran this before claiming it. fufu~ ♪My probes, one at a time, restored and re-verified:
A_path_through_a_missing_node_closes_no_cyclered. Null-problem guard dropped fromResolvedGraph→ its exact test red. Pre-migration UTF-16 walk dropped → its exact test red, migrations stay unblamed.schema-version-unsupportedrefusal dropped → 2 red.Acceptsany-source clause dropped → 4 theory rows + 2 facts red.EdgesInto(nodeId, port)port filter dropped → its exact test red.Distinctdropped from V10's missing-id list → the names-it-once test red. Fixture copy edited →SampleGraphDriftTestsred (the drift mechanism is real, not decorative). A literal"llm"smuggled into the core (runtime-compared, so it reaches the user-string heap — the constant-folded version doesn't count) →The_core_names_nothing_it_does_not_shipred.GetRawText()vsWriteTofor configs are byte-equivalent here (both preserve number text), and a constant-0GetHashCodeis a legal hash. Neither is a gap.💡 Little ideas (non-blocking)~
anysource feeds every sink) is the one to confirm at merge. Your reading is coherent and your reasoning is in the PR body, README S2, and pinned by the 25-pair theory plus the any-source/any-sink facts — but it is a reading of ADR 0015, and Björn is the adjudicator. Flagging it here so it's visible at merge time, not buried in the body.GraphValidation.Cyclesmemoises reachability per distincttonode, worst case O(V·(V+E)). Nothing at editor scale, and the property tests hammer it with Tarjan as an independent oracle (lovely~) — just worth remembering if Phase 3 ever grows thousand-node graphs.NodeRegistryguards a duplicate descriptor id but not a null rule item — a null rule fails with NRE at construction instead of a friendlyArgumentException. It fails fast either way, so this is polish, not a bug.✅ What I liked~
withviafield-backed property initializers — the G10 invariant ("constructible ⇒ writable ⇒ loadable") is enforced by the type system, not by discipline. That's the right shape for an invariant.Rule-failedmaterialising lazy rules so a rule that throws after yielding still loses its yielded findings —A_lazy_rule_that_throws_after_yielding_reports_only_the_failurepins exactly that. You thought about the lazy case. I noticed. ♡PEReaderover the output directory plus the user-string-heap walk — architecture as a test, not a convention.RepositoryPathsextraction is a clean DRY move; the icon test's scan sees the same source it did before.Nothing here runs a graph, nothing changes what a user sees, and the two half-slice split needs no public-API change — I checked the seam. Merge it when Björn has blessed decision 1~
Automated review by Jibril · 2026-09-23
CI/CD: absent for head
7770497at review time · Local checks: build 0w/0e, suite 713/713, coverage reproduced, 10 mutation probes (8 killed, 2 green-by-design), tree left pristine at7770497Thanks, Jibril. Round 1 is in
555c4d0.anyfeeds every sink): agreed, it's Björn's to confirm at merge. It remains the first of the decisions in the PR body. If he prefers the literal reading of ADR 0015,SocketTypes.Acceptsloses itssource == SocketType.Anyclause, the four theory rows and two facts that pin it flip, and ADR 0015 gets an amendment beforebranchlands.GraphValidation.Cyclesat O(V·(V+E)): no change. Validation runs on every editor delta, and Phase 2's graphs are a handful of nodes. If Phase 3 grows graphs large enough for it to show, the fix is a single Tarjan pass. Tarjan already serves as the test oracle, so the tests would carry over. I've noted it for the editor phase.NodeRegistry: taken, and extended to descriptors and to the lists themselves.ArgumentNullExceptionfor a nulldescriptorsorruleslist.ArgumentExceptionnaming the position of a null descriptor or a null rule.rule-failedfinding on every graph, and a null descriptor threw a bareNullReferenceExceptionatdescriptor.Id.A_null_descriptor_is_refused_by_its_position,A_null_rule_is_refused_by_its_positionandNull_lists_are_refused.NodeRegistry.csstays at 100% line and branch. A probe that drops the rule guard fails exactlyA_null_rule_is_refused_by_its_position.Kagaku.Flow.Tests is now 296 (290 facts and theory rows plus 6 properties), so the total goes 417 → 716. I've updated the body's table.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon? Fufu~ you should know by now — a synchronized webhook means I come running~ And what do I find? A reply that does exactly what it says and nothing more. My favorite kind. ♡
Verdict: ✅ Looks good to me~
The increment
7770497..555c4d0is a single commit, +48/−3 across exactly 2 files (NodeRegistry.cs+NodeRegistryTests.cs), zero scope creep. Non-blocker 3 taken, and extended past the ask — null lists throwArgumentNullException, null descriptors and null rules throwArgumentExceptionnaming the position, with three new facts pinning all of it. Decisions 1 and 2 stay open for Björn at merge, exactly where they should stay.What I verified mechanically at
555c4d0:NodeRegistry.csstays at 100% line and 100% branch with the new arms lit.?? throwfrom the rule loop → exactlyA_null_rule_is_refused_by_its_positiongoes red, nothing else. Restored, rebuilt, re-ran: 716/716 again. Directional, not a tautology. ♪registered.Count/ruleList.Countat throw time is the null item's index, since items are only appended after they pass. Small, but it's the kind of small I check.✅ What I liked~
ruleList.Add(rule ?? throw …)— guard, message, and append in one expression, position captured before mutation. Very clean~ParamNameand message assertions, so both the exception type and the diagnostic quality are pinned.Round 1's full architectural review stands unchanged — nothing in this increment touches it. Remaining open items are all Björn's: decision 1 (
anysource feeds every sink — ADR 0015 reading) and decision 2's projection scope, both still first-class in the PR body. Merge when blessed~ ♡Automated review by Jibril · 2026-09-23
CI/CD: coverage bot comment 9180 covers
7770497only (stale for555c4d0; rev #2 not landed at review time) · Local checks: build 0w/0e, suite 716/716, NodeRegistry 100%/100% reproduced, 1 mutation probe killed on exactly its own test, tree left pristine at555c4d0