Repository navigation
Activity log: roll severity and message count into counters (1/3) - #1398
Conversation
An activity's terminal status was a function of its message LIST:
`Finish()` computed it from `Messages.Max(LogLevel)` and `HasErrors()`
scanned the list. Both are O(list) and — the reason this matters — both
silently change meaning the moment `Messages` stops holding the whole
transcript.
Add two incremental roll-ups to `ActivityLog`:
MessageCount — total messages ever appended (read via TotalMessageCount,
which falls back to Messages.Count for logs persisted
before the field existed)
MaxSeverity — highest LogLevel ever appended (default Trace = the
identity element of the max, so a legacy log rolls up to
exactly what a scan of its Messages yields)
`ActivityLog.Append(...)` is now the ONE way to add messages; it advances
both roll-ups in the same step. Every append site in src/ is migrated to
it, including the fresh-log initialisers that used to assign `Messages`
wholesale.
`GetFinalStatus()`/`HasErrors()` now fold the counter together with the
list, so they answer identically before and after the list is trimmed.
Two behaviour corrections fall out, both required for the counter and the
list to agree:
- `LogLevel.None` is excluded from the status fold. It is numerically above
Critical but means "no logging", so one None entry became the list's Max
and dropped the whole activity into the Succeeded arm, swallowing a real
Error logged beside it.
- `HasErrors()` now also reports Critical, matching `Errors()`, which has
always included it.
`RunningActivitiesStripe` keys its `DistinctUntilChanged` on the message
count. Moved to `TotalMessageCount`: keying on the in-list count would
freeze the stripe for exactly the long-running activities it exists to
show, once that list becomes a window.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR is the first step in the activity-satellite migration, refactoring ActivityLog so an activity’s derived state (final status and “has errors”) can be computed from incremental counters (MessageCount, MaxSeverity) rather than scanning the full Messages list—preserving semantics once Messages becomes a bounded window.
Changes:
- Introduces
ActivityLog.MessageCount/MaxSeverityroll-ups and a canonicalActivityLog.Append(...)API to advance them. - Migrates in-repo activity log append sites to
Append(...)and updates the running-activities stripe to key offTotalMessageCount. - Adds a new unit test suite and a “What’s New” entry covering the status fold corrections.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Data.Test/ActivityLogRollupTest.cs | Adds regression tests for the roll-ups and back-compat behavior. |
| src/MeshWeaver.Mesh.Contract/MeshExtensions.cs | Updates activity log construction to use Append(...) so counters advance with message writes. |
| src/MeshWeaver.Hosting/Persistence/MonotonicWriteGuardStorageAdapter.cs | Records write-conflict activities using Append(messages) rather than overwriting Messages. |
| src/MeshWeaver.Graph/RunningActivitiesStripe.cs | Switches UI caption + DistinctUntilChanged key to TotalMessageCount for future windowing. |
| src/MeshWeaver.Graph/Configuration/NodeTypeCompilationActivity.cs | Routes compilation activity message writes through ActivityLog.Append(...). |
| src/MeshWeaver.Graph/Configuration/MeshNodeCompilationService.cs | Refactors helper append methods to use Append(...). |
| src/MeshWeaver.GitSync/ActivityRunner.cs | Updates append/finish logging paths to use Append(...). |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-activity-status-from-counters.md | Documents the behavior change/fix for activity status derivation. |
| src/MeshWeaver.Data/WorkspaceOperations.cs | Uses Append(messages) when creating/finalizing data-update activity logs. |
| src/MeshWeaver.Data/Activity.cs | Updates logger mutation helpers to append via Append(...). |
| src/MeshWeaver.Data.Contract/ActivityLog.cs | Adds the counters + Append(...) and updates status/error derivation logic. |
| src/MeshWeaver.ContentCollections.Indexing.Graph/ContentIndexingActivity.cs | Migrates indexing activity logging to Append(...). |
Suppressed comments (1)
src/MeshWeaver.Data.Contract/ActivityLog.cs:211
HasErrors()claims to report Error or Critical, but for legacy logs (whereMaxSeverityis still the default) it only scansMessagesforLogLevel.Error. A legacy log that contains only a Critical entry will incorrectly return false, and it also diverges fromActivityLogExtensions.Errors()which includes Critical.
public bool HasErrors() =>
MaxSeverity is LogLevel.Error or LogLevel.Critical
|| Messages.Any(m => m.LogLevel == LogLevel.Error);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| public ActivityLog Append(IReadOnlyList<LogMessage> messages) | ||
| { | ||
| if (messages.Count == 0) return this; | ||
| var severity = MaxSeverity; | ||
| foreach (var m in messages) | ||
| if (m.LogLevel > severity && m.LogLevel != LogLevel.None) | ||
| severity = m.LogLevel; | ||
| return this with | ||
| { | ||
| Messages = Messages.AddRange(messages), | ||
| MessageCount = TotalMessageCount + messages.Count, | ||
| MaxSeverity = severity, | ||
| }; | ||
| } |
| [InlineData(LogLevel.Trace, ActivityStatus.Succeeded, false)] | ||
| [InlineData(LogLevel.Information, ActivityStatus.Succeeded, false)] | ||
| [InlineData(LogLevel.Warning, ActivityStatus.Warning, false)] | ||
| [InlineData(LogLevel.Error, ActivityStatus.Failed, true)] | ||
| public void LegacyLog_WithoutRollups_AnswersFromTheListExactlyAsBefore( |
Test Results (shard 0)723 tests 723 ✅ 9m 13s ⏱️ Results for commit b7cae6d. |
Test Results (shard 3) 11 files 11 suites 5m 42s ⏱️ Results for commit b7cae6d. |
Test Results (shard 5)1 906 tests 1 714 ✅ 5m 18s ⏱️ Results for commit b7cae6d. |
Test Results (shard 4)1 949 tests 1 941 ✅ 8m 7s ⏱️ Results for commit b7cae6d. |
Test Results (shard 1)1 419 tests 1 419 ✅ 6m 48s ⏱️ Results for commit b7cae6d. |
Test Results (shard 2)2 438 tests 2 437 ✅ 7m 6s ⏱️ Results for commit b7cae6d. |
Test Results 66 files 66 suites 42m 16s ⏱️ Results for commit b7cae6d. |
Merge order: 1 of 3. Stacked ahead of #1399 and #1400 — merge this one first.
Part of the activity satellite migration (#1284 / #1351). This PR is the prerequisite refactor; the actual window + segments land in 2/3.
The problem
An activity's terminal status was a function of its message list:
Finish()computed it fromMessages.Max(LogLevel)andHasErrors()scanned the list. Both are O(list), and — the reason this blocks the migration — both silently change meaning the momentMessagesstops holding the whole transcript.What this does
Two incremental roll-ups on
ActivityLog:MessageCountTotalMessageCount, which falls back toMessages.Countfor logs persisted before the field existed)MaxSeverityLogLevelever appended (defaultTrace— the identity element of the max, so a legacy log rolls up to exactly what a scan of itsMessagesyields)ActivityLog.Append(...)becomes the one way to add messages and advances both in the same step. Every append site insrc/is migrated to it, including the fresh-log initialisers that assignedMessageswholesale.GetFinalStatus()/HasErrors()fold the counter together with the list, so they answer identically before and after the list is trimmed.Two behaviour corrections fall out
Both are required for the counter and the list to agree — that agreement is what makes the window safe in 2/3:
LogLevel.Noneis excluded from the status fold. It is numerically aboveCriticalbut means "no logging", so oneNoneentry became the list'sMaxand dropped the whole activity into theSucceededarm, swallowing a realErrorlogged beside it. Caught by a new test, not by inspection.HasErrors()now also reportsCritical, matchingErrors(), which has always included it.RunningActivitiesStripe(the landmine this defuses early)The stripe keys its
DistinctUntilChangedon the message count. Moved toTotalMessageCount: keying on the in-list count would freeze the stripe for exactly the long-running activities it exists to show, once that list becomes a window in 2/3.Verification
ActivityLogRollupTest(new, pure, 15 cases) — the roll-ups, theNonerule, the sub-activity fold, and a back-compatibility half asserting a pre-roll-up log (MessageCount = 0,MaxSeverity = Trace) produces byte-identical answers.MeshWeaver.Data.Test344/344 ·MeshWeaver.Graph.Test1095/1095 ·MeshWeaver.GitSync.Test161/161 ·MeshWeaver.AI.Testactivity filter 34/34.-c Release -p:CIRun=true -warnaserror(0 warnings, 0 errors).🤖 Generated with Claude Code