Skip to content

fix(bake): one partition that cannot answer costs the batched discovery its own types, not all of them - #5655

Merged
meshweaver-cloud[bot] merged 4 commits into
mainfrom
fix/batched-source-discovery
Sep 24, 2026
Merged

meshweaver-cloud[bot] merged 4 commits into
mainfrom
fix/batched-source-discovery

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

What

NodeTypeBatchBake.ResolveSources no longer treats one unanswerable partition as a failure of every pending NodeType.

Why (measured)

On memex-cloud, core bf5ac85526, 2026-09-24. These are Logs actions on memex.meshweaver.cloud: Ops/logs-memexcloud-20260924-discovery-limit25000, …-batch-discovery and …-gf5df-42703.

  • On all 8 boots between 07:12Z and 08:06Z, nodeType:Code partitions:all settled at 1586 nodes. About 25 ms later each boot logged batched source discovery did not establish the source sets — … for ALL N pending type(s), with N between 38 and 80.
  • The exception line on pod …6848f8-gf5df came 13 µs after the abandonment at 07:34:15.089Z: Npgsql.PostgresException: 42703: column n.created_by does not exist. The namespace:*/Source fetch UNIONs every schema's code table, and at least one of those tables lacks the V57 authorship columns (Systemorph/MeshWeaver.Plugins#2350).

Test

OnePartitionCostsOnlyItsOwnTypesTest has two cases.

  • The first uses the real mesh plus a test query provider. The provider refuses every mesh-wide nodeType:Code fetch and the anchored reads of one type. The healthy type must keep its real source set, and the broken type must be absent. Negative control: I replaced the fallback with a rethrow, and the test then fails with the provider's 42703 fault, which is the pre-fix behaviour.
  • The second is a pure AssemblePerQuery truth table: a failed query and a contradicted empty set each withhold only their own type.

Related bake/prewarmer suites, run locally in Release: 81/81 passed (ABakeCompilesTheDefinitionItResolvedTest, EmptySourceSetNeedsAWitnessTest, SourceDiscoveryChunkTimingTest, AGatedSweepHearsItsOwnCompileTest, DeclaredSourcesMissingIsNotAnImageVerdictTest, …). MeshWeaver.Hosting and MeshWeaver.Hosting.Test build with -c Release -warnaserror: 0 warnings, 0 errors.

Not established / not done here

  • Which schema's table lacks created_by, and why V57 missed it. That is tracked in Systemorph/MeshWeaver.Plugins#2350. Until it is repaired, the global pass will keep falling back on memex-cloud; with this change the fallback is per-type, not all-or-nothing.
  • memex (the control instance) was not read, because the systemorph MCP is disconnected.

Doc: Doc/Architecture/SourceSetEstablishment, new section "The unit of 'I don't know' is the type, not the batch".

After deploy: nothing needs recycling. This is boot-time pre-warm code, so it takes effect on the next pod start of an image that carries it.

Refs #5654, Refs #5544

Pairs-with: none — no public type or member removed (ResolveSources overloads unchanged; the new members are internal/private)

🤖 Generated with Claude Code

…ry its own types, not all of them

The batched source discovery resolved every pending NodeType from three mesh-wide
fetches — each one UNION over every partition schema on Postgres — and surfaced any
fault as a fault of the WHOLE batch. On memex-cloud (bf5ac85) every boot's
`namespace:*/Source` fetch hit 42703 in one schema's code table, so all 38–80 pending
types took the activation sweep (5–14 min per boot; hours on memex with #5643).

When the global pass cannot be trusted (fault, timeout, ceiling, or a record that
contradicts an empty set), ResolveSources now re-resolves each type from its OWN
anchored queries and withholds only the types whose own queries fail; the sweep
warms exactly those by activation and bakes the rest directly.

Refs #5654, #5544, Systemorph/MeshWeaver.Plugins#2350

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

  1 files  ±0    1 suites  ±0   3m 1s ⏱️ +45s
332 tests ±0  332 ✅ ±0  0 💤 ±0  0 ❌ ±0 
336 runs  ±0  336 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

960 tests  +517   960 ✅ +517   7m 9s ⏱️ + 6m 20s
  1 suites  -   2     0 💤 ±  0 
  1 files    -   2     0 ❌ ±  0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Critical source-completeness and empty-set validation issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity · 2 Medium severity · 3 Low severity

Open (7)
What changed in this PR

This PR isolates batched source-discovery failures to affected NodeTypes instead of abandoning the entire batch.

Changes:

  • Adds per-query fallback discovery and per-type source assembly.
  • Updates prewarming to activate types lacking established sources.
  • Adds integration and truth-table tests.
  • Documents type-level source-establishment semantics.

Unresolved review findings include incomplete-query handling, empty-source validation, cancellation and timeout behavior, and mutable collection usage.

File Summary
test/​MeshWeaver.Hosting.Test/​OnePartitionCostsOnlyItsOwnTypesTest.cs Tests healthy and failed partition behavior.
src/​MeshWeaver.Hosting/​NodeTypeBatchBake.cs Implements per-query fallback and source assembly.
src/​MeshWeaver.Hosting/​DynamicTypePreWarmer.cs Activates types whose sources were not established.
src/​MeshWeaver.Documentation/​Data/​Architecture/​SourceSetEstablishment.md Documents type-level discovery uncertainty.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +351 to +357
return queries
.Select(q => RunQuery(meshService, q, logger, discovery)
.Timeout(DiscoveryBudget)
.Select(nodes => new QueryAnswer(nodes, null))
// Not a swallow: the fault becomes THIS query's recorded answer, and every type that
// reads from it is withheld from the batch and named, with this fault, below.
.Catch<QueryAnswer, Exception>(ex => Observable.Return(new QueryAnswer(null, ex)))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2e23aa7. RunQuery now records SilentProviders on every frame it folds, and it refuses the settled map as a FLOOR (SourceDiscoveryFailedException) when any are named. The fan-in folds SnapshotIncomplete into SilentProviders, so a provider that dropped a partition's rows is covered. This applies to both the global pass and the per-type pass. In the test, the broken type's own read now answers an Initial marked SnapshotIncomplete. With the check disabled, the test fails on 'the type whose own query fails is withheld'.

Comment on lines +406 to +408
if (failure is null
&& DiscoveryUnestablished(matched.Count, pending.DeclaresSources, pending.KnownSourceCount))
failure = "its own queries resolved an EMPTY set that its record contradicts";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change needed here. The predicate already returns true for that state: DiscoveryUnestablished(matchedCount: 0, declaresSources: true, knownSourceCount: null) evaluates as 0 == 0 && null != 0 (true) && (true || null > 0) = true. It is pinned by the existing EmptySourceSetNeedsAWitnessTest.AbsentSnapshot_LeavesThePreExistingRuleUntouched (Assert.True for exactly those arguments). AssemblePerQuery calls the same predicate, so it withholds such a type.

Comment on lines +359 to +360
.Merge(PerQueryConcurrency)
.ToList()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2e23aa7. The per-type pass now computes ONE deadline (now + DiscoveryBudget) when it is subscribed, and every query uses Timeout(deadline). A query still running or not yet started at that deadline fails as itself, so its types alone are withheld, and the pass ends within one budget. Per-query fault isolation is kept.

Comment on lines +148 to +156
private async Task UntilListed(string path, CancellationToken ct)
=> await Observable.Interval(200.Milliseconds()).StartWith(0L)
.SelectMany(_ => MeshService
.Query<MeshNode>(MeshQueryRequest.FromQuery($"path:{path}").AsSystem())
.Take(1))
.Where(change => change.Items.Any(n => n.Path == path))
.FirstAsync()
.Should().Within(TestTimeouts.Convergence)
.Emit($"discovery must be able to see {path}", ct);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept as is, on purpose. What gates discovery is whether the query index LISTS the node, because ResolveSources reads through queries and not through the node stream. A GetMeshNodeStream read would prove the store has it while the index could still trail, and the test would then measure an index lag instead of the fallback. The wait re-issues the query every 200 ms (Interval + SelectMany), so a stale negative is retried rather than trusted. This is the same readiness wait ABakeCompilesTheDefinitionItResolvedTest.UntilListed uses.

Comment thread src/MeshWeaver.Hosting/NodeTypeBatchBake.cs Outdated
Comment thread src/MeshWeaver.Hosting/NodeTypeBatchBake.cs
Comment on lines +96 to +100
var definitions = new Dictionary<string, NodeTypeDefinition?>
{
[GoodType] = new() { Configuration = "config => config" },
[BrokenType] = new() { Configuration = "config => config" },
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2e23aa7: both fixtures (definitions, and answers/perType in the pure test) are now ImmutableDictionary / ImmutableList.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

396 tests   - 1 252   396 ✅  - 1 252   56s ⏱️ - 2m 9s
  1 suites  -     1     0 💤 ±    0 
  1 files    -     1     0 ❌ ±    0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

…ype pass (review)

- RunQuery refuses a frame naming SilentProviders (a provider that did not answer,
  or answered over a partition it dropped — SnapshotIncomplete), so neither pass
  folds a PARTIAL set; the test's broken type now answers incomplete, and a
  negative control (check disabled) fails the test.
- The per-type pass shares ONE DiscoveryBudget deadline instead of one per query.
- Established sets are ImmutableList; test fixtures immutable; log wording.

Refs #5654

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

    2 files   -   1      2 suites   - 1   2m 45s ⏱️ - 3m 2s
1 619 tests  - 391  1 619 ✅  - 391  0 💤 ±0  0 ❌ ±0 
1 619 runs   - 392  1 619 ✅  - 392  0 💤 ±0  0 ❌ ±0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

    1 files   -     2      1 suites   - 2   5m 57s ⏱️ - 1m 7s
1 987 tests +1 245  1 987 ✅ +1 437  0 💤  - 192  0 ❌ ±0 
1 988 runs  +1 246  1 988 ✅ +1 438  0 💤  - 192  0 ❌ ±0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    2 files   -     3      2 suites   - 3   7m 55s ⏱️ - 7m 53s
2 565 tests  - 1 502  2 565 ✅  - 1 500  0 💤  - 2  0 ❌ ±0 
2 569 runs   - 1 502  2 569 ✅  - 1 500  0 💤  - 2  0 ❌ ±0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    8 files   -     9      8 suites   - 9   27m 44s ⏱️ - 7m 7s
7 859 tests  - 1 383  7 859 ✅  - 1 189  0 💤  - 194  0 ❌ ±0 
7 868 runs   - 1 383  7 868 ✅  - 1 189  0 💤  - 194  0 ❌ ±0 

Results for commit b9c36fd. ± Comparison against base commit af2d799.

♻️ This comment has been updated with latest results.

rbuergi and others added 2 commits September 24, 2026 11:12
…unded (MergeBoundedRatchetGuard)

Its inners complete synchronously (.Catch → Return), exactly the shape that makes
a bare Merge(n) recurse per queued inner.

Refs #5654

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants