SQL 리뷰 계층과 AgentLoop 응답 분해 helper를 추가해 코드 탭 마감 품질을 높임
- SqlReviewService를 추가해 SQL fallback 결과에 review severity, key findings, review checklist를 붙이고 schema migration, seed/reference data, reporting query마다 다른 검토 포인트를 안내하도록 확장했습니다. - SqlAnalysisService와 CodeLanguageCatalog를 업데이트해 SQL fallback summary와 workflow summary가 rollback notes, dependency order, row-count guard 같은 리뷰 힌트를 직접 포함하도록 보강했습니다. - AgentLoopResponseClassificationService를 추가해 LLM 응답에서 text/tool_use 분리, no-tool 연속 카운트 계산, thinking summary 생성을 helper로 분리했고 AgentLoopService 본체는 해당 helper를 사용하도록 정리했습니다. - README, docs/DEVELOPMENT.md, docs/NEXT_ROADMAP.md에 2026-04-15 11:50 (KST) 기준 이력을 반영했습니다. 검증 결과 - dotnet build src/AxCopilot/AxCopilot.csproj -c Release -v minimal -p:OutputPath=bin\\verify_loop_sql_finalize\\ -p:IntermediateOutputPath=obj\\verify_loop_sql_finalize\\ : 경고 0 / 오류 0 - dotnet test src/AxCopilot.Tests/AxCopilot.Tests.csproj -c Release -v minimal --filter "AgentLoopResponseClassificationServiceTests|AgentLoopLlmRequestPreparationServiceTests|AgentLoopIterationPreparationServiceTests|SqlAnalysisServiceTests|SqlReviewServiceTests|CodeLanguageCatalogTests|WorkspaceContextGeneratorTests" -p:OutputPath=bin\\verify_loop_sql_finalize_tests\\ -p:IntermediateOutputPath=obj\\verify_loop_sql_finalize_tests\\ : 통과 48
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
using AxCopilot.Services;
|
||||
using AxCopilot.Services.Agent;
|
||||
using FluentAssertions;
|
||||
using Xunit;
|
||||
|
||||
namespace AxCopilot.Tests.Services;
|
||||
|
||||
public class AgentLoopResponseClassificationServiceTests
|
||||
{
|
||||
[Fact]
|
||||
public void Classify_ShouldSplitTextAndToolCalls_AndResetNoToolCounter()
|
||||
{
|
||||
var blocks = new List<ContentBlock>
|
||||
{
|
||||
new() { Type = "text", Text = "I will inspect the repository." },
|
||||
new() { Type = "tool_use", ToolName = "file_read", ToolId = "tool-1" }
|
||||
};
|
||||
|
||||
var result = AgentLoopResponseClassificationService.Classify(blocks, consecutiveNoToolResponses: 2);
|
||||
|
||||
result.TextResponse.Should().Contain("inspect the repository");
|
||||
result.ToolCalls.Should().HaveCount(1);
|
||||
result.NextConsecutiveNoToolResponses.Should().Be(0);
|
||||
result.BuildThinkingSummary().Should().Contain("inspect the repository");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Classify_ShouldIncrementNoToolCounter_WhenOnlyTextBlocksArePresent()
|
||||
{
|
||||
var blocks = new List<ContentBlock>
|
||||
{
|
||||
new() { Type = "text", Text = "Plan step one." },
|
||||
new() { Type = "text", Text = "Plan step two." }
|
||||
};
|
||||
|
||||
var result = AgentLoopResponseClassificationService.Classify(blocks, consecutiveNoToolResponses: 1);
|
||||
|
||||
result.ToolCalls.Should().BeEmpty();
|
||||
result.TextParts.Should().HaveCount(2);
|
||||
result.NextConsecutiveNoToolResponses.Should().Be(2);
|
||||
}
|
||||
}
|
||||
@@ -105,5 +105,6 @@ public class CodeLanguageCatalogTests
|
||||
summary.Should().Contain("dialect");
|
||||
summary.Should().Contain("migration order");
|
||||
summary.Should().Contain("dependencies");
|
||||
summary.Should().Contain("rollback");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,6 +54,8 @@ public class SqlAnalysisServiceTests
|
||||
summary.Should().Contain("users");
|
||||
summary.Should().Contain("script:");
|
||||
summary.Should().Contain("review focus:");
|
||||
summary.Should().Contain("review severity:");
|
||||
summary.Should().Contain("review checklist:");
|
||||
}
|
||||
finally
|
||||
{
|
||||
|
||||
50
src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs
Normal file
50
src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs
Normal file
@@ -0,0 +1,50 @@
|
||||
using AxCopilot.Services;
|
||||
using FluentAssertions;
|
||||
using Xunit;
|
||||
|
||||
namespace AxCopilot.Tests.Services;
|
||||
|
||||
public class SqlReviewServiceTests
|
||||
{
|
||||
[Fact]
|
||||
public void Review_ShouldMarkDestructiveMigrationAsHighSeverity()
|
||||
{
|
||||
var report = new SqlAnalysisReport(
|
||||
"PostgreSQL",
|
||||
"schema migration",
|
||||
["ALTER TABLE", "DROP TABLE"],
|
||||
["orders", "legacy_orders"],
|
||||
["customer_dim"],
|
||||
["Destructive DROP statement is present."],
|
||||
["Run the script in a disposable database and confirm rollback strategy first."],
|
||||
["Review forward migration order and any missing rollback note."]);
|
||||
|
||||
var review = SqlReviewService.Review(report);
|
||||
|
||||
review.Severity.Should().Be("high");
|
||||
review.Findings.Should().Contain(f => f.Contains("schema change", StringComparison.OrdinalIgnoreCase)
|
||||
|| f.Contains("rollback", StringComparison.OrdinalIgnoreCase));
|
||||
review.Checklist.Should().Contain(c => c.Contains("rollback", StringComparison.OrdinalIgnoreCase));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Review_ShouldRecommendIdempotencyChecksForSeedScript()
|
||||
{
|
||||
var report = new SqlAnalysisReport(
|
||||
"Generic SQL",
|
||||
"seed / reference data",
|
||||
["INSERT", "UPDATE"],
|
||||
["seed_status", "users"],
|
||||
["account_status_map"],
|
||||
["Explicit transaction boundaries are not visible in the script."],
|
||||
["Confirm the script is safe to rerun and that seed data remains idempotent."],
|
||||
["Verify the script is idempotent before rerunning seed data."]);
|
||||
|
||||
var review = SqlReviewService.Review(report);
|
||||
|
||||
review.Severity.Should().Be("medium");
|
||||
review.Findings.Should().Contain(f => f.Contains("idempotent", StringComparison.OrdinalIgnoreCase));
|
||||
review.Checklist.Should().Contain(c => c.Contains("rerun", StringComparison.OrdinalIgnoreCase)
|
||||
|| c.Contains("idempotent", StringComparison.OrdinalIgnoreCase));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,52 @@
|
||||
using AxCopilot.Services;
|
||||
|
||||
namespace AxCopilot.Services.Agent;
|
||||
|
||||
internal sealed record AgentLoopResponseClassificationResult(
|
||||
string TextResponse,
|
||||
IReadOnlyList<string> TextParts,
|
||||
List<ContentBlock> ToolCalls,
|
||||
int NextConsecutiveNoToolResponses)
|
||||
{
|
||||
public string BuildThinkingSummary(int maxLength = 150)
|
||||
{
|
||||
if (string.IsNullOrEmpty(TextResponse))
|
||||
return string.Empty;
|
||||
|
||||
return TextResponse.Length > maxLength
|
||||
? TextResponse[..maxLength] + "…"
|
||||
: TextResponse;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// LLM 응답 블록을 텍스트와 tool_use로 분리하고, 무도구 응답 연속 횟수를 계산한다.
|
||||
/// </summary>
|
||||
internal static class AgentLoopResponseClassificationService
|
||||
{
|
||||
public static AgentLoopResponseClassificationResult Classify(
|
||||
IReadOnlyList<ContentBlock> blocks,
|
||||
int consecutiveNoToolResponses)
|
||||
{
|
||||
var textParts = new List<string>();
|
||||
var toolCalls = new List<ContentBlock>();
|
||||
|
||||
foreach (var block in blocks)
|
||||
{
|
||||
if (block.Type == "text" && !string.IsNullOrWhiteSpace(block.Text))
|
||||
textParts.Add(block.Text);
|
||||
else if (block.Type == "tool_use")
|
||||
toolCalls.Add(block);
|
||||
}
|
||||
|
||||
var nextConsecutiveNoToolResponses = toolCalls.Count == 0
|
||||
? consecutiveNoToolResponses + 1
|
||||
: 0;
|
||||
|
||||
return new AgentLoopResponseClassificationResult(
|
||||
string.Join("\n", textParts),
|
||||
textParts,
|
||||
toolCalls,
|
||||
nextConsecutiveNoToolResponses);
|
||||
}
|
||||
}
|
||||
@@ -726,21 +726,12 @@ public partial class AgentLoopService
|
||||
return $"⚠ LLM 오류: {ex.Message}";
|
||||
}
|
||||
|
||||
// 응답에서 텍스트와 도구 호출 분리
|
||||
var textParts = new List<string>();
|
||||
var toolCalls = new List<ContentBlock>();
|
||||
|
||||
foreach (var block in blocks)
|
||||
{
|
||||
if (block.Type == "text" && !string.IsNullOrWhiteSpace(block.Text))
|
||||
textParts.Add(block.Text);
|
||||
else if (block.Type == "tool_use")
|
||||
toolCalls.Add(block);
|
||||
}
|
||||
|
||||
// 텍스트 부분
|
||||
var textResponse = string.Join("\n", textParts);
|
||||
consecutiveNoToolResponses = toolCalls.Count == 0 ? consecutiveNoToolResponses + 1 : 0;
|
||||
var responseClassification = AgentLoopResponseClassificationService.Classify(
|
||||
blocks,
|
||||
consecutiveNoToolResponses);
|
||||
var textResponse = responseClassification.TextResponse;
|
||||
var toolCalls = responseClassification.ToolCalls;
|
||||
consecutiveNoToolResponses = responseClassification.NextConsecutiveNoToolResponses;
|
||||
|
||||
// 워크플로우 상세 로그: LLM 응답
|
||||
WorkflowLogService.LogLlmResponse(_conversationId, _currentRunId, iteration,
|
||||
@@ -804,9 +795,7 @@ public partial class AgentLoopService
|
||||
// Thinking UI: 텍스트 응답 중 도구 호출이 있으면 "사고 과정"으로 표시
|
||||
if (!string.IsNullOrEmpty(textResponse) && toolCalls.Count > 0)
|
||||
{
|
||||
var thinkingSummary = textResponse.Length > 150
|
||||
? textResponse[..150] + "…"
|
||||
: textResponse;
|
||||
var thinkingSummary = responseClassification.BuildThinkingSummary();
|
||||
EmitEvent(AgentEventType.Thinking, "", thinkingSummary);
|
||||
}
|
||||
|
||||
|
||||
@@ -442,7 +442,10 @@ public static class CodeLanguageCatalog
|
||||
if (!string.IsNullOrWhiteSpace(primaryGuidance))
|
||||
parts.Add("focus: " + primaryGuidance);
|
||||
if (string.Equals(capability.Key, "sql", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
parts.Add("analysis: detect dialect, script intent, destructive risk, migration order, and object dependencies");
|
||||
parts.Add("review: confirm rollback notes, dependency order, and row-count guards before apply");
|
||||
}
|
||||
|
||||
if (parts.Count == 0)
|
||||
return capability.DisplayName;
|
||||
|
||||
@@ -57,29 +57,34 @@ public static class SqlAnalysisService
|
||||
|
||||
public static string BuildFallbackSummary(string? filePathOrExtension)
|
||||
{
|
||||
SqlAnalysisReport report;
|
||||
if (!string.IsNullOrWhiteSpace(filePathOrExtension) && File.Exists(filePathOrExtension))
|
||||
{
|
||||
var sql = File.ReadAllText(filePathOrExtension);
|
||||
return Analyze(sql, filePathOrExtension).ToFallbackSummary();
|
||||
report = Analyze(sql, filePathOrExtension);
|
||||
}
|
||||
else
|
||||
{
|
||||
report = new SqlAnalysisReport(
|
||||
"Generic SQL",
|
||||
"generic script",
|
||||
["DDL/DML review"],
|
||||
[],
|
||||
[],
|
||||
["Review destructive statements and transaction boundaries before execution."],
|
||||
[
|
||||
"Confirm migration order, object dependencies, and rollback strategy",
|
||||
"Run the script in a disposable database before applying it to shared environments",
|
||||
"Review broad UPDATE/DELETE predicates, indexes, and constraint impact"
|
||||
],
|
||||
[
|
||||
"Identify whether the script is a migration, seed, or ad-hoc query before execution",
|
||||
"Check dependency order for tables, views, indexes, and procedures",
|
||||
"Prepare a rollback or recovery note for destructive or high-impact changes"
|
||||
]);
|
||||
}
|
||||
|
||||
return new SqlAnalysisReport(
|
||||
"Generic SQL",
|
||||
"generic script",
|
||||
["DDL/DML review"],
|
||||
[],
|
||||
[],
|
||||
["Review destructive statements and transaction boundaries before execution."],
|
||||
[
|
||||
"Confirm migration order, object dependencies, and rollback strategy",
|
||||
"Run the script in a disposable database before applying it to shared environments",
|
||||
"Review broad UPDATE/DELETE predicates, indexes, and constraint impact"
|
||||
],
|
||||
[
|
||||
"Identify whether the script is a migration, seed, or ad-hoc query before execution",
|
||||
"Check dependency order for tables, views, indexes, and procedures",
|
||||
"Prepare a rollback or recovery note for destructive or high-impact changes"
|
||||
]).ToFallbackSummary();
|
||||
return report.ToFallbackSummary() + Environment.NewLine + SqlReviewService.Review(report).ToFallbackSummary();
|
||||
}
|
||||
|
||||
private static IReadOnlyList<string> DetectStatementKinds(string sql)
|
||||
|
||||
119
src/AxCopilot/Services/SqlReviewService.cs
Normal file
119
src/AxCopilot/Services/SqlReviewService.cs
Normal file
@@ -0,0 +1,119 @@
|
||||
namespace AxCopilot.Services;
|
||||
|
||||
public sealed record SqlReviewResult(
|
||||
string Severity,
|
||||
IReadOnlyList<string> Findings,
|
||||
IReadOnlyList<string> Checklist)
|
||||
{
|
||||
public string ToFallbackSummary()
|
||||
{
|
||||
var lines = new List<string> { $"review severity: {Severity}" };
|
||||
if (Findings.Count > 0)
|
||||
lines.Add("key findings: " + string.Join(" | ", Findings.Take(3)));
|
||||
if (Checklist.Count > 0)
|
||||
lines.Add("review checklist: " + string.Join(" | ", Checklist.Take(4)));
|
||||
return string.Join(Environment.NewLine, lines);
|
||||
}
|
||||
}
|
||||
|
||||
public static class SqlReviewService
|
||||
{
|
||||
public static SqlReviewResult Review(SqlAnalysisReport report)
|
||||
{
|
||||
var findings = new List<string>();
|
||||
var checklist = new List<string>();
|
||||
|
||||
if (string.Equals(report.ScriptIntent, "schema migration", StringComparison.OrdinalIgnoreCase) ||
|
||||
string.Equals(report.ScriptIntent, "schema change", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Treat the script as an ordered schema change and verify migration sequencing.");
|
||||
checklist.Add("Apply the script in migration order and confirm dependent views, indexes, and procedures are ready.");
|
||||
}
|
||||
|
||||
if (string.Equals(report.ScriptIntent, "seed / reference data", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Seed/reference data should remain idempotent across reruns.");
|
||||
checklist.Add("Confirm rerun safety and expected upsert behavior for reference rows.");
|
||||
}
|
||||
|
||||
if (string.Equals(report.ScriptIntent, "query / reporting", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Reporting/query SQL should be validated for read scope, join width, and downstream consumers.");
|
||||
checklist.Add("Check result-set width, join selectivity, and whether the query feeds dashboards or extracts.");
|
||||
}
|
||||
|
||||
foreach (var risk in report.Risks)
|
||||
{
|
||||
if (risk.Contains("DROP", StringComparison.OrdinalIgnoreCase) ||
|
||||
risk.Contains("TRUNCATE", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Destructive DDL is present and should be paired with rollback or recovery notes.");
|
||||
checklist.Add("Capture backup or rollback evidence before running destructive DDL.");
|
||||
}
|
||||
else if (risk.Contains("without WHERE", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Broad data change risk is present and row-count expectations must be reviewed.");
|
||||
checklist.Add("Validate affected row counts and predicates before executing broad UPDATE/DELETE statements.");
|
||||
}
|
||||
else if (risk.Contains("transaction", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Transaction scope is unclear for a mutating script.");
|
||||
checklist.Add("Define explicit transaction boundaries or document why the script is safe without them.");
|
||||
}
|
||||
else if (risk.Contains("SELECT *", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
findings.Add("Wildcard projection may widen downstream contract unexpectedly.");
|
||||
checklist.Add("Replace SELECT * with explicit columns when the query feeds stable consumers.");
|
||||
}
|
||||
}
|
||||
|
||||
if (report.Dependencies.Count > 0)
|
||||
{
|
||||
findings.Add("Referenced dependencies should be checked before rollout: " +
|
||||
string.Join(", ", report.Dependencies.Take(4)));
|
||||
checklist.Add("Validate dependency order for referenced tables, views, functions, and lookup sources.");
|
||||
}
|
||||
|
||||
foreach (var note in report.ReviewNotes)
|
||||
{
|
||||
if (note.Contains("rollback", StringComparison.OrdinalIgnoreCase))
|
||||
checklist.Add("Document rollback or recovery notes alongside the change.");
|
||||
if (note.Contains("lock", StringComparison.OrdinalIgnoreCase))
|
||||
checklist.Add("Review locking, constraint, and index impact during rollout.");
|
||||
if (note.Contains("derived database objects", StringComparison.OrdinalIgnoreCase))
|
||||
checklist.Add("Revalidate derived objects and deployment order for procedures, views, and triggers.");
|
||||
}
|
||||
|
||||
if (findings.Count == 0)
|
||||
findings.Add("No high-risk SQL pattern was detected, but dependency and execution-order review is still recommended.");
|
||||
if (checklist.Count == 0)
|
||||
checklist.Add("Run the script in a disposable database and capture verification notes before shared rollout.");
|
||||
|
||||
var severity = DetermineSeverity(report);
|
||||
return new SqlReviewResult(
|
||||
severity,
|
||||
findings.Distinct(StringComparer.OrdinalIgnoreCase).Take(4).ToList(),
|
||||
checklist.Distinct(StringComparer.OrdinalIgnoreCase).Take(5).ToList());
|
||||
}
|
||||
|
||||
private static string DetermineSeverity(SqlAnalysisReport report)
|
||||
{
|
||||
if (report.Risks.Any(risk =>
|
||||
risk.Contains("DROP", StringComparison.OrdinalIgnoreCase) ||
|
||||
risk.Contains("TRUNCATE", StringComparison.OrdinalIgnoreCase) ||
|
||||
risk.Contains("without WHERE", StringComparison.OrdinalIgnoreCase)))
|
||||
{
|
||||
return "high";
|
||||
}
|
||||
|
||||
if (string.Equals(report.ScriptIntent, "schema migration", StringComparison.OrdinalIgnoreCase) ||
|
||||
string.Equals(report.ScriptIntent, "schema change", StringComparison.OrdinalIgnoreCase) ||
|
||||
report.Dependencies.Count > 0 ||
|
||||
report.Risks.Any(risk => risk.Contains("transaction", StringComparison.OrdinalIgnoreCase)))
|
||||
{
|
||||
return "medium";
|
||||
}
|
||||
|
||||
return "low";
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user