Skip to content

Commit 6bf2d2b

Browse files
loningclaude
andcommitted
fix-codex round-1: address PR #707 reviewer comments (cluster-026)
Applied 5 fixes (FIX_DONE:707:round-1:applied-5:rejected-0:blocked-0): - (A) architect: add method-level Refactor (iter15/cluster-026) markers on VoicePresenceModule actor-turn provider event normalization, response identity normalization, provider response mapping, retirement - (A) architect: add method-level Refactor marker on OpenAI provider MapSessionEvent (emits provider-native response ids) - (A) architect: add method-level Refactor markers on MiniCPM provider CancelResponseAsync + ReadCompletionStreamAsync - (A) quality: add why-comment for intentional remote audio input no-op until raw host-origin transport path exists - (A) tests: add source-regression test asserting provider adapters no longer own response epoch/counter state; module owns provider response mapping Build pass; VoicePresence.Tests 120/120; OpenAI.Tests 24/24; MiniCPM.Tests 12/12; test_stability_guards pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c1e12b6 commit 6bf2d2b

4 files changed

Lines changed: 73 additions & 0 deletions

File tree

src/Aevatar.Foundation.VoicePresence.MiniCPM/MiniCPMRealtimeProvider.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ public Task InjectEventAsync(VoiceConversationEventInjection injection, Cancella
137137
"MiniCPM-o demo protocol does not support structured external event injection.");
138138
}
139139

140+
// Refactor (iter15/cluster-026-voice-provider-background-state):
141+
// Old pattern: cancel mutated active/suppressed response epoch state and emitted synthetic cancel events.
142+
// New principle: cancel only sends provider stop; VoicePresenceModule owns actor cancellation state.
140143
public async Task CancelResponseAsync(CancellationToken ct)
141144
{
142145
using var message = new HttpRequestMessage(HttpMethod.Post, BuildUri(_options.StopPath));
@@ -251,6 +254,9 @@ private async Task RunCompletionsLoopAsync(ChannelWriter<VoiceProviderEvent> wri
251254
}
252255
}
253256

257+
// Refactor (iter15/cluster-026-voice-provider-background-state):
258+
// Old pattern: completion stream allocated actor response ids and suppressed stale audio in provider state.
259+
// New principle: stream parsing emits provider-native response ids; module actor turn maps and suppresses.
254260
private async Task ReadCompletionStreamAsync(
255261
Stream stream,
256262
ChannelWriter<VoiceProviderEvent> writer,

src/Aevatar.Foundation.VoicePresence.OpenAI/OpenAIRealtimeProvider.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,9 @@ private async Task RunDispatchLoopAsync(ChannelReader<VoiceProviderEvent> reader
214214
}
215215
}
216216

