[dotnet-code] Clarify completion marker feedback internals - #830
[dotnet-code] Clarify completion marker feedback internals#830Michelle Clayton (michelle-clayton-work) wants to merge 1 commit into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the loop completion-marker evaluator internals by extracting unexported helpers that (1) prepare the feedback template by substituting the completion marker once at construction time and (2) format per-evaluation feedback by substituting the latest response text. This aligns the Go evaluator structure more closely with the referenced .NET constructor/evaluation split, without changing the public API or intended behavior.
Changes:
- Replace inline completion-marker placeholder substitution in
NewCompletionMarkerEvaluatorwithprepareCompletionMarkerFeedbackTemplate(...). - Replace inline per-evaluation last-response substitution in
EvaluatewithformatCompletionMarkerFeedback(...).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Parity Review: ✅ No IssuesThis PR only extracts unexported helper functions within Scope: Internal refactor aligning Go evaluator structure with the .NET Cross-repo parity: The change mirrors the .NET pattern correctly. No Python or .NET public surfaces are affected. The
|
|
Copilot fix linter failures |
The failure was a transient |
Summary
Extracted unexported helpers for preparing completion-marker feedback templates and formatting per-response feedback. This keeps the Go evaluator structure closer to the .NET constructor/evaluation split, where the completion marker is substituted once and the latest response placeholder is substituted during evaluation.
.NET Reference
dotnet/src/Microsoft.Agents.AI/Harness/Loop/CompletionMarkerLoopEvaluator.cs- separates fixed completion marker template preparation from per-evaluation last response substitution.Public API and Behavior
No public Go API changed. No intentional behavior change was made.
Tests
go test ./agent/harness/loopNotes
Rejected candidates from the random sample:
dotnet/src/Microsoft.Agents.AI.Workflows/InProcessExecution.cs/ Goworkflow/inprocexecution entry points: Go is already structurally different due to package-level environment types and changing it would risk public API churn.dotnet/src/Microsoft.Agents.AI.Workflows/Observability/Tags.cs/ Go workflow observability constants: tags are already centralized in Go and no safe cleanup was apparent.dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsSource.cs/ Goagent/skillssources: the .NET disposable base-class shape does not translate cleanly to Go without changing source semantics or public API.An open
[dotnet-code]PR exists but its details were filtered in this environment, so this PR intentionally stays limited to loop completion-marker internals.Closes #815