From 717d0f2143a38dc8996a19037859348dce36d581 Mon Sep 17 00:00:00 2001 From: lacvet Date: Wed, 15 Apr 2026 11:51:42 +0900 Subject: [PATCH] =?UTF-8?q?=EF=BB=BFSQL=20=EB=A6=AC=EB=B7=B0=20=EA=B3=84?= =?UTF-8?q?=EC=B8=B5=EA=B3=BC=20AgentLoop=20=EC=9D=91=EB=8B=B5=20=EB=B6=84?= =?UTF-8?q?=ED=95=B4=20helper=EB=A5=BC=20=EC=B6=94=EA=B0=80=ED=95=B4=20?= =?UTF-8?q?=EC=BD=94=EB=93=9C=20=ED=83=AD=20=EB=A7=88=EA=B0=90=20=ED=92=88?= =?UTF-8?q?=EC=A7=88=EC=9D=84=20=EB=86=92=EC=9E=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- README.md | 13 ++ docs/DEVELOPMENT.md | 35 ++++++ docs/NEXT_ROADMAP.md | 7 ++ ...tLoopResponseClassificationServiceTests.cs | 42 +++++++ .../Services/CodeLanguageCatalogTests.cs | 1 + .../Services/SqlAnalysisServiceTests.cs | 2 + .../Services/SqlReviewServiceTests.cs | 50 ++++++++ .../AgentLoopResponseClassificationService.cs | 52 ++++++++ .../Services/Agent/AgentLoopService.cs | 25 ++-- src/AxCopilot/Services/CodeLanguageCatalog.cs | 3 + src/AxCopilot/Services/SqlAnalysisService.cs | 41 +++--- src/AxCopilot/Services/SqlReviewService.cs | 119 ++++++++++++++++++ 12 files changed, 354 insertions(+), 36 deletions(-) create mode 100644 src/AxCopilot.Tests/Services/AgentLoopResponseClassificationServiceTests.cs create mode 100644 src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs create mode 100644 src/AxCopilot/Services/Agent/AgentLoopResponseClassificationService.cs create mode 100644 src/AxCopilot/Services/SqlReviewService.cs diff --git a/README.md b/README.md index 2425008..7022535 100644 --- a/README.md +++ b/README.md @@ -2022,3 +2022,16 @@ MIT License - 검증: - `dotnet build src/AxCopilot/AxCopilot.csproj -c Release -v minimal -p:OutputPath=bin\\verify_code_sql_doc\\ -p:IntermediateOutputPath=obj\\verify_code_sql_doc\\` 경고 0 / 오류 0 - `dotnet test src/AxCopilot.Tests/AxCopilot.Tests.csproj -c Release -v minimal --filter "SqlDialectDetectorTests|SqlAnalysisServiceTests|CodeLanguageCatalogTests|WorkspaceContextGeneratorTests|ArtifactQualityReviewServiceTests|ArtifactRepairGuideServiceTests|DeckQualityReviewServiceTests|HtmlSkillConsultingSectionsTests" -p:OutputPath=bin\\verify_code_sql_doc_tests\\ -p:IntermediateOutputPath=obj\\verify_code_sql_doc_tests\\` 통과 62 + +업데이트: 2026-04-15 11:50 (KST) +- SQL을 단순 정적 요약에서 `전용 리뷰` 단계로 더 끌어올렸습니다. 새 [SqlReviewService.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot/Services/SqlReviewService.cs)는 `severity`, `key findings`, `review checklist`를 계산해 schema migration, seed/reference data, reporting query마다 다른 검토 포인트와 rollback·row-count·dependency 체크리스트를 fallback 결과에 붙입니다. +- [SqlAnalysisService.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot/Services/SqlAnalysisService.cs)는 SQL fallback summary 끝에 위 리뷰 결과를 함께 출력하도록 확장됐고, [CodeLanguageCatalog.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot/Services/CodeLanguageCatalog.cs)는 SQL workflow summary에 `review` 힌트를 추가해 rollback notes, dependency order, row-count guard를 더 명시적으로 안내합니다. +- 에이전틱 루프 본체도 한 단계 더 가벼워졌습니다. 새 [AgentLoopResponseClassificationService.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot/Services/Agent/AgentLoopResponseClassificationService.cs)는 LLM 응답 블록에서 `text/tool_use` 분리, no-tool 연속 카운트 계산, thinking summary 생성을 담당하고, [AgentLoopService.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot/Services/Agent/AgentLoopService.cs)는 이 helper를 통해 응답 해석 책임을 줄였습니다. +- 테스트: + - [SqlReviewServiceTests.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs) + - [AgentLoopResponseClassificationServiceTests.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot.Tests/Services/AgentLoopResponseClassificationServiceTests.cs) + - [SqlAnalysisServiceTests.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot.Tests/Services/SqlAnalysisServiceTests.cs) + - [CodeLanguageCatalogTests.cs](/E:/AX%20Copilot%20-%20Codex/src/AxCopilot.Tests/Services/CodeLanguageCatalogTests.cs) +- 검증: + - `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 diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index b7049e5..7ba2f8d 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -1247,3 +1247,38 @@ UI ?붿옄???€洹쒕え 由ы뙥?좊쭅 ???꾪뿕 ?묒뾽 ??湲곕줉???덉쟾 ### 검증 - `dotnet build src/AxCopilot/AxCopilot.csproj -c Release -v minimal -p:OutputPath=bin\\verify_code_sql_doc\\ -p:IntermediateOutputPath=obj\\verify_code_sql_doc\\` 경고 0 / 오류 0 - `dotnet test src/AxCopilot.Tests/AxCopilot.Tests.csproj -c Release -v minimal --filter "SqlDialectDetectorTests|SqlAnalysisServiceTests|CodeLanguageCatalogTests|WorkspaceContextGeneratorTests|ArtifactQualityReviewServiceTests|ArtifactRepairGuideServiceTests|DeckQualityReviewServiceTests|HtmlSkillConsultingSectionsTests" -p:OutputPath=bin\\verify_code_sql_doc_tests\\ -p:IntermediateOutputPath=obj\\verify_code_sql_doc_tests\\` 통과 62 + +업데이트: 2026-04-15 11:50 (KST) + +### SQL review 계층 추가 +- 새 `SqlReviewService.cs` + - `SqlReviewResult`를 도입해 `review severity`, `key findings`, `review checklist`를 구조화했습니다. + - schema migration/schema change는 migration sequencing, dependent object readiness를 우선 체크합니다. + - seed/reference data는 rerun safety와 idempotent upsert 관점을 별도 체크합니다. + - query/reporting SQL은 join width와 downstream consumer 영향 검토 포인트를 추가합니다. + - destructive DDL, broad DML, unclear transaction scope, wildcard projection을 findings/checklist로 변환합니다. +- `SqlAnalysisService.cs` + - `BuildFallbackSummary()`가 `SqlReviewService.Review(report)` 결과를 이어붙여 SQL fallback을 `analysis + review` 2단 구조로 반환하도록 변경했습니다. +- `CodeLanguageCatalog.cs` + - SQL workflow summary에 `review: confirm rollback notes, dependency order, and row-count guards before apply` 힌트를 추가했습니다. + +### AgentLoop 응답 분해 helper 추가 +- 새 `AgentLoopResponseClassificationService.cs` + - LLM 응답 블록을 `TextResponse`, `TextParts`, `ToolCalls`, `NextConsecutiveNoToolResponses`로 분류합니다. + - `BuildThinkingSummary()`를 제공해 thinking preview 길이 제한도 helper에서 처리합니다. +- `AgentLoopService.cs` + - 수동 `text/tool_use` 분리 루프를 제거하고 `AgentLoopResponseClassificationService.Classify()`를 사용하도록 정리했습니다. + - no-tool 응답 누적 카운트와 thinking summary 생성 책임을 helper 호출로 대체했습니다. + +### 테스트 +- 새 `SqlReviewServiceTests.cs` + - destructive migration은 `high` severity와 rollback checklist를 반환하는지 검증 + - seed/reference data는 idempotency와 rerun safety 체크리스트를 반환하는지 검증 +- 새 `AgentLoopResponseClassificationServiceTests.cs` + - text/tool_use 분리와 no-tool counter reset/increment 동작 검증 +- 기존 `SqlAnalysisServiceTests.cs`, `CodeLanguageCatalogTests.cs` + - SQL fallback summary에 `review severity`, `review checklist`, rollback review 힌트가 들어가는지 회귀 검증 + +### 검증 +- `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 diff --git a/docs/NEXT_ROADMAP.md b/docs/NEXT_ROADMAP.md index 3926459..a1b5968 100644 --- a/docs/NEXT_ROADMAP.md +++ b/docs/NEXT_ROADMAP.md @@ -210,3 +210,10 @@ 1. SQL fallback은 `dialect/statement/risk` 수준을 넘어 `script intent/dependency/review focus`까지 확장됐습니다. 다음 선택지는 dialect별 migration lint, schema dependency graph, rollback 시뮬레이션처럼 더 깊은 전용 리뷰 계층입니다. 2. HTML 문서는 `decision_matrix`와 `metric_strip`이 들어오면서 board/strategy 보고서의 의사결정 구조를 더 직접적으로 표현할 수 있게 됐습니다. 남은 작업은 bundled skill을 목적형으로 더 쪼개고 print/export polish를 마감하는 수준입니다. 3. PPT critic은 headline, trade-off, phase milestone, chart takeaway, KPI context를 세밀하게 보기 시작했습니다. 이후 남는 작업은 finance/sales/board fixture 확대와 slide-level auto-repair 정교화처럼 golden 마감 중심입니다. + +업데이트: 2026-04-15 11:50 (KST) + +### 추가 진행 메모 +1. SQL은 이제 fallback 요약에 별도 `review severity / findings / checklist`가 붙는 수준까지 올라왔습니다. 남은 고도화는 dialect별 migration lint, schema dependency graph, rollback simulation 같은 더 깊은 검증 계층입니다. +2. Agent loop는 응답 분해 helper가 추가되어 `RunAsync` 본체가 한 단계 더 얇아졌습니다. 이후 분리 후보는 tool dispatch와 finalize 쪽이라, 구조적 마감은 거의 끝나가고 있습니다. +3. 문서 쪽은 기능 확장보다 `golden fixture 확대`, `목적형 bundled skill`, `print/export polish`처럼 완성도 마감 항목이 중심으로 남았습니다. diff --git a/src/AxCopilot.Tests/Services/AgentLoopResponseClassificationServiceTests.cs b/src/AxCopilot.Tests/Services/AgentLoopResponseClassificationServiceTests.cs new file mode 100644 index 0000000..51f7647 --- /dev/null +++ b/src/AxCopilot.Tests/Services/AgentLoopResponseClassificationServiceTests.cs @@ -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 + { + 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 + { + 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); + } +} diff --git a/src/AxCopilot.Tests/Services/CodeLanguageCatalogTests.cs b/src/AxCopilot.Tests/Services/CodeLanguageCatalogTests.cs index 496e8a0..a975ffb 100644 --- a/src/AxCopilot.Tests/Services/CodeLanguageCatalogTests.cs +++ b/src/AxCopilot.Tests/Services/CodeLanguageCatalogTests.cs @@ -105,5 +105,6 @@ public class CodeLanguageCatalogTests summary.Should().Contain("dialect"); summary.Should().Contain("migration order"); summary.Should().Contain("dependencies"); + summary.Should().Contain("rollback"); } } diff --git a/src/AxCopilot.Tests/Services/SqlAnalysisServiceTests.cs b/src/AxCopilot.Tests/Services/SqlAnalysisServiceTests.cs index 7b17365..32d896b 100644 --- a/src/AxCopilot.Tests/Services/SqlAnalysisServiceTests.cs +++ b/src/AxCopilot.Tests/Services/SqlAnalysisServiceTests.cs @@ -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 { diff --git a/src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs b/src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs new file mode 100644 index 0000000..29364c8 --- /dev/null +++ b/src/AxCopilot.Tests/Services/SqlReviewServiceTests.cs @@ -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)); + } +} diff --git a/src/AxCopilot/Services/Agent/AgentLoopResponseClassificationService.cs b/src/AxCopilot/Services/Agent/AgentLoopResponseClassificationService.cs new file mode 100644 index 0000000..ba3b871 --- /dev/null +++ b/src/AxCopilot/Services/Agent/AgentLoopResponseClassificationService.cs @@ -0,0 +1,52 @@ +using AxCopilot.Services; + +namespace AxCopilot.Services.Agent; + +internal sealed record AgentLoopResponseClassificationResult( + string TextResponse, + IReadOnlyList TextParts, + List ToolCalls, + int NextConsecutiveNoToolResponses) +{ + public string BuildThinkingSummary(int maxLength = 150) + { + if (string.IsNullOrEmpty(TextResponse)) + return string.Empty; + + return TextResponse.Length > maxLength + ? TextResponse[..maxLength] + "…" + : TextResponse; + } +} + +/// +/// LLM 응답 블록을 텍스트와 tool_use로 분리하고, 무도구 응답 연속 횟수를 계산한다. +/// +internal static class AgentLoopResponseClassificationService +{ + public static AgentLoopResponseClassificationResult Classify( + IReadOnlyList blocks, + int consecutiveNoToolResponses) + { + var textParts = new List(); + var toolCalls = new List(); + + 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); + } +} diff --git a/src/AxCopilot/Services/Agent/AgentLoopService.cs b/src/AxCopilot/Services/Agent/AgentLoopService.cs index f5e23bb..3eabef5 100644 --- a/src/AxCopilot/Services/Agent/AgentLoopService.cs +++ b/src/AxCopilot/Services/Agent/AgentLoopService.cs @@ -726,21 +726,12 @@ public partial class AgentLoopService return $"⚠ LLM 오류: {ex.Message}"; } - // 응답에서 텍스트와 도구 호출 분리 - var textParts = new List(); - var toolCalls = new List(); - - 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); } diff --git a/src/AxCopilot/Services/CodeLanguageCatalog.cs b/src/AxCopilot/Services/CodeLanguageCatalog.cs index 8b458e6..42ffa6e 100644 --- a/src/AxCopilot/Services/CodeLanguageCatalog.cs +++ b/src/AxCopilot/Services/CodeLanguageCatalog.cs @@ -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; diff --git a/src/AxCopilot/Services/SqlAnalysisService.cs b/src/AxCopilot/Services/SqlAnalysisService.cs index 7cc3c59..5edf9e0 100644 --- a/src/AxCopilot/Services/SqlAnalysisService.cs +++ b/src/AxCopilot/Services/SqlAnalysisService.cs @@ -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 DetectStatementKinds(string sql) diff --git a/src/AxCopilot/Services/SqlReviewService.cs b/src/AxCopilot/Services/SqlReviewService.cs new file mode 100644 index 0000000..ce349d3 --- /dev/null +++ b/src/AxCopilot/Services/SqlReviewService.cs @@ -0,0 +1,119 @@ +namespace AxCopilot.Services; + +public sealed record SqlReviewResult( + string Severity, + IReadOnlyList Findings, + IReadOnlyList Checklist) +{ + public string ToFallbackSummary() + { + var lines = new List { $"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(); + var checklist = new List(); + + 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"; + } +}