Skip to content

Commit 48b4e4d

Browse files
committed
style(comments): strip obvious comments and label WHY/TODO/HACK in core Llm+orchestration
Extend the comment convention to the portable-core LLM client, transport, skill tools and orchestrators (13 files): remove restate-the-code comments and section banners, tag genuine rationale with an explicit WHY:/TODO:/HACK: prefix (first line of each block). Comment-only, code verified unchanged. Bulk pass by gpt-5.3-spark agents, reviewed and de-duplicated by hand.
1 parent 8bc0ab1 commit 48b4e4d

13 files changed

Lines changed: 252 additions & 257 deletions

Assets/CoreAI/Runtime/Core/Features/Llm/CallSkillToolLlmTool.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ private sealed class CallSkillToolProxy : LlmToolBase, IAIFunctionLlmTool, ISkil
3333

3434
private readonly IReadOnlyCollection<string> _allowedToolNames;
3535

36-
// When the backing list is a live MutableSkillCatalog (skill authoring), the tool map is
36+
// WHY: When the backing list is a live MutableSkillCatalog (skill authoring), the tool map is
3737
// rebuilt per call so a tool exposed by a just-authored skill is immediately invocable.
3838
private readonly bool _isLive;
3939
private readonly Dictionary<string, SkillToolDescriptor> _toolsByName;
@@ -60,7 +60,7 @@ private Dictionary<string, SkillToolDescriptor> ResolveToolMap()
6060
public override string ParametersSchema =>
6161
"{\"type\":\"object\",\"properties\":{\"tool_name\":{\"type\":\"string\",\"description\":\"Skill tool name returned by read_skill.\"},\"arguments_json\":{\"type\":\"string\",\"description\":\"JSON object string with the skill tool parameters.\"}},\"required\":[\"tool_name\",\"arguments_json\"]}";
6262

63-
// call_skill_tool dispatches to an arbitrary resolved skill tool whose effect the outer
63+
// WHY: call_skill_tool dispatches to an arbitrary resolved skill tool whose effect the outer
6464
// policy cannot see, so it is treated conservatively as mutating: AllowDuplicates=false so
6565
// ToolExecutionPolicy suppresses only a CROSS-TURN byte-identical echo (structured no-op)
6666
// while still allowing intra-turn repeats and never suppressing the retry of a FAILED call.
@@ -116,7 +116,7 @@ private static Dictionary<string, SkillToolDescriptor> BuildToolMap(
116116
continue;
117117
}
118118

119-
// First-registered wins (deterministic, matches the order read_skill enumerates skills).
119+
// WHY: First-registered wins (deterministic, matches the order read_skill enumerates skills).
120120
// Previously this was last-write-wins, so two skills exposing a same-named tool silently
121121
// shadowed each other: read_skill advertised skill A's tool while call_skill_tool ran
122122
// skill B's. Keeping the first and warning makes the collision visible and predictable.
@@ -206,4 +206,4 @@ private static Exception Unwrap(Exception ex)
206206
return ex.InnerException ?? ex;
207207
}
208208
}
209-
}
209+
}

Assets/CoreAI/Runtime/Core/Features/Llm/CircuitBreakerLlmClientDecorator.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@ public async Task<LlmCompletionResult> CompleteAsync(
9696
}
9797
catch (OperationCanceledException)
9898
{
99-
// Cancellation is caller intent, not a backend fault — do not count it, do not swallow it.
99+
// WHY: Cancellation is caller intent, not a backend fault — do not count it, do not swallow it.
100100
throw;
101101
}
102102
catch (Exception ex)
103103
{
104-
// An unexpected throw from the inner client is treated as a transient backend fault.
104+
// WHY: An unexpected throw from the inner client is treated as a transient backend fault.
105105
RecordFailure();
106106
throw new LlmClientException(ex.Message, LlmErrorCode.ProviderError);
107107
}
@@ -136,7 +136,7 @@ public async IAsyncEnumerable<LlmStreamChunk> CompleteStreamingAsync(
136136
while (true)
137137
{
138138
LlmStreamChunk chunk;
139-
// C# forbids `yield` inside a catch, so capture any inner-stream fault here and emit the
139+
// WHY: C# forbids `yield` inside a catch, so capture any inner-stream fault here and emit the
140140
// terminal error chunk AFTER the try/catch instead.
141141
string moveError = null;
142142
try
@@ -185,7 +185,7 @@ public async IAsyncEnumerable<LlmStreamChunk> CompleteStreamingAsync(
185185
await e.DisposeAsync().ConfigureAwait(false);
186186
}
187187

188-
// Classify the whole stream: a stream that produced an error chunk (or nothing at all) is a
188+
// WHY: Classify the whole stream: a stream that produced an error chunk (or nothing at all) is a
189189
// failure; a stream that ended cleanly is a success that closes/keeps-closed the breaker.
190190
bool ok = sawAnyChunk && !sawTerminalFailure;
191191
RecordResult(ok, ok ? LlmErrorCode.None : terminalCode);
@@ -227,7 +227,7 @@ private bool TryEnter(out string rejectReason)
227227
return false;
228228
}
229229

230-
// Closed or HalfOpen: allow through (HalfOpen admits the single probe already in flight).
230+
// WHY: Closed or HalfOpen: allow through (HalfOpen admits the single probe already in flight).
231231
rejectReason = null;
232232
return true;
233233
}
@@ -247,7 +247,7 @@ private void RecordResult(bool ok, LlmErrorCode code)
247247
}
248248
else
249249
{
250-
// A caller-caused failure (auth, invalid request, context length, empty) is not the backend's
250+
// WHY: A caller-caused failure (auth, invalid request, context length, empty) is not the backend's
251251
// health problem — do not trip the breaker, but a half-open probe that returned such a result
252252
// still means the backend is reachable, so treat it as a soft success for state purposes.
253253
RecordSuccess();
@@ -273,7 +273,7 @@ private void RecordFailure()
273273
{
274274
if (_state == State.HalfOpen)
275275
{
276-
// The probe failed — re-open for another cooldown.
276+
// WHY: The probe failed — re-open for another cooldown.
277277
_state = State.Open;
278278
_openedAtMs = _nowMs();
279279
_log?.Invoke("[CircuitBreaker] re-opened: half-open probe failed.");
@@ -311,4 +311,4 @@ private static bool IsTransientFailure(LlmErrorCode code)
311311
}
312312
}
313313
}
314-
#endif
314+
#endif

Assets/CoreAI/Runtime/Core/Features/Llm/HttpClientOpenAiTransport.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public sealed class HttpClientOpenAiTransport : IOpenAiHttpTransport
3838

3939
private static HttpClient CreateSharedHttpClient()
4040
{
41-
// HttpClientHandler is available on .NET Standard 2.0 (Unity's default Mono/IL2CPP profile).
41+
// WHY: HttpClientHandler is available on .NET Standard 2.0 (Unity's default Mono/IL2CPP profile).
4242
// Bypass any system/WinINET proxy: Mono's HttpClient uses the system proxy by default, which
4343
// can route even 127.0.0.1 / localhost requests through a proxy or VPN filter driver. A local
4444
// LLM endpoint must never go through a proxy. (SocketsHttpHandler is not exposed by Unity's
@@ -54,7 +54,7 @@ private static HttpClient CreateSharedHttpClient()
5454
}
5555
catch
5656
{
57-
/* some profiles may not support the setter; fall back to default proxy behavior */
57+
// WHY: some profiles may not support the setter; fall back to default proxy behavior
5858
}
5959

6060
return new HttpClient(handler) { Timeout = Timeout.InfiniteTimeSpan };
@@ -188,4 +188,4 @@ private static HttpClient GetStreamingHttpClient()
188188
}
189189
}
190190
}
191-
#endif
191+
#endif

0 commit comments

Comments
 (0)