217+
// Refactor (iter15/cluster-026-voice-provider-background-state):
218+
// Old pattern: MapSessionEvent allocated actor response epochs and mutated provider-owned response dictionaries.
219+
// New principle: OpenAI events carry provider-native response ids; VoicePresenceModule maps them in actor turn.
217220
private VoiceProviderEvent? MapSessionEvent(OpenAIRealtimeSessionEvent sessionEvent) =>
218221
sessionEvent switch
219222
{

src/Aevatar.Foundation.VoicePresence/Modules/VoicePresenceModule.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ private async Task HandleModuleSignalAsync(
151151
await HandleRemoteSessionCloseRequestedAsync(signal.RemoteSessionCloseRequested, ctx, ct);
152152
break;
153153
case VoiceModuleSignal.SignalOneofCase.RemoteAudioInputReceived:
154+
// Remote PCM stays off EventEnvelope until a raw transport path exists for host-origin audio.
154155
break;
155156
case VoiceModuleSignal.SignalOneofCase.RemoteControlInputReceived:
156157
await HandleRemoteControlInputReceivedAsync(signal.RemoteControlInputReceived, ct);
@@ -334,6 +335,9 @@ private async Task StopRelayAsync()
334335

335336
// ── State machine dispatch (used by both event pipeline and relay) ──
336337

338+
// Refactor (iter15/cluster-026-voice-provider-background-state):
339+
// Old pattern: provider callbacks arrived with actor response epochs already assigned in background loops.
340+
// New principle: provider events are normalized to actor response ids inside this actor turn.
337341
internal async Task HandleProviderEventAsync(
338342
VoiceProviderEvent providerEvent,
339343
IEventHandlerContext ctx,
@@ -402,6 +406,9 @@ internal async Task HandleProviderEventAsync(
402406
}
403407
}
404408

409+
// Refactor (iter15/cluster-026-voice-provider-background-state):
410+
// Old pattern: provider-specific receive loops suppressed and completed response epochs directly.
411+
// New principle: this actor-turn normalizer owns cancellation suppression and response-id materialization.
405412
private bool TryNormalizeProviderEvent(
406413
VoiceProviderEvent providerEvent,
407414
out VoiceProviderEvent normalizedEvent)
@@ -482,6 +489,9 @@ private bool TryNormalizeProviderEvent(
482489
}
483490
}
484491

492+
// Refactor (iter15/cluster-026-voice-provider-background-state):
493+
// Old pattern: providers allocated fallback response epochs when provider ids were missing.
494+
// New principle: fallback actor response ids are allocated only by the module state machine turn.
485495
private bool TryNormalizeResponseIdentity(string providerResponseId, int suppliedResponseId, out int responseId)
486496
{
487497
if (!string.IsNullOrWhiteSpace(providerResponseId))
@@ -506,6 +516,9 @@ private bool TryNormalizeResponseIdentity(string providerResponseId, int supplie
506516
return true;
507517
}
508518

519+
// Refactor (iter15/cluster-026-voice-provider-background-state):
520+
// Old pattern: OpenAI/MiniCPM adapters owned provider-id to actor-epoch dictionaries and counters.
521+
// New principle: provider-id to actor response-id mapping is actor runtime state owned by this module.
509522
private int GetOrCreateProviderResponse(string providerResponseId, int suppliedResponseId)
510523
{
511524
if (_providerResponseIds.TryGetValue(providerResponseId, out var existing))
@@ -520,6 +533,9 @@ private int GetOrCreateProviderResponse(string providerResponseId, int suppliedR
520533
private bool TryGetProviderResponse(string providerResponseId, out int responseId) =>
521534
_providerResponseIds.TryGetValue(providerResponseId, out responseId);
522535

536+
// Refactor (iter15/cluster-026-voice-provider-background-state):
537+
// Old pattern: providers retired response epochs from background completion/cancel callbacks.
538+
// New principle: the actor turn retires provider response mappings when committed lifecycle events arrive.
523539
private void RetireProviderResponse(string providerResponseId)
524540
{
525541
if (string.IsNullOrWhiteSpace(providerResponseId))

test/Aevatar.Foundation.VoicePresence.Tests/VoicePresenceModuleTests.cs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,37 @@ await module.HandleAsync(CreateEnvelope(new VoiceProviderEvent
391391
module.StateMachine.State.ShouldBe(VoicePresenceState.UserSpeaking);
392392
}
393393

394+
[Fact]
395+
public void Provider_adapters_should_not_own_response_epoch_state()
396+
{
397+
var repoRoot = FindRepositoryRoot();
398+
var providerSources = new[]
399+
{
400+
Path.Combine(repoRoot, "src/Aevatar.Foundation.VoicePresence.OpenAI/OpenAIRealtimeProvider.cs"),
401+
Path.Combine(repoRoot, "src/Aevatar.Foundation.VoicePresence.MiniCPM/MiniCPMRealtimeProvider.cs"),
402+
};
403+
var forbiddenTokens = new[]
404+
{
405+
"_responseEpochs",
406+
"_nextResponseId",
407+
"_activeResponseId",
408+
"_suppressedResponseId",
409+
"Interlocked.Increment",
410+
};
411+
412+
foreach (var sourcePath in providerSources)
413+
{
414+
File.Exists(sourcePath).ShouldBeTrue(sourcePath);
415+
var source = StripLineComments(File.ReadAllLines(sourcePath));
416+
foreach (var token in forbiddenTokens)
417+
source.ShouldNotContain(token, Case.Sensitive, $"{Path.GetFileName(sourcePath)} must emit provider-native ids only");
418+
}
419+
420+
var moduleSourcePath = Path.Combine(repoRoot, "src/Aevatar.Foundation.VoicePresence/Modules/VoicePresenceModule.cs");
421+
var moduleSource = File.ReadAllText(moduleSourcePath);
422+
moduleSource.ShouldContain("_providerResponseIds");
423+
}
424+
394425
[Fact]
395426
public async Task Remote_session_signals_should_not_forward_audio_or_publish_audio_outputs()
396427
{
@@ -899,6 +930,23 @@ private static EventEnvelope CreateEnvelope(IMessage payload)
899930
};
900931
}
901932

933+
private static string FindRepositoryRoot()
934+
{
935+
var current = new DirectoryInfo(AppContext.BaseDirectory);
936+
while (current != null)
937+
{
938+
if (File.Exists(Path.Combine(current.FullName, "aevatar.slnx")))
939+
return current.FullName;
940+
941+
current = current.Parent;
942+
}
943+
944+
throw new DirectoryNotFoundException("Could not find repository root containing aevatar.slnx.");
945+
}
946+
947+
private static string StripLineComments(IEnumerable<string> lines) =>
948+
string.Join(Environment.NewLine, lines.Where(static line => !line.TrimStart().StartsWith("//", StringComparison.Ordinal)));
949+
902950
private sealed class RecordingVoiceProvider : IRealtimeVoiceProvider
903951
{
904952
public int ConnectCalls { get; private set; }

0 commit comments

Comments
 (0)