Repository navigation
Batch bake is the default again for a gated bake — verified on all three portals - #1238
Merged
Merged
Conversation
…ree portals It was walked back to opt-in (#1217) after its first production run resolved almost no sources. Both causes are fixed and pinned by tests: the global fetch was truncated by a default query limit AND `nodeType:Code` could not see Postgres-stored Code nodes at all (#1216/#1220), and `scope:subtree` was silently dropped for a WILDCARD namespace so nested sources were missed (#1232). Discovery also refuses to answer rather than guess now — a type that declares sources but resolves none fails the whole batch to the activation path, so a discovery bug can no longer wear the non-gating NoSources content verdict. Verified in production (2026-08-11, ci.2982/ci.2990), every sweep with compileErrors=0 timedOut=0 contentBroken=0: memex-cloud 237/237 in 62s (the same fleet took 18m55s on the activation path) memex 83 baked + 1 pending in 6.9s atioz 39/39 in 38s; the follower finished in 1.1s off the shared store The default is what every deployment gets, so it has to be the verified path. Carrying it as three live `kubectl set env` patches meant a helm upgrade, a new namespace or a fresh deployment silently fell back to the 18-minute sweep. PreWarm:BatchBake returns to being the escape hatch, not the enabler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Restores batch bake as the default warm-up mode for readiness-gated deployments now that the previously identified production-scale source-discovery defects have been fixed and verified in production.
Changes:
- Change
PreWarm:BatchBakedefault behavior to follow the readiness gate: default ON for gated pods, default activation-path for ungated pods (still overrideable via config). - Update inline documentation/comments explaining the historical rollback, the fixes that made it safe again, and the production verification.
- Add a What’s New entry announcing the restored default and the expected improvement in update preparation time.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/MeshWeaver.Hosting/DynamicTypePreWarmerHostedService.cs | Restores batch-bake as the default for readiness-gated warm-ups, with config override preserved and expanded rationale in comments. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-11-batch-bake-default-on.md | Adds a What’s New entry describing the restored default and the production-verified speedup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Copilot review finding on the What's New entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
CI round 2 is red on Round 1 of this same branch, carrying the entire code change, was GREEN. The only delta since is one word in the What's New entry ("nearly nineteen" → "nearly nineteen minutes", the Copilot finding above). A doc-only edit turning a green run red on two unrelated tests in two different shards is not this PR. The shard message is Not re-running (the repo's no-rerun-to-green rule). The substantive verification for this PR stands: round 1 green, plus 35/35 locally across the pre-warmer / bake-gate / batch-bake suites and a Release |
rbuergi
added a commit
that referenced
this pull request
Aug 12, 2026
The residual half of the SearchResultStormTest flake was never a baseline problem — it is a
defect in StorageAdapterMeshQueryProvider, and the two committed fixes on this branch (a
quiescence marker, created after the Initial frame) were necessary but could not touch it.
A mesh query emits from two code paths that must agree on what a result is. The snapshot
(RunQueryNodes) excludes satellite-path nodes — they live in their own storage tables, so a
content query never sees them on PG — and partition roots for a bare descendants listing. The
live path's write-lag supplement in ProcessBatch excluded neither: it admitted any notified
entity that passed QueryEvaluator.Matches, and Matches consults only the query's filter and
free-text term, so for `path:X scope:descendants` — no filter, no term — it is true for
EVERYTHING under X.
So a satellite write under the searched root was injected as a structural `Added`, and the very
next change re-queried, correctly excluded it again, and emitted it as `Removed`. Two re-renders
of the whole result list and a row that flashes into a live search grid and vanishes, with
nothing about the node having changed between the two frames — only which code path judged it.
Captured directly from a failing run:
[FRAME] Added structural=True TestData/storm/a/_Activity/write-conflict-12@v1
[FRAME] Removed structural=True TestData/storm/a/_Activity/write-conflict-12@v1
That satellite is written by a MonotonicWriteGuard conflict resolution, which the 25-update
storm provokes intermittently — hence "Expected 2 … but found 4" on roughly 2 runs in 5, and
hence unrelated PRs going red (#1176, #1238, main at 9a05081).
Both exclusions now live in one predicate, IsExcludedFromResults, applied by the snapshot AND by
the supplement, so live can only ever show what the search matched. Duplicating the rules in two
places is how they drifted apart, so the fix is the shared predicate rather than a second copy.
A Deleted notification still removes unconditionally — dropping a path that is gone can never be
wrong, and an excluded path was never in the set.
CatalogSubscription_SatelliteWriteUnderTheRoot_NeverEntersTheResultSet pins it deterministically
and does not depend on provoking a conflict: it writes the satellite straight through storage —
the exact notification the live pipeline consumes — and uses an ordinary node created afterwards
as the ordered proof that the feed processed it. Fail-without on this tree reports "found 2",
the Added/Removed pair above.
What this does NOT fix, filed separately: why a strictly sequential writer provokes that conflict
at all. Every patch-driven write is persisted twice — the post-commit flush (which the ack chains
off) and the per-node persistence sampler, whose 200 ms-sampled value goes stale in the hub inbox
while the row advances — and the two routes are not ordered against each other. No data is lost,
but it fires the guard's "find the writer that adopted a stale own-node snapshot" alarm on a
workload that has no fork, which is exactly what makes a genuine rollback hard to spot.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restores the default that #1217 walked back, now that the defects behind that walk-back are fixed and the path is proven in production.
Why it was turned off, and what changed
Its first production run (memex-cloud,
ci.2947) finished in 19s withcompiled=4 compileErrors=59 contentBroken=169of 237 — batched discovery was resolving almost nothing. Two real defects, both now fixed and pinned:nodeType:Codecould not see Postgres-stored Code nodes at all (Code lives in the per-schemacodesatellite table, routed by path segment, and is deliberately not nodeType-resolvable).scope:subtreewas silently dropped for a WILDCARD namespace, sonamespace:*/Source scope:subtreematched only namespaces ending in/Sourceand missed nested sources such asClaims/SampleData/Source/Fixtures/….Discovery also no longer guesses: a type that declares sources but resolves none fails the whole batch to the activation path instead of emitting the non-gating
NoSourcesverdict — so a discovery bug can no longer disguise itself as content breakage.Production evidence (2026-08-11)
ci.2982ci.2990ci.2990All three with
compileErrors=0 timedOut=0 contentBroken=0, gate armed throughout.Why the default and not just the env var
The default is what every deployment gets. Carrying batch bake as three live
kubectl set envpatches meant a helm upgrade, a new namespace, or a fresh deployment silently fell back to the 18-minute sweep — the failure being invisible is exactly what makes it worth fixing at the default.PreWarm:BatchBakegoes back to being the escape hatch (falseforces the activation sweep on a gated pod), not the enabler.No change is needed in the tracked helm values: they already carry
PreWarm__GateReadiness/PreWarm__DynamicTypes, and batch bake follows the gate. Adding an explicit key there would be redundant and would pin the mode against any future default change.Tests: 35/35 across the pre-warmer, bake-gate and batch-bake suites; Release
-warnaserrorclean. What's New entry included (Category: Feature).🤖 Generated with Claude Code