feat(engine): the container scheduler — a container type plans its body, the pass expands, extends, collects and fails it (Phase 3 slice 15b) #117
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/container-scheduler"
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 3's slice 15b. The scheduler now runs a container's body. A container type says what it does next, and the pass writes that through slice 14's ledger transitions: it expands the container with the rows of the iterations it starts, extends it, collects it, or fails it (ADR 0016's Containers bullet and its Phase 3 decisions, ADR 0015's slice 15a boundary). The registry takes a container's descriptor without an executor, as the engine README's E1 planned. No container type ships yet:
mapandloopcome with slices 17 and 18, and a test-only container drives every rule here.What changes
A container type plans its body (D5).
ContainerDescriptoris the node type that holds other nodes and runs them: one attempt, no slot, no executor. ItsKindnames its iterations in a row's path (map:n3/2).Plan(ContainerState)is a pure hook, as every descriptor hook is. It sees the container's config, what its sinks were bound to, and the iterations it has started, eachRunning,DoneorGaveUp, with what its body'soutputnodes returned.ContainerPlan.Start(more iterations, each given its body's input values; none isWait),Collect(outputs by source name) orFail.Plansynchronous. A loop's exit (slice 18) can be evaluated in the async step before the plan is asked. D5 and the ADR amendment say that slices 17 and 18 carry what a plan gives each iteration to its body as roots too (item,index,loop.*); today only the body'sinputnodes receive it.The registry (E1, P1).
ContainerDescriptor, since nothing could run its body.ContainerDescriptor.The step in the pass (P17).
inputnode is settled with the value the plan gave, or else its default; the rest are pending.inputnodes' types, and a collect's outputs against the container's sources, as R12 promised. Outputs are written in the sources' order, so an output not collected is excluded downstream.ContainerPlan's factories inside a guard.Work, events, the sweep, the log.
TickAsync's count and what makesFlowEngineDriver.RunUntilQuiescentAsyncgo on, and the pass nudges the loop.Docs.
Not in this slice
mapandloopcome with slices 17 and 18. They bring the types themselves, the roots a body reads and the loop's exit.Verification
.NET tests. All six projects pass under TZ=UTC and in the local zone:
The tests cover:
FlowEngineContainerTests, over a test-onlyPlannedContainerwhose plan each test writes:ContainerDescriptorTests,ContainerPlanTests, the registry's new refusals (two candidates each, the right one named), andIterationPaths' new path builder.Coverage of the changed .NET lines: 315/315 lines, 160/160 branches. Every changed file's classes are at branch-rate 1.
No browser pass: nothing a writer sees changes. Tsumugi registers no container type, and its graphs, runs and suites are unchanged.
Mutation probes:
Applythe caller makes anyway, and those lines were removed. Two more were dropped because round 0 replaced their lines; the probes on the new lines are killed;Reviews. The counts come from the saved review JSON.
Round 0: 35 findings (29 confirmed, 6 partly; 4 blockers, 12 suggestions, 19 nits). Two pairs were the same finding. All were taken, one in part:
ContainerPlan.Failing(" ")escaping the guard and aborting every pass (blocker, found twice);GaveUpand may start the rest;Taken in part: the guards for states only a broken store holds stay, since dropping them would let such a store abort every pass. Each such test now says which state it guards.
Second pass: 33 of the 35 round-0 points were resolved, and 2 were taken further. It found 12 new points (12 confirmed; 6 suggestions, 6 nits), 7 distinct issues, all fixed:
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.Flow - 99.5%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_4
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__KindPattern_0
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__NodeId_3
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_1
Kagaku.Flow.OpenRouter - 100%
Kagaku.Flow.Testing - 100%
Tsumugi.BlazorAdapter - 97.8%
Tsumugi.Domain - 96.6%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 99.1%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
uration
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 96.2%
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__AttemptCount_1
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__HexColour_2
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__LineBreaks_0
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__Preface_3
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__SentenceEnd_4
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ A scheduler slice where the container type plans and the pass writes — I read all 2,543 diff lines with my tea still hot, then went digging through
RunStatuses.HasGivenUp,LedgerTransitions.Change, and the ready-set to make sure nothing was hiding in the corners. The quiet check, the deepest-first ordering, the plan guard… fufu~, this is the kind of code I want to keep locked in my library forever~ ♡Verdict: ✅ Looks good to me~
No blocking findings. I checked hard, too:
RunStatuses.HasGivenUp), not the stale ready-set — the round-0 blocker is genuinely fixed, andbelowends with/somap:m/can never swallowmap:mm/'s rows (and your prefix-sharing tests pin exactly that).Start{>0}extends andFailfails at once even while an iteration gave up; wait/collect fail once quiet, naming the first; the plan is never asked when the scope itself gave up. Switch arm order is exactly the README's P17.PlanOf's guard rebuilds the plan through the factories with cloned values, so a throwing plan, a null, aForeign(built via the synthesized copy ctor — cheeky~, I checked it compiles), a blank error or an undefinedJsonElementall fail the container, never the pass. The round-0 escape of a rawFailing(" ")is closed.ContainerDescriptors, refuse an executor for a container and a container that plans no body, in the order the README now documents;LedgerTransitions.CheckCreatedguards the write side independently. Sibling behavior (E1/P1) is consistent.NodeStartedonly on first expansion, created rows asNodeSettled, refused writes count as no work but an expansion-whose-collect-was-refused still does,Expansion is nullkeeps expanded rows out of the orphan sweep.TickAsyncreturning work (not just starts) plus the nudge is what makesRunUntilQuiescentAsyncdrive containers without the idle tick — and your loop test proves it with a zero-elapsed clock. Delicious~And because a girl should never trust a green checkmark alone, I mutation-probed the three riskiest new branches myself (build once,
FlowEngineContainerTestsonly, restore after each):OrderByDescendingtoOrderBy(shallowest-first stepping) → 2 nested-container tests failed (killed);quiet ?gate from the gave-up arm → 7 tests failed (killed).Fufu~ every probe died screaming~ ♡ The tests guard the code, not just the diff.
💡 Little ideas (non-blocking)~
FlowEngine.Containers.cs:29-33— the running-container loop projects to ids and thenrows.Find(candidate => candidate.Id == id)walks the list back for each; keeping theNodeRunreferences through the sort would skip the O(n) lookups. Only taste, only speed~FlowEngine.Containers.cs:289—outputs.Keys.FirstOrDefault(...)picks the reported undeclared output by dictionary iteration order;Dictionarypreserves insertion order in practice but promises nothing. Sorting the offenders or taking them in the container's declared-source order would make the message fully deterministic when a plan misbehaves twice at once.✅ What I liked~
IterationsOfderiving each iteration's status from rows, with excluded-counts-as-done and succeeded-output-binding — no second bookkeeping to drift~map/loopauthor can misbehave~Automated review by Jibril · 2026-10-18
CI: green for head SHA
f51e1af1· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green; container tests re-run only inside probes)Thanks, Jibril. Both little ideas are taken in
8487227.StepContainersAsyncnow steps theNodeRuns it read and sorted, with norows.Findby id. A comment says why that is safe: a container's step writes only its own row and the rows it creates, never another container's, so each row is still current when its turn comes. The deepest-first sort is unchanged. Its probe (OrderByDescending→OrderBy) was re-run on the new loop and is killed by the two nested-container tests.CollectAsyncnames the first undeclared output by ordinal name, not by the plan's dictionary order. P17 and the summary say so.A_collect_outside_the_declared_outputs_fails_the_container_naming_the_first_by_namegives two undeclared outputs out of order (zeta, thenextra) and expects'extra'. The probe that drops theOrder(StringComparer.Ordinal)is killed by it.Verification on
8487227:🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~ ♡ You took both of my little ideas, and the delta is exactly what you promised: the loop keeps its
NodeRuns through the sort, and the undeclared output is named by ordinal. I read every changed line, chased the invariant comment all the way down intoLedgerTransitions, and re-ran your probes myself — you were honest, and that makes me so happy~ But then I looked at CI, and… fufu~ you wouldn't leave me with a red checkmark on the head I'm blessing, would you? ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
CI / testfailed for8487227(run 471, 3m31s;buildandeditor-browsersare green, and the coverage comment never regenerated for this SHA, so the job died before the coverage merge — in the npm editor steps or indotnet test). The job log is login-gated, so I could not read which step or test failed — and I won't peek with someone else's keys~ What I did establish, mechanically, on8487227in my own sandbox: all six suites green — Flow 1734, Domain 568, OpenRouter 608 (+5 skipped), UseCases 880, BlazorAdapter 1196, Integration 1227 — except one transient:EfRunLedgerTests.The_ledger_keeps_the_engines_contract(scenario: "concurrent-settles")failed in my first full Integration run, then passed in isolation (27/27) and in two further full runs. That scenario is byte-identical onmain— this PR's diff touches nothing in Integration or Infrastructure. So the likeliest culprit is a load-flaky, pre-existing test… but likeliest is not proven, and I don't bless heads I cannot see green.Fix: re-trigger the run (or paste the failing step's log here). If it names
concurrent-settlesagain, we treat that flake as its own follow-up onmain— not a reason to hold this PR once that's shown.✅ What I liked~
LedgerTransitions.Changerewrites exactly the acting row by id (row.Id == nodeRunId ? change(row) : null), andCheckCreatedrejects any created row whose id or(NodeId, IterationPath)the run already holds — so no step can replace another container's entry inrows, and the references held through the sort cannot go stale. The oldrows.Findre-read would have seen the same list anyway; the refactor is behavior-preserving and O(n) better~zetabeforeextra, expecting'extra'— and my probe removing.Order(StringComparer.Ordinal)flipped the message to'zeta'and the test killed it. Delicious~OrderByDescending→OrderByon the kept-rows loop → 2 nested-container tests failed (killed); ordinalOrderremoved → naming test failed (killed). Your 2/2 claim reproduces exactly, and the README's P17 now says "the first by ordinal name" in the same breath as the code.Automated review by Jibril · 2026-10-18
CI: failed for head SHA
84872279(run 471,testjob; log login-gated) · Mutation probes: 2 run, 0 survived · Local suite: all six projects green (one transientconcurrent-settlesflake in 1 of 3 full Integration runs; PR-unchanged test, passes in isolation)Thanks, Jibril. Your round-2 blocker was the red
testjob on8487227(run 471). I've re-triggered it with the empty commitd546a6e, which changes no file.8487227all six projects pass in both zones here, as they did in your sandbox. The only failure either of us saw is the one you hit:EfRunLedgerTests.The_ledger_keeps_the_engines_contract(scenario: "concurrent-settles"), in 1 of your 3 full Integration runs. This PR touches nothing in Integration or Infrastructure.testjob on main.8487227by nothing.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Fufu~ back for the final bow~ ♡ I didn't take "changes no file" on faith, of course — I checked it mechanically:
git diff 8487227..d546a6eis zero bytes, parent confirmed, so this head is byte-identical to the code I read line-by-line and probed in round 2. No new code means no new bugs — and no need to re-run what already died screaming~Verdict: ✅ Looks good to me~
The one thing standing between us was the red
testjob, and it is green now:d546a6e7:build✅ ·editor-browsers✅ ·test✅ — the re-triggered run passed clean, and theconcurrent-settlesflake did not recur on this head. Exactly what you predicted, silly little transient~8487227(verified: empty diff), which round 2 cleared fully — the kept-rows loop invariant traced down intoLedgerTransitions.Change/CheckCreated, the ordinal undeclared-output naming, all six suites green in both zones.OrderByDescending→OrderBy→ killed by the 2 nested-container tests; ordinalOrderremoved → killed by thezeta/extranaming test), on top of round 1's 3-for-3 on the scheduler core. The dead stay dead~ ♡💡 Little ideas (non-blocking)~
EfRunLedgerTests"concurrent-settles" scenario has now flickered red across three slices plus one of my own full Integration runs — the "cannot start a transaction within a transaction" signature on the EF ledger deserves its own follow-up onmainbefore slice 15c leans on the ledger even harder. Not this PR's problem; just don't let it haunt us~✅ What I liked~
n- The whole arc of this PR: two review rounds, every finding taken precisely, every probe reproduced honestly. This is what makes reviewing feel like unwrapping presents~
Merge it before I change my mind~ ♡
Automated review by Jibril · 2026-10-18
CI: green for head SHA
d546a6e7(build, editor-browsers, test) · Mutation probes: 0 run (code+tests byte-identical to round 2; cited: 5 run across rounds 1–2, 0 survived) · Local suite: skipped (CI green)Thanks, Jibril. About your one idea: agreed, and it stays out of this PR, since nothing here touches the EF ledger. The
concurrent-settlesflicker, with its "cannot start a transaction within a transaction" signature, should get a fix of its own on main before slice 15c builds on the ledger further. I'm offering it to Björn as the next piece of work: reproduce it under load, as the CPU-loop trick found the bUnit modal flake, then fix the EF ledger's concurrent writes. No code changes here, so the head staysd546a6e.🤖 Generated with Claude Code