Step 529: add pre-apply validation gate for constrained edits
This commit is contained in:
@@ -3640,4 +3640,13 @@ target_link_libraries(step528_test PRIVATE
|
||||
tree_sitter_javascript tree_sitter_typescript
|
||||
tree_sitter_java tree_sitter_rust tree_sitter_go)
|
||||
|
||||
add_executable(step529_test tests/step529_test.cpp)
|
||||
target_include_directories(step529_test PRIVATE src)
|
||||
target_link_libraries(step529_test PRIVATE
|
||||
nlohmann_json::nlohmann_json
|
||||
unofficial::tree-sitter::tree-sitter
|
||||
tree_sitter_python tree_sitter_cpp tree_sitter_elisp
|
||||
tree_sitter_javascript tree_sitter_typescript
|
||||
tree_sitter_java tree_sitter_rust tree_sitter_go)
|
||||
|
||||
# Step 12: Dear ImGui shell scaffolding created (main.cpp exists but not built due to dependencies)
|
||||
|
||||
130
editor/src/PreApplyValidationGate.h
Normal file
130
editor/src/PreApplyValidationGate.h
Normal file
@@ -0,0 +1,130 @@
|
||||
#pragma once
|
||||
// Step 529: Pre-Apply Validation Gate
|
||||
|
||||
#include <algorithm>
|
||||
#include <set>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
#include <nlohmann/json.hpp>
|
||||
|
||||
#include "ConstraintViolationDiagnostics.h"
|
||||
|
||||
using json = nlohmann::json;
|
||||
|
||||
struct CandidateEdit {
|
||||
std::string nodeKind;
|
||||
std::string operation;
|
||||
std::vector<std::string> referencedSymbols;
|
||||
};
|
||||
|
||||
struct PreApplyValidationResult {
|
||||
bool allowed = false;
|
||||
bool mayMutateBuffer = false;
|
||||
TaskitemContract contract;
|
||||
ScopeSnapshot scopeSnapshot;
|
||||
ConstraintDiagnosticPacket diagnostics;
|
||||
};
|
||||
|
||||
class PreApplyValidationGate {
|
||||
public:
|
||||
static PreApplyValidationResult validate(const json& taskitemJson,
|
||||
const CandidateEdit& edit,
|
||||
const std::vector<ScopedSymbol>& visibleSymbols,
|
||||
const LegalOperationGraph& graph,
|
||||
const SymbolScopeExtractor& scopeExtractor) {
|
||||
PreApplyValidationResult result;
|
||||
|
||||
auto schema = TypedTaskitemContractSchema::parseAndValidate(taskitemJson);
|
||||
if (!schema.valid) {
|
||||
result.diagnostics = schemaFailurePacket(schema.errors);
|
||||
return result;
|
||||
}
|
||||
result.contract = schema.contract;
|
||||
|
||||
std::set<std::string> forbidden(result.contract.forbiddenSymbols.begin(),
|
||||
result.contract.forbiddenSymbols.end());
|
||||
result.scopeSnapshot = scopeExtractor.buildSnapshot(result.contract.language,
|
||||
edit.nodeKind,
|
||||
visibleSymbols,
|
||||
forbidden);
|
||||
|
||||
ConstraintCheckInput input;
|
||||
input.contract = result.contract;
|
||||
input.language = result.contract.language;
|
||||
input.nodeKind = edit.nodeKind;
|
||||
input.requestedOp = edit.operation;
|
||||
input.requestedSymbols = edit.referencedSymbols;
|
||||
input.scopeSnapshot = result.scopeSnapshot;
|
||||
|
||||
result.diagnostics = ConstraintViolationDiagnostics::evaluate(input, graph, scopeExtractor);
|
||||
|
||||
if (!targetAllowed(result.contract, edit.nodeKind)) {
|
||||
result.diagnostics.violations.push_back({
|
||||
"target_out_of_contract",
|
||||
"Target node kind is outside contract allowedTargets",
|
||||
edit.nodeKind,
|
||||
false
|
||||
});
|
||||
result.diagnostics.ok = false;
|
||||
result.diagnostics.recommendedAction = "escalate";
|
||||
}
|
||||
|
||||
dedupe(result.diagnostics.violations);
|
||||
result.allowed = result.diagnostics.violations.empty();
|
||||
result.mayMutateBuffer = result.allowed;
|
||||
result.diagnostics.ok = result.allowed;
|
||||
if (result.allowed) {
|
||||
result.diagnostics.recommendedAction = "proceed";
|
||||
} else if (result.diagnostics.recommendedAction == "proceed") {
|
||||
result.diagnostics.recommendedAction = deriveAction(result.diagnostics.violations);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private:
|
||||
static bool targetAllowed(const TaskitemContract& contract,
|
||||
const std::string& nodeKind) {
|
||||
return std::find(contract.allowedTargets.begin(),
|
||||
contract.allowedTargets.end(),
|
||||
nodeKind) != contract.allowedTargets.end();
|
||||
}
|
||||
|
||||
static void dedupe(std::vector<ConstraintViolation>& violations) {
|
||||
std::vector<ConstraintViolation> out;
|
||||
for (const auto& v : violations) {
|
||||
bool exists = false;
|
||||
for (const auto& current : out) {
|
||||
if (current.code == v.code && current.field == v.field) {
|
||||
exists = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!exists) out.push_back(v);
|
||||
}
|
||||
violations.swap(out);
|
||||
}
|
||||
|
||||
static std::string deriveAction(const std::vector<ConstraintViolation>& violations) {
|
||||
for (const auto& v : violations) {
|
||||
if (!v.retryable) return "escalate";
|
||||
}
|
||||
return violations.empty() ? "proceed" : "retry";
|
||||
}
|
||||
|
||||
static ConstraintDiagnosticPacket schemaFailurePacket(
|
||||
const std::vector<std::string>& errors) {
|
||||
ConstraintDiagnosticPacket packet;
|
||||
packet.ok = false;
|
||||
packet.recommendedAction = "escalate";
|
||||
for (const auto& error : errors) {
|
||||
packet.violations.push_back({
|
||||
"under_constrained_contract",
|
||||
"Taskitem contract failed schema validation",
|
||||
error,
|
||||
false
|
||||
});
|
||||
}
|
||||
return packet;
|
||||
}
|
||||
};
|
||||
202
editor/tests/step529_test.cpp
Normal file
202
editor/tests/step529_test.cpp
Normal file
@@ -0,0 +1,202 @@
|
||||
// Step 529: Pre-Apply Validation Gate (12 tests)
|
||||
|
||||
#include "PreApplyValidationGate.h"
|
||||
|
||||
#include <iostream>
|
||||
|
||||
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 json validTaskitem() {
|
||||
return {
|
||||
{"id", "ti-529"},
|
||||
{"nodeId", "n-1"},
|
||||
{"language", "cpp"},
|
||||
{"allowedTargets", json::array({"Function"})},
|
||||
{"allowedOps", json::array({"rename", "update"})},
|
||||
{"allowedSymbols", json::array({"cursor", "count", "EditorState", "render", "ghost"})},
|
||||
{"forbiddenSymbols", json::array({"unsafeGlobal"})},
|
||||
{"expectedDiagnosticsAdd", json::array({"none"})},
|
||||
{"expectedDiagnosticsRemove", json::array()}
|
||||
};
|
||||
}
|
||||
|
||||
static std::vector<ScopedSymbol> visibleSymbols() {
|
||||
return {
|
||||
{"cursor", "Variable", 3},
|
||||
{"count", "Parameter", 3},
|
||||
{"EditorState", "Type", 0},
|
||||
{"render", "Function", 1},
|
||||
{"unsafeGlobal", "Variable", 0}
|
||||
};
|
||||
}
|
||||
|
||||
static bool hasCode(const PreApplyValidationResult& result,
|
||||
const std::string& code) {
|
||||
for (const auto& v : result.diagnostics.violations) {
|
||||
if (v.code == code) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
void test_valid_edit_is_allowed_before_apply() {
|
||||
TEST(valid_edit_is_allowed_before_apply);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "rename", {"cursor", "count"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(result.allowed, "valid edit should be allowed");
|
||||
CHECK(result.mayMutateBuffer, "valid edit may mutate buffer");
|
||||
CHECK(result.diagnostics.recommendedAction == "proceed", "valid edit should proceed");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_illegal_operation_rejected_before_mutation() {
|
||||
TEST(illegal_operation_rejected_before_mutation);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "delete", {"cursor"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "illegal op should be rejected");
|
||||
CHECK(!result.mayMutateBuffer, "illegal op should block mutation");
|
||||
CHECK(hasCode(result, "illegal_operation"), "missing illegal operation diagnostic");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_out_of_scope_symbol_rejected_before_mutation() {
|
||||
TEST(out_of_scope_symbol_rejected_before_mutation);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "rename", {"ghost"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "out-of-scope symbol should be rejected");
|
||||
CHECK(!result.mayMutateBuffer, "out-of-scope symbol should block mutation");
|
||||
CHECK(hasCode(result, "out_of_scope_symbol"), "missing out-of-scope diagnostic");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_unsupported_context_rejected_before_mutation() {
|
||||
TEST(unsupported_context_rejected_before_mutation);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"MacroBlock", "rename", {"cursor"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "unsupported context should be rejected");
|
||||
CHECK(hasCode(result, "unsupported_context"), "missing unsupported context diagnostic");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_target_outside_allowed_targets_rejected() {
|
||||
TEST(target_outside_allowed_targets_rejected);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Class", "rename", {"cursor"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "target outside contract should be rejected");
|
||||
CHECK(hasCode(result, "target_out_of_contract"), "missing target contract diagnostic");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_forbidden_symbol_rejected() {
|
||||
TEST(forbidden_symbol_rejected);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "rename", {"unsafeGlobal"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "forbidden symbol should be rejected");
|
||||
CHECK(hasCode(result, "forbidden_symbol"), "missing forbidden symbol diagnostic");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_missing_schema_field_rejected() {
|
||||
TEST(missing_schema_field_rejected);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
auto item = validTaskitem();
|
||||
item.erase("allowedOps");
|
||||
CandidateEdit edit{"Function", "rename", {"cursor"}};
|
||||
auto result = PreApplyValidationGate::validate(item, edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "schema-invalid taskitem should be rejected");
|
||||
CHECK(hasCode(result, "under_constrained_contract"), "missing schema violation code");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_duplicate_allowed_ops_rejected_by_schema() {
|
||||
TEST(duplicate_allowed_ops_rejected_by_schema);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
auto item = validTaskitem();
|
||||
item["allowedOps"] = json::array({"rename", "rename"});
|
||||
CandidateEdit edit{"Function", "rename", {"cursor"}};
|
||||
auto result = PreApplyValidationGate::validate(item, edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(!result.allowed, "duplicate allowedOps should fail schema gate");
|
||||
CHECK(hasCode(result, "under_constrained_contract"), "expected schema rejection diagnostic");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_retry_action_for_retryable_only_failure() {
|
||||
TEST(retry_action_for_retryable_only_failure);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "rename", {"ghost"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(result.diagnostics.recommendedAction == "retry", "retryable-only failure should retry");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_multiple_failures_are_aggregated() {
|
||||
TEST(multiple_failures_are_aggregated);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Class", "delete", {"unsafeGlobal"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(hasCode(result, "target_out_of_contract"), "missing target violation");
|
||||
CHECK(hasCode(result, "unsupported_context"), "missing unsupported context violation");
|
||||
CHECK(hasCode(result, "forbidden_symbol"), "missing forbidden symbol violation");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_empty_symbol_list_can_still_validate() {
|
||||
TEST(empty_symbol_list_can_still_validate);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "rename", {}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor);
|
||||
CHECK(result.allowed, "empty symbol list should be valid for rename");
|
||||
PASS();
|
||||
}
|
||||
|
||||
void test_forbidden_symbol_not_in_snapshot_still_reports_violation() {
|
||||
TEST(forbidden_symbol_not_in_snapshot_still_reports_violation);
|
||||
LegalOperationGraph graph;
|
||||
SymbolScopeExtractor extractor;
|
||||
CandidateEdit edit{"Function", "rename", {"unsafeGlobal"}};
|
||||
auto result = PreApplyValidationGate::validate(validTaskitem(), edit, {
|
||||
{"cursor", "Variable", 3}
|
||||
}, graph, extractor);
|
||||
CHECK(hasCode(result, "forbidden_symbol"), "forbidden symbol should still be diagnosed");
|
||||
PASS();
|
||||
}
|
||||
|
||||
int main() {
|
||||
std::cout << "Step 529: Pre-Apply Validation Gate\n";
|
||||
|
||||
test_valid_edit_is_allowed_before_apply(); // 1
|
||||
test_illegal_operation_rejected_before_mutation(); // 2
|
||||
test_out_of_scope_symbol_rejected_before_mutation(); // 3
|
||||
test_unsupported_context_rejected_before_mutation(); // 4
|
||||
test_target_outside_allowed_targets_rejected(); // 5
|
||||
test_forbidden_symbol_rejected(); // 6
|
||||
test_missing_schema_field_rejected(); // 7
|
||||
test_duplicate_allowed_ops_rejected_by_schema(); // 8
|
||||
test_retry_action_for_retryable_only_failure(); // 9
|
||||
test_multiple_failures_are_aggregated(); // 10
|
||||
test_empty_symbol_list_can_still_validate(); // 11
|
||||
test_forbidden_symbol_not_in_snapshot_still_reports_violation();// 12
|
||||
|
||||
std::cout << "\nResults: " << passed << "/" << (passed + failed) << " passed\n";
|
||||
return failed == 0 ? 0 : 1;
|
||||
}
|
||||
38
progress.md
38
progress.md
@@ -8738,3 +8738,41 @@ constraint diagnostics before execution is allowed.
|
||||
- **Steps completed:** 5
|
||||
- **New tests in phase plan:** 56/56 passing
|
||||
- **Constraint model + diagnostics + integration:** PASS
|
||||
|
||||
### Step 529: Pre-Apply Validation Gate
|
||||
**Status:** PASS (12/12 tests)
|
||||
|
||||
Adds a deterministic pre-apply gate that validates candidate edits against
|
||||
contract schema, allowed targets, legal operation graph, and in-scope symbol
|
||||
snapshot before any mutation is allowed.
|
||||
|
||||
**Files added:**
|
||||
- `editor/src/PreApplyValidationGate.h` - pre-apply validation module:
|
||||
- schema-first taskitem validation gate
|
||||
- forbidden-aware scope snapshot build for candidate edit context
|
||||
- operation/symbol constraint evaluation via structured diagnostics
|
||||
- explicit `allowed` and `mayMutateBuffer` decision flags
|
||||
- target-kind contract enforcement (`target_out_of_contract`)
|
||||
- `editor/tests/step529_test.cpp` - 12 tests covering:
|
||||
- valid pre-apply allow path
|
||||
- illegal op, unsupported context, and out-of-scope symbol rejections
|
||||
- target contract mismatch rejection
|
||||
- forbidden symbol rejection
|
||||
- schema-failure rejection paths
|
||||
- retry vs escalate action semantics
|
||||
- multi-violation aggregation behavior
|
||||
- empty symbol-list edge case
|
||||
|
||||
**Files modified:**
|
||||
- `editor/CMakeLists.txt` - `step529_test` target
|
||||
|
||||
**Verification run:**
|
||||
- `cmake -S editor -B editor/build-native` - PASS
|
||||
- `cmake --build editor/build-native --target step529_test step528_test` - PASS
|
||||
- `./editor/build-native/step529_test` - PASS (12/12)
|
||||
- `./editor/build-native/step528_test` - PASS (8/8) regression coverage
|
||||
|
||||
**Architecture gate check:**
|
||||
- `editor/src/PreApplyValidationGate.h` within header-size limit (`130` <= `600`)
|
||||
- `editor/tests/step529_test.cpp` within test-file size guidance (`202` lines)
|
||||
- Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md`
|
||||
|
||||
Reference in New Issue
Block a user