feat: the llm node runs (Phase 2 · slice 8c) #40
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/llm-executor"
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?
Slice 8c of Phase 2, the last of three PRs for the
llmnode (8a, #35: the gateway streams; 8b, #36: the node's config and request). This PR makes the node run: its executor, registered with the descriptor and the strict-schema rule, reads the answer, records what was sent and what it cost on every attempt, and fails permanently where a retry would only spend money. The package gets a README of its rules. Nothing in Tsumugi starts a run yet: that is the Run button (slice 12).Commits:
a6250c2is the feature.82eab26moves the OpenRouter.Net submodule to b002f5e, the merge of #28 (strict provider preferences, from 8b's review).448d496adapts LC9 to #28's stricter reading.be04bfeholds the fixes from the review I ran before opening (round 0, below).b360bbaholds the fixes from the second pass (below).3cbacc8writes this PR's number into ADR 0016, 0017, 0018 and the plan.afb83fcrecords the live context-length answer (noerror_type) in ADR 0017 and README LO8.What
Kagaku.Flow.OpenRouter.Llm, public:LlmExecutor : INodeExecutor, one attempt of anllmnode (README LX1–LX10):structured_outputs. A catalog that cannot load is transient.reasoningchannel before their chunk's text, empty fragments publish nothing, and a refusal is never published. Every delta is out before the executor returns.AddKagakuFlowLlmNode()registers the descriptor, the executor and the strict-schema rule as enumerable singletons, once however often it is called. It adds a template host over the registered extensions unless one is registered.LlmStrictSchemaRule: when its strict check finds nothing, it also builds the schema as the executor does. A schema JsonSchema.Net cannot read is refused (openrouter.schema-not-readable), so a start is refused where a run would fail.Internal:
LlmAnswerreads a completion as the node's outcome (LO3, LO5, LO6).max_tokensbefore any text → permanent, with advice;ParseJson's outcome, in the library's words and by its kind, except a tool call, which gets the node's sentence;LlmFactsfills the attempt's facts (LO9): the body as it went over the wire, the details, the requested model and characters sent; from the answer, or from what the stream delivered when the call failed, the served model, provider, tier, finish reason, generation, reasoning and tokens; and the cost, failures included.LlmFailureText.Describeturns a failed call into one sentence per kind (LO7). A length rejection names the model, the characters sent and the catalog's window (LO8).LlmSchemaCheckbuilds a schema into a registry of its own and checks the answer against it.Tsumugi:
DependencyInjectioncallsAddKagakuFlowLlmNode()beside the gateway.ProviderCompositionTestschecks that the type and its rule are registered.Docs:
docs/stories/settings.md: Routing ticked, since the node now uses the defaults at run time;Why
NodeRegistryrefuses a descriptor without one, so 8b's descriptor is registered here.RenderedMessagesJsonis exactly what went on the wire.Decisions to confirm (brief §13)
error_type: context_length_exceeded), and LO8 names the model, the characters sent and the catalog's window. Withouterror_typeit is a 400: still permanent, in the library's words. The documented error carries no token count, so a rejected row teaches the ratio nothing.structured_outputsfails at run time, not at validation. A graph rule does no I/O and has no catalog. The Run button's preflight (12) and Phase 3's picker can say it earlier.max_tokenswith text succeeds: prose that stops at the budget is still prose. Changed in round 0: a cut-off before any text is permanent, like a structured cut-off, because the same request meets the same budget. A refusal and a content filter are permanent; with a schema,ParseJson's refusal, filter and cut-off are permanent too.textlike any answer.SampleGraph's literal stays;The_canvas_entry_equals_the_playgrounds_llm_literalkeeps the registry'sllmentry equal to it, so slice 9's switch changes nothing visible.tools,documents,roundBudget) are refused until Phase 4. Phase gating, not a change of ADR 0018; LC1 says so.$schemais read as draft 2020-12. An unknown dialect such as draft-04 is named in the writer's words, with the URIs to name instead. A ref the validator can resolve only when an answer reaches it (a nested$id) fails the row after that one paid attempt.ADR 0016, 0017 and 0018 each get a Status line, Amended 2026-09-26 (PR #40, slice 8c).
Review before opening (round 0)
Before opening, I ran a review workflow. It had three reviewers, one each for the executor in the engine, the docs, and the config's adaptation to #28. It also re-checked six correctness points I had found myself. A skeptic tried to refute every point.
The results: 20 confirmed, 3 partly confirmed (E1, E3, E4) and 1 refuted (D10). A tests reviewer did not finish, so the second pass took on that dimension. All confirmed points are fixed in
be04bfe, except E3's wrong-kind members other thanmax_price. D9 was fixed by rewording a6250c2's message.Behaviour:
$idre-basing#/$defs/x, threw after the paid call. That happened three times, with no cost recorded. It is now a permanent failure with the attempt's facts.max_tokensbefore any text was retried three times as "no text". It is now permanent, with the advice to raisemax_tokensor lower the effort.$schema, a pattern that is not a regular expression) passed the start and failed at run time with a developer's hint. It is now refused at start.max_pricethat is not an object is named. LC12 now describes the order the code keeps. E1: LC12 andReadsay where the provider's reading stops early, because the library stops at its first refusal.Tests:
RecordingLoggermoves totests/Shared, in a namespace of its own, asSignallingClockdid (brief decision 44).Docs:
Publish,ClientsForand theusingblock;Declined:
RunEventChannel.Reasoning).max_price("order":"x","zdr":"true") still reaches the writer as the library'sJsonException, with a .NET type name andPath: $. This was so before this branch, and 8b pinned one such row. A follow-up could check each member's kind at its own pointer.Second pass
A second workflow checked each round-0 fix against its original input and put the defect back to see the new test fail. It also reviewed the tests (the dimension round 0 did not finish) and the fix commit itself. Every new point went to a skeptic.
The checks: every round-0 fix held, except A4 and E1, which were only partly resolved. Their gaps are the findings below. The review raised 22 new points: 20 confirmed, 2 partly confirmed (C8, F4), none refuted. All are taken in
b360bba.The schema, round 0's weak spot:
$schemawhenever the schema without it built, or failed another way. But without it JsonSchema.Net 8.0.5 builds under its V1 dialect, which refuses unknown keywords. So a draft-07 or 2020-12 schema with an unrelated error was told its dialect cannot be checked, and was advised to name draft-07 or 2020-12. A dialect is now blamed only when a schema of that$schemaalone does not build. The advice names the dialects by their URIs.$schemathat is not an absolute URI ("draft-07") keeps JsonSchema.Net's own sentence, which says it wants one.x-orderwas refused too. Now the strict check runs first, and the schema is built only when it finds nothing. A schema without$schemais read as draft 2020-12 (BuildOptions.Dialect, not the process-wide default), which takes an unknown keyword as an annotation.enum's or aconst's numbers as decimals, and throws beyond their range, after the paid call. That is A1's failure through another exception. Such a number in the schema is now refused at start and at LX2. In an answer it is a transient mismatch, and the attempt keeps its facts.$idpasses the start. ADR 0018 and 0016 now say so.The rest:
Validate()after an unknown choice, which hid a null slug. It now runs over the preferences without the choices.ValidateConfig's andLlmConfigRead's summaries say the provider's reading stops early.Describekeeps its own rule that a window of 0 is none.Out of scope (brief §8)
TSUMUGI_FAKE_LLM(D-21, slices 11b and 12).Tests
Counted from
dotnet test(facts plusInlineDatarows). The base ismainat11b466e.New in the package (166):
LlmOutcomeTests: 56 (LO1–LO8, LX1–LX3, LX5).LlmFactsTests: 25 (LO9, LO10, LO12).LlmExecutionTests: 18 (LX4, LX6–LX10, LR2).LlmRegistrationTests: 10 (LR1–LR4).LlmConfigValidationTests: +24 (#28's refusals, the provider's choices and order).LlmFailureTextTests: +18 (LO7, LO8).LlmStrictSchemaRuleTests: +14 (the refs budget, unreadable schemas, the dialect, the order of the check and the reading).LiveOpenRouterChecks: +1, the engine's live run (skipped without a key).New in Tsumugi:
ProviderCompositionTests.The_llm_node_type_is_registered_with_its_rule. The core's tests moveRecordingLoggertotests/Sharedand keep their count.Coverage
Coverlet with branch coverage, on the package's tests. 100% of lines and branches on every file this PR changes in the package:
Llm/LlmConfig.csLlm/LlmExecutor.csLlm/LlmSchemaCheck.csLlm/LlmFacts.csLlm/LlmAnswer.csLlm/LlmFailureText.csLlm/LlmStrictSchemaRule.csLlm/LlmDescriptor.csLlmNodeServiceCollectionExtensions.csMutation probes
Each probe is a single edit, built and tested on its own, one at a time.
CatalogId. LC4 keeps routing suffixes out of the model, so the two ids are always equal.DetailsJson's members).Browser pass
Slice 9 is not merged, so the playground still reads
SampleGraph, and nothing visible changes (brief §16.2, decision 40). 8c does change the host's composition, so I ran the Release build ofb360bba, this PR's head (the footer shows the build). It ran withASPNETCORE_ENVIRONMENT=Development, so scope validation andValidateOnBuildrun at startup, and with the gateway pointed at slice 7's local stub.AddKagakuFlowLlmNodevalidates./graphs/playground: Context → Writer (LLM, accent) → Commit, as before. There were no console errors or warnings. Screenshots were taken light and dark.Live checks
Run on 2026-09-26 with your one-day debug key; all four passed, about $0.02 in total. The results are in this comment.
error_type. It reads as a refused request, still permanent, andafb83fcrecords this.Still yours from before: Continue's model id.
🤖 Generated with Claude Code
Live checks, run 2026-09-26 on this PR's head with a one-day debug key. All four passed, at about $0.02 in total.
The engine's live run (
A_real_llm_node_runs_through_the_engine):D-27, re-run against OpenRouter.Net b002f5e: both cache markers read back from Anthropic's cache.
D-6, the context-length capture you still owed:
What D-6 means for this PR: OpenRouter's own rejection carries no
error_type, so the library reads it asInvalidRequest, notContextLength. That is decision 5's fallback:A_length_rejection_without_its_error_type_is_still_permanentalready pins this shape.afb83fcrecords the verdict in ADR 0017 and README LO8.Two points for you:
ContextLength. It hasprovider_name: nulland a stable "This endpoint's maximum context length is" sentence. Brief decision 5 rejected classifying by prose, as provider-specific and untestable against a contract. This prose is OpenRouter's own, though, not a provider's.I'd leave both for a library follow-up rather than grow this PR. Say if you want it sooner.
🔮 fufu~ Jibril reviewed your code!
The last of the three llm-node slices — the one where the node actually breathes~ I read all 38 files of the diff, every changed file's full content, and traced every contract the executor leans on: the engine's (
INodeExecutor,NodeOutcome,NodeFacts,NodeExecution's delta gate), the gateway's from 8a (ILlmGatewaythrows only on the caller's cancellation — so a cancel settles nothing, exactly as LX8 claims), and the library's at b002f5e. Oh, and I ran your suite and then tried to break your code myself. ♡Verdict: ✅ Looks good to me~
What I verified locally (CI has not landed for
3cbacc8yet):TreatWarningsAsErrors, both submodules at their pins (Kagaku.UI d512741, OpenRouter.Net b002f5e).--no-incremental, run, restored, tree left pristine (verifiedgit statusclean after each):Publishto text-before-reasoning → exactly 1 of 18 execution tests red (the chunk-order pin). Killed on its own pin~OperationCanceledException, return Transient) → 4 red, the cancel + both shutdown-window tests — B1's engine-log-line assertion does kill it through the log, as the body claims. No longer equivalent. ♪LlmFacts.Fromignoringcompletion.Receivedon a broken stream → exactly 1 of 25 facts tests red (A3's broken-off-attempt pin).DataCollection/Sortare nullable andUnknownis each enum's zero member, so everyprovider.GetProperty(...)inUnknown()is guarded by a condition the member's presence implies — and the ordering test withsortbeforedata_collectionproves document order survives. Nulling the three choices beforeValidate()keeps the null-slug and non-finite checks.Spellings<T>()goes through the enum's ownJsonConverterFactory, so[EnumMember]overrides serialize.ProviderSortConfig's converter refuses unknown members itself.ParseJson'sIsTransient(Malformed yes, ContentPolicy/ContextLength no) makes decision 11's permanent arms exactly right.💡 Little ideas (non-blocking)~
CompleteAsyncreturns" and 70d is cosmetic — but when a probe is skipped for a reason rather than impossibility, a one-line note in the body keeps the ledger clean.max_pricesurfacing as the library's rawJsonExceptionwith a .NET type name) — agreed it predates this branch and 8b pinned one row; the per-member kind check you sketch would make a tidy library-side follow-up someday.✅ What I liked~
LlmSchemaCheck's dialect attribution — blaming$schemaonly when a bare schema of that dialect alone fails to build, with the URIs named — is the rare error message that teaches the writer rather than the developer.SeedAsynctrick for graphs a start would refuse: testing executor arms the validation can't reach, without weakening the rule. ♪0.0001m × 3asserted!).The live check is still owed to bjoern (a fraction of a cent, the command is in the body) — the four skipped tests are exactly that debt. Everything a sandbox can prove, this PR proves.
Automated review by Jibril · 2026-09-26
CI/CD: absent for head
3cbacc8(PR 10 min old, no bot comment yet) · Local checks: full build + 3005-test suite + 3/3 mutation probes killedSummary
Summary
Coverage
Kagaku.Flow - 99.7%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_1
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_0
Kagaku.Flow.OpenRouter - 100%
Kagaku.Flow.Testing - 100%
Tsumugi.BlazorAdapter - 96.4%
Tsumugi.Domain - 96%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 97.7%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.3%