fix: actually update the last heartbeat start #101

2026-05-23T12:01:50-04:00

jasoncouture commented

2026-05-23 12:01:50 -04:00

(Migrated from github.com)

Summary

Original fix on this branch (239d96e) called Interlocked.Exchange<DateTimeOffset> which compiles but throws NotSupportedException at runtime — Interlocked.Exchange<T> only supports ref types, primitives, and enums. Replaced with plain read/write of _lastHeartbeatStart; the tick handler is the sole writer and is never concurrent, so no interlock is needed.
Added AgentHeartbeatServiceTests.SecondHeartbeatBriefsOnlyParentTurnsSincePreviousHeartbeat — fires two heartbeats with a lifecycle-stopping event in between and asserts the second briefing only includes parent turns added after the first heartbeat fired. Fails on the broken interlock form, passes on the plain assignment.
Drive-by: TransientAgentTests.AssistantTurnIsForwardedToParentOnCompletion was already failing on main after refactor 9126cde switched ChannelMessage.ChannelId to the subagent: prefix. Updated the assertion to match.

Test plan

dotnet test — all 550 pass.

## Summary - Original fix on this branch (`239d96e`) called `Interlocked.Exchange<DateTimeOffset>` which compiles but throws `NotSupportedException` at runtime — `Interlocked.Exchange<T>` only supports ref types, primitives, and enums. Replaced with plain read/write of `_lastHeartbeatStart`; the tick handler is the sole writer and is never concurrent, so no interlock is needed. - Added `AgentHeartbeatServiceTests.SecondHeartbeatBriefsOnlyParentTurnsSincePreviousHeartbeat` — fires two heartbeats with a lifecycle-stopping event in between and asserts the second briefing only includes parent turns added after the first heartbeat fired. Fails on the broken interlock form, passes on the plain assignment. - Drive-by: `TransientAgentTests.AssistantTurnIsForwardedToParentOnCompletion` was already failing on `main` after refactor `9126cde` switched `ChannelMessage.ChannelId` to the `subagent:` prefix. Updated the assertion to match. ## Test plan - [x] `dotnet test` — all 550 pass.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-05-23 12:02:59 -04:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates heartbeat summarization to only include parent turns since the previous heartbeat start, adds unit test coverage for that behavior, and aligns a transient agent test expectation with the new subagent channel-id format.

Changes:

Fix parent-turn selection window in AgentHeartbeatService by snapshotting the previous heartbeat start time before updating it.
Add AgentHeartbeatServiceTests to verify the second heartbeat only briefs newly appended parent turns.
Update TransientAgentTests to expect subagent:-prefixed channel IDs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
tests/LlamaShears.UnitTests/Agent/Core/TransientAgentTests.cs	Updates expected channel ID formatting for subagent messages.
tests/LlamaShears.UnitTests/Agent/Core/AgentHeartbeatServiceTests.cs	Adds regression test ensuring heartbeats brief only new parent turns since the previous heartbeat.
src/LlamaShears.Core/AgentHeartbeatService.cs	Fixes heartbeat turn filtering by using the prior heartbeat start timestamp.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview > [!NOTE] > Copilot was unable to run its full agentic suite in this review. Updates heartbeat summarization to only include parent turns since the previous heartbeat start, adds unit test coverage for that behavior, and aligns a transient agent test expectation with the new subagent channel-id format. **Changes:** - Fix parent-turn selection window in `AgentHeartbeatService` by snapshotting the previous heartbeat start time before updating it. - Add `AgentHeartbeatServiceTests` to verify the second heartbeat only briefs newly appended parent turns. - Update `TransientAgentTests` to expect `subagent:`-prefixed channel IDs. ### Reviewed changes Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments. | File | Description | | ---- | ----------- | | tests/LlamaShears.UnitTests/Agent/Core/TransientAgentTests.cs | Updates expected channel ID formatting for subagent messages. | | tests/LlamaShears.UnitTests/Agent/Core/AgentHeartbeatServiceTests.cs | Adds regression test ensuring heartbeats brief only new parent turns since the previous heartbeat. | | src/LlamaShears.Core/AgentHeartbeatService.cs | Fixes heartbeat turn filtering by using the prior heartbeat start timestamp. | --- 💡 <a href="/jasoncouture/llama-shears/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

src/LlamaShears.Core/AgentHeartbeatService.cs

					
				@ -103,2 +103,4 @@

				            cancellationToken);

				        var heartbeatContextStorage = await _contextStore.OpenAsync(handle.SessionPath.Current, cancellationToken);

				        var lastHeartbeatStartedAt = _lastHeartbeatStart;

				        _lastHeartbeatStart = _timeProvider.GetLocalNow();

Updating _lastHeartbeatStart using _timeProvider.GetLocalNow() inside the handler can diverge from the tick time represented by the event envelope (and from the timestamps used elsewhere), which can cause missed or duplicated turns if the TimeProvider's notion of 'now' differs from the tick's at value. Prefer setting _lastHeartbeatStart based on the tick timestamp from envelope.Data (e.g., the SystemTick time) so the selection window is driven by the event time rather than wall-clock-at-handle-time.

Updating `_lastHeartbeatStart` using `_timeProvider.GetLocalNow()` inside the handler can diverge from the tick time represented by the event envelope (and from the timestamps used elsewhere), which can cause missed or duplicated turns if the TimeProvider's notion of 'now' differs from the tick's `at` value. Prefer setting `_lastHeartbeatStart` based on the tick timestamp from `envelope.Data` (e.g., the `SystemTick` time) so the selection window is driven by the event time rather than wall-clock-at-handle-time.

tests/LlamaShears.UnitTests/Agent/Core/TransientAgentTests.cs

					
				@ -152,3 +152,3 @@

				            Arg.Is<ChannelMessage>(m => m.Text == "final answer"

				                && m.ChannelId == _path.Current.ToString()),

				                && m.ChannelId == $"subagent:{_path.Current}"),

				            Arg.Any<Guid>(),

The \"subagent:\" prefix is a protocol/format detail that is now duplicated in the test. To reduce brittleness if the channel-id scheme changes again, prefer building the expected channel ID via a shared helper/constant used by production code (or a dedicated test helper), rather than hardcoding the prefix in-line.

The `\"subagent:\"` prefix is a protocol/format detail that is now duplicated in the test. To reduce brittleness if the channel-id scheme changes again, prefer building the expected channel ID via a shared helper/constant used by production code (or a dedicated test helper), rather than hardcoding the prefix in-line.

github-actions[bot] commented

2026-05-23 12:03:33 -04:00

(Migrated from github.com)

Package	Line Rate	Branch Rate	Complexity	Health
LlamaShears.Core.Eventing	89%	84%	58	✔
LlamaShears.Core.Abstractions	55%	36%	463	➖
LlamaShears.Api	30%	30%	473	❌
LlamaShears.Provider.Ollama	44%	28%	188	❌
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
LlamaShears.Core	46%	43%	1640	❌
LlamaShears.Provider.Onnx.Embeddings	37%	38%	72	❌
LlamaShears.Provider.OpenAI	66%	65%	229	➖
LlamaShears.Api.Web	1%	1%	493	❌
LlamaShears.Hosting	33%	21%	27	❌
LlamaShears.Plugins	0%	100%	1	❌
LlamaShears.Core.Eventing	85%	74%	58	✔
LlamaShears	51%	33%	60	➖
LlamaShears.Plugins.Host	34%	24%	36	❌
LlamaShears.Core.Abstractions	45%	24%	463	❌
LlamaShears.Api	6%	2%	473	❌
LlamaShears.Provider.Ollama	3%	1%	188	❌
LlamaShears.IntegrationTests.Fixtures	73%	57%	64	➖
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
StrangeSoft.Plugins.Host	20%	21%	87	❌
LlamaShears.Core	42%	28%	1640	❌
LlamaShears.Provider.Onnx.Embeddings	3%	0%	72	❌
LlamaShears.Provider.OpenAI	2%	0%	229	❌
LlamaShears.Api.Web	18%	7%	493	❌
LlamaShears.Hosting	26%	8%	27	❌
LlamaShears.Plugins	0%	100%	1	❌
LlamaShears.Core.Eventing	83%	70%	58	✔
LlamaShears	51%	33%	60	➖
LlamaShears.Plugins.Host	34%	24%	36	❌
LlamaShears.Core.Abstractions	35%	21%	463	❌
LlamaShears.Api	7%	3%	473	❌
LlamaShears.Provider.Ollama	3%	1%	188	❌
LlamaShears.IntegrationTests.Fixtures	70%	52%	64	➖
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
StrangeSoft.Plugins.Host	20%	21%	87	❌
LlamaShears.Core	30%	18%	1640	❌
LlamaShears.Provider.Onnx.Embeddings	3%	0%	72	❌
LlamaShears.Provider.OpenAI	2%	0%	229	❌
LlamaShears.Api.Web	17%	6%	493	❌
LlamaShears.Hosting	26%	8%	27	❌
LlamaShears.Analyzers.CodeFixes	85%	69%	60	✔
LlamaShears.Analyzers	88%	76%	199	✔
Summary	46% (11906 / 36006)	36% (2566 / 10743)	11687	❌

Package | Line Rate | Branch Rate | Complexity | Health -------- | --------- | ----------- | ---------- | ------ LlamaShears.Core.Eventing | 89% | 84% | 58 | ✔ LlamaShears.Core.Abstractions | 55% | 36% | 463 | ➖ LlamaShears.Api | 30% | 30% | 473 | ❌ LlamaShears.Provider.Ollama | 44% | 28% | 188 | ❌ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ LlamaShears.Core | 46% | 43% | 1640 | ❌ LlamaShears.Provider.Onnx.Embeddings | 37% | 38% | 72 | ❌ LlamaShears.Provider.OpenAI | 66% | 65% | 229 | ➖ LlamaShears.Api.Web | 1% | 1% | 493 | ❌ LlamaShears.Hosting | 33% | 21% | 27 | ❌ LlamaShears.Plugins | 0% | 100% | 1 | ❌ LlamaShears.Core.Eventing | 85% | 74% | 58 | ✔ LlamaShears | 51% | 33% | 60 | ➖ LlamaShears.Plugins.Host | 34% | 24% | 36 | ❌ LlamaShears.Core.Abstractions | 45% | 24% | 463 | ❌ LlamaShears.Api | 6% | 2% | 473 | ❌ LlamaShears.Provider.Ollama | 3% | 1% | 188 | ❌ LlamaShears.IntegrationTests.Fixtures | 73% | 57% | 64 | ➖ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ StrangeSoft.Plugins.Host | 20% | 21% | 87 | ❌ LlamaShears.Core | 42% | 28% | 1640 | ❌ LlamaShears.Provider.Onnx.Embeddings | 3% | 0% | 72 | ❌ LlamaShears.Provider.OpenAI | 2% | 0% | 229 | ❌ LlamaShears.Api.Web | 18% | 7% | 493 | ❌ LlamaShears.Hosting | 26% | 8% | 27 | ❌ LlamaShears.Plugins | 0% | 100% | 1 | ❌ LlamaShears.Core.Eventing | 83% | 70% | 58 | ✔ LlamaShears | 51% | 33% | 60 | ➖ LlamaShears.Plugins.Host | 34% | 24% | 36 | ❌ LlamaShears.Core.Abstractions | 35% | 21% | 463 | ❌ LlamaShears.Api | 7% | 3% | 473 | ❌ LlamaShears.Provider.Ollama | 3% | 1% | 188 | ❌ LlamaShears.IntegrationTests.Fixtures | 70% | 52% | 64 | ➖ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ StrangeSoft.Plugins.Host | 20% | 21% | 87 | ❌ LlamaShears.Core | 30% | 18% | 1640 | ❌ LlamaShears.Provider.Onnx.Embeddings | 3% | 0% | 72 | ❌ LlamaShears.Provider.OpenAI | 2% | 0% | 229 | ❌ LlamaShears.Api.Web | 17% | 6% | 493 | ❌ LlamaShears.Hosting | 26% | 8% | 27 | ❌ LlamaShears.Analyzers.CodeFixes | 85% | 69% | 60 | ✔ LlamaShears.Analyzers | 88% | 76% | 199 | ✔ **Summary** | **46%** (11906 / 36006) | **36%** (2566 / 10743) | **11687** | ❌

Sign in to join this conversation.

No reviewers