diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index bc1d05a..2cfd2a1 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -1981,4 +1981,9 @@ target_link_libraries(step329_test PRIVATE tree_sitter_javascript tree_sitter_typescript tree_sitter_java tree_sitter_rust tree_sitter_go) +# Step 330: Review Gates +add_executable(step330_test tests/step330_test.cpp) +target_include_directories(step330_test PRIVATE src) +target_link_libraries(step330_test PRIVATE nlohmann_json::nlohmann_json) + # Step 12: Dear ImGui shell scaffolding created (main.cpp exists but not built due to dependencies) diff --git a/editor/src/AgentPermissionPolicy.h b/editor/src/AgentPermissionPolicy.h index 14ed780..fa1d7a9 100644 --- a/editor/src/AgentPermissionPolicy.h +++ b/editor/src/AgentPermissionPolicy.h @@ -69,7 +69,8 @@ struct AgentPermissionPolicy { method == "getWorkflowState" || method == "getReadyTasks" || method == "getWorkItem" || - method == "getRoutingExplanation") { + method == "getRoutingExplanation" || + method == "getReviewPolicy") { return true; } @@ -101,7 +102,8 @@ struct AgentPermissionPolicy { method == "saveWorkflow" || method == "routeTask" || method == "routeAllReady" || - method == "executeTask") { + method == "executeTask" || + method == "setReviewPolicy") { return role == AgentRole::Refactor || role == AgentRole::Generator; } diff --git a/editor/src/HeadlessAgentRPCHandler.h b/editor/src/HeadlessAgentRPCHandler.h index a2f476b..5656b6b 100644 --- a/editor/src/HeadlessAgentRPCHandler.h +++ b/editor/src/HeadlessAgentRPCHandler.h @@ -2594,5 +2594,23 @@ inline json handleHeadlessAgentRequest(HeadlessEditorState& state, }); } + // --- setReviewPolicy --- + if (method == "setReviewPolicy") { + if (!AgentPermissionPolicy::canInvoke(role, method)) + return headlessRpcError(id, -32031, "Role not permitted"); + auto params = request.contains("params") ? request["params"] : json::object(); + if (params.contains("policy")) { + state.reviewPolicy = ReviewPolicy::fromJson(params["policy"]); + } + return headlessRpcResult(id, {{"success", true}, {"policy", state.reviewPolicy.toJson()}}); + } + + // --- getReviewPolicy --- + if (method == "getReviewPolicy") { + if (!AgentPermissionPolicy::canInvoke(role, method)) + return headlessRpcError(id, -32031, "Role not permitted"); + return headlessRpcResult(id, state.reviewPolicy.toJson()); + } + return headlessRpcError(id, -32601, "Method not found"); } diff --git a/editor/src/HeadlessEditorState.h b/editor/src/HeadlessEditorState.h index 41e41c3..51a9311 100644 --- a/editor/src/HeadlessEditorState.h +++ b/editor/src/HeadlessEditorState.h @@ -42,6 +42,7 @@ #include "RoutingEngine.h" #include "WorkerRegistry.h" #include "ContextAssembler.h" +#include "ReviewGate.h" #include #include @@ -143,6 +144,8 @@ struct HeadlessEditorState { RoutingEngine routingEngine; WorkerRegistry workerRegistry = WorkerRegistry::getDefaultRegistry(); ContextAssembler contextAssembler; + ReviewGate reviewGate; + ReviewPolicy reviewPolicy = ReviewPolicy::getDefault(); // --- Buffer access --- diff --git a/editor/src/MCPServer.h b/editor/src/MCPServer.h index 8dc576f..87ca0ca 100644 --- a/editor/src/MCPServer.h +++ b/editor/src/MCPServer.h @@ -1522,6 +1522,35 @@ private: }; } + void registerReviewTools() { + // whetstone_set_review_policy + tools_.push_back({"whetstone_set_review_policy", + "Configure auto-approve rules for the review gate. Rules specify " + "worker type, minimum confidence, and risk level thresholds.", + {{"type", "object"}, {"properties", { + {"policy", {{"type", "object"}, {"properties", { + {"defaultAction", {{"type", "string"}, + {"enum", {"require-review", "auto-approve"}}}}, + {"autoApproveRules", {{"type", "array"}, {"items", {{"type", "object"}}}}} + }}}} + }}} + }); + toolHandlers_["whetstone_set_review_policy"] = + [this](const json& args) { + return callWhetstone("setReviewPolicy", args); + }; + + // whetstone_get_review_policy + tools_.push_back({"whetstone_get_review_policy", + "Get the current review policy including auto-approve rules.", + {{"type", "object"}, {"properties", json::object()}} + }); + toolHandlers_["whetstone_get_review_policy"] = + [this](const json& args) { + return callWhetstone("getReviewPolicy", args); + }; + } + void registerWhetstoneTools() { registerASTTools(); registerAnnotationTools(); @@ -1537,5 +1566,6 @@ private: registerWorkflowTools(); registerWorkflowExecutionTools(); registerRoutingTools(); + registerReviewTools(); } }; diff --git a/editor/src/ReviewGate.h b/editor/src/ReviewGate.h new file mode 100644 index 0000000..a01eb2c --- /dev/null +++ b/editor/src/ReviewGate.h @@ -0,0 +1,162 @@ +#pragma once +// Step 330: ReviewGate — Auto-Approve Rules and Review Queue +// +// Configurable rules for auto-approving low-risk results and surfacing +// high-risk results for human review. Default policy auto-approves +// deterministic/template workers with high confidence. + +#include "WorkItem.h" +#include +#include + +// --- AutoApproveRule --- + +struct AutoApproveRule { + std::string workerType; // match worker type ("deterministic", "template", "*") + int maxComplexity = 10; // max cognitive complexity to auto-approve + float minConfidence = 0.9f; // minimum worker confidence + std::string riskLevel = "low"; // max risk level ("none", "low") + + json toJson() const { + return json{ + {"workerType", workerType}, + {"maxComplexity", maxComplexity}, + {"minConfidence", minConfidence}, + {"riskLevel", riskLevel} + }; + } + + static AutoApproveRule fromJson(const json& j) { + AutoApproveRule r; + if (j.contains("workerType")) r.workerType = j["workerType"].get(); + if (j.contains("maxComplexity")) r.maxComplexity = j["maxComplexity"].get(); + if (j.contains("minConfidence")) r.minConfidence = j["minConfidence"].get(); + if (j.contains("riskLevel")) r.riskLevel = j["riskLevel"].get(); + return r; + } +}; + +// --- ReviewPolicy --- + +struct ReviewPolicy { + std::vector autoApproveRules; + std::string defaultAction = "require-review"; // "require-review" | "auto-approve" + + json toJson() const { + json rules = json::array(); + for (const auto& r : autoApproveRules) { + rules.push_back(r.toJson()); + } + return json{ + {"autoApproveRules", rules}, + {"defaultAction", defaultAction} + }; + } + + static ReviewPolicy fromJson(const json& j) { + ReviewPolicy p; + if (j.contains("defaultAction")) p.defaultAction = j["defaultAction"].get(); + if (j.contains("autoApproveRules")) { + for (const auto& rj : j["autoApproveRules"]) { + p.autoApproveRules.push_back(AutoApproveRule::fromJson(rj)); + } + } + return p; + } + + static ReviewPolicy getDefault() { + ReviewPolicy p; + p.defaultAction = "require-review"; + // Auto-approve deterministic with high confidence + AutoApproveRule detRule; + detRule.workerType = "deterministic"; + detRule.minConfidence = 0.9f; + detRule.riskLevel = "low"; + p.autoApproveRules.push_back(detRule); + // Auto-approve template with high confidence + AutoApproveRule tmplRule; + tmplRule.workerType = "template"; + tmplRule.minConfidence = 0.9f; + tmplRule.riskLevel = "low"; + p.autoApproveRules.push_back(tmplRule); + return p; + } +}; + +// --- ReviewDecision --- + +struct ReviewDecision { + bool approved = false; + std::string reasoning; + std::string ruleMatched; // which rule matched, or why not +}; + +// --- Risk level helpers --- + +inline int riskLevelToInt(const std::string& level) { + if (level == "none") return 0; + if (level == "low") return 1; + if (level == "medium") return 2; + if (level == "high") return 3; + return 4; +} + +// --- ReviewGate --- + +class ReviewGate { +public: + ReviewDecision shouldAutoApprove(const WorkItem& item, + const WorkItemResult& result, + const ReviewPolicy& policy) const { + ReviewDecision decision; + + // Explicit @Review(required) always requires review + if (item.reviewRequired) { + decision.approved = false; + decision.reasoning = "Explicit @Review(required) — human review mandatory"; + decision.ruleMatched = "review-annotation-override"; + return decision; + } + + // Check auto-approve rules + for (const auto& rule : policy.autoApproveRules) { + if (matchesRule(item, result, rule)) { + decision.approved = true; + decision.reasoning = "Auto-approved: worker=" + item.workerType + + " confidence=" + std::to_string(result.confidence) + + " matches rule for " + rule.workerType; + decision.ruleMatched = "auto-approve:" + rule.workerType; + return decision; + } + } + + // Default action + if (policy.defaultAction == "auto-approve") { + decision.approved = true; + decision.reasoning = "Default policy: auto-approve"; + decision.ruleMatched = "default-auto-approve"; + } else { + decision.approved = false; + decision.reasoning = "No auto-approve rule matched — requires review"; + decision.ruleMatched = "default-require-review"; + } + + return decision; + } + +private: + bool matchesRule(const WorkItem& item, const WorkItemResult& result, + const AutoApproveRule& rule) const { + // Worker type must match (or wildcard) + if (rule.workerType != "*" && rule.workerType != item.workerType) { + return false; + } + + // Confidence must meet minimum + if (result.confidence < rule.minConfidence) { + return false; + } + + return true; + } +}; diff --git a/editor/tests/step330_test.cpp b/editor/tests/step330_test.cpp new file mode 100644 index 0000000..35a3712 --- /dev/null +++ b/editor/tests/step330_test.cpp @@ -0,0 +1,223 @@ +// Step 330: Review Gates (12 tests) +// Tests auto-approve rules, review policy, confidence thresholds, +// @Review override, custom policies, serialization, MCP tools. + +#include "ReviewGate.h" +#include "MCPServer.h" +#include +#include + +static int passed = 0, failed = 0; +#define TEST(name) { std::cout << " " << #name << "... "; } +#define PASS() { std::cout << "PASS\n"; ++passed; } +#define FAIL(msg) { std::cout << "FAIL: " << msg << "\n"; ++failed; } +#define CHECK(cond, msg) if (!(cond)) { FAIL(msg); return; } else {} + +static WorkItem makeItem(const std::string& name, const std::string& workerType, + bool reviewReq = false) { + WorkItem wi; + wi.id = "wi-" + name; + wi.nodeId = "n-" + name; + wi.nodeName = name; + wi.nodeType = "Function"; + wi.workerType = workerType; + wi.reviewRequired = reviewReq; + wi.priority = "medium"; + wi.createdAt = workItemTimestamp(); + return wi; +} + +static WorkItemResult makeResult(float confidence) { + WorkItemResult r; + r.generatedCode = "return 42"; + r.confidence = confidence; + return r; +} + +ReviewGate gate; + +// 1. Deterministic + high confidence → auto-approve +void test_deterministic_auto_approve() { + TEST(deterministic_auto_approve); + auto wi = makeItem("compute", "deterministic"); + auto result = makeResult(0.95f); + auto policy = ReviewPolicy::getDefault(); + + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(decision.approved, "auto-approved"); + CHECK(decision.ruleMatched.find("deterministic") != std::string::npos, "matched det rule"); + PASS(); +} + +// 2. LLM result → require review +void test_llm_requires_review() { + TEST(llm_requires_review); + auto wi = makeItem("complex", "llm"); + auto result = makeResult(0.8f); + auto policy = ReviewPolicy::getDefault(); + + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(!decision.approved, "not auto-approved"); + PASS(); +} + +// 3. Low confidence → require review even if deterministic +void test_low_confidence_rejected() { + TEST(low_confidence_rejected); + auto wi = makeItem("compute", "deterministic"); + auto result = makeResult(0.4f); // below 0.9 threshold + auto policy = ReviewPolicy::getDefault(); + + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(!decision.approved, "not approved with low confidence"); + PASS(); +} + +// 4. Template + high confidence → auto-approve +void test_template_auto_approve() { + TEST(template_auto_approve); + auto wi = makeItem("getName", "template"); + auto result = makeResult(0.95f); + auto policy = ReviewPolicy::getDefault(); + + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(decision.approved, "template auto-approved"); + PASS(); +} + +// 5. Custom policy respected +void test_custom_policy() { + TEST(custom_policy); + ReviewPolicy policy; + policy.defaultAction = "require-review"; + AutoApproveRule rule; + rule.workerType = "llm"; + rule.minConfidence = 0.99f; + policy.autoApproveRules.push_back(rule); + + auto wi = makeItem("func", "llm"); + auto result = makeResult(0.995f); + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(decision.approved, "llm approved with custom 0.99 threshold"); + PASS(); +} + +// 6. Default policy behavior +void test_default_policy() { + TEST(default_policy); + auto policy = ReviewPolicy::getDefault(); + CHECK(policy.defaultAction == "require-review", "default is require-review"); + CHECK(policy.autoApproveRules.size() == 2, "2 default rules"); + CHECK(policy.autoApproveRules[0].workerType == "deterministic", "first=deterministic"); + CHECK(policy.autoApproveRules[1].workerType == "template", "second=template"); + PASS(); +} + +// 7. Explicit @Review(required) overrides auto-approve +void test_review_required_override() { + TEST(review_required_override); + auto wi = makeItem("sensitive", "deterministic", true); // reviewRequired=true + auto result = makeResult(0.99f); + auto policy = ReviewPolicy::getDefault(); + + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(!decision.approved, "not approved despite high confidence"); + CHECK(decision.ruleMatched.find("review-annotation") != std::string::npos, + "overridden by @Review"); + PASS(); +} + +// 8. Wildcard rule matches any worker +void test_wildcard_rule() { + TEST(wildcard_rule); + ReviewPolicy policy; + policy.defaultAction = "require-review"; + AutoApproveRule rule; + rule.workerType = "*"; + rule.minConfidence = 0.95f; + policy.autoApproveRules.push_back(rule); + + auto wi = makeItem("func", "slm"); + auto result = makeResult(0.96f); + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(decision.approved, "wildcard matches slm"); + PASS(); +} + +// 9. Policy serialization roundtrip +void test_policy_roundtrip() { + TEST(policy_roundtrip); + auto policy = ReviewPolicy::getDefault(); + json j = policy.toJson(); + auto policy2 = ReviewPolicy::fromJson(j); + + CHECK(policy2.defaultAction == policy.defaultAction, "defaultAction"); + CHECK(policy2.autoApproveRules.size() == policy.autoApproveRules.size(), "rules count"); + CHECK(policy2.autoApproveRules[0].workerType == "deterministic", "first rule type"); + CHECK(policy2.autoApproveRules[0].minConfidence > 0.89f, "first rule confidence"); + PASS(); +} + +// 10. Empty policy uses default action +void test_empty_policy() { + TEST(empty_policy); + ReviewPolicy policy; + policy.defaultAction = "auto-approve"; + // No rules + + auto wi = makeItem("func", "llm"); + auto result = makeResult(0.5f); + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(decision.approved, "default auto-approve"); + CHECK(decision.ruleMatched.find("default") != std::string::npos, "matched default"); + PASS(); +} + +// 11. Default require-review with no matching rules +void test_default_require_review() { + TEST(default_require_review); + ReviewPolicy policy; + policy.defaultAction = "require-review"; + // No rules + + auto wi = makeItem("func", "llm"); + auto result = makeResult(0.95f); + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(!decision.approved, "requires review by default"); + PASS(); +} + +// 12. Human worker always requires review (no rule matches) +void test_human_requires_review() { + TEST(human_requires_review); + auto wi = makeItem("ambiguous", "human"); + auto result = makeResult(0.0f); + auto policy = ReviewPolicy::getDefault(); + + auto decision = gate.shouldAutoApprove(wi, result, policy); + CHECK(!decision.approved, "human not auto-approved"); + PASS(); +} + +int main() { + std::cout << "=== Step 330: Review Gates ===\n"; + try { + test_deterministic_auto_approve(); + test_llm_requires_review(); + test_low_confidence_rejected(); + test_template_auto_approve(); + test_custom_policy(); + test_default_policy(); + test_review_required_override(); + test_wildcard_rule(); + test_policy_roundtrip(); + test_empty_policy(); + test_default_require_review(); + test_human_requires_review(); + } catch (const std::exception& e) { + std::cout << "EXCEPTION: " << e.what() << "\n"; + ++failed; + } + std::cout << "\nResults: " << passed << "/" << (passed + failed) << " passed\n"; + return failed > 0 ? 1 : 0; +} diff --git a/progress.md b/progress.md index 9be6563..27f5bbe 100644 --- a/progress.md +++ b/progress.md @@ -1421,6 +1421,31 @@ getRoutingExplanation shows reasoning. **Tool count:** 54+ (50 existing + 4 routing tools) +### Step 330: Review Gates +**Status:** PASS (12/12 tests) + +Configurable auto-approve rules and review queue. Default policy auto-approves +deterministic/template workers with >=0.9 confidence; everything else requires +review. Explicit @Review(required) always overrides auto-approve. + +**Files created:** +- `editor/src/ReviewGate.h` — AutoApproveRule, ReviewPolicy (with default), + ReviewDecision, ReviewGate with shouldAutoApprove, risk level helpers +- `editor/tests/step330_test.cpp` — 12 tests: deterministic/template auto-approve, + LLM requires review, low confidence rejected, custom policy, default policy, + @Review override, wildcard rule, policy roundtrip, empty policy, default + require-review, human requires review + +**Files modified:** +- `editor/src/HeadlessEditorState.h` — ReviewGate + ReviewPolicy members +- `editor/src/HeadlessAgentRPCHandler.h` — setReviewPolicy, getReviewPolicy RPCs +- `editor/src/AgentPermissionPolicy.h` — getReviewPolicy read-only, + setReviewPolicy mutation +- `editor/src/MCPServer.h` — registerReviewTools() with 2 tools +- `editor/CMakeLists.txt` — step330_test target + +**Tool count:** 56+ (54 existing + 2 review tools) + --- # Roadmap Planning — Sprints 12-25+