diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index c099f63..37a130b 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -4522,4 +4522,13 @@ target_link_libraries(step626_test PRIVATE tree_sitter_javascript tree_sitter_typescript tree_sitter_java tree_sitter_rust tree_sitter_go) +add_executable(step627_test tests/step627_test.cpp) +target_include_directories(step627_test PRIVATE src) +target_link_libraries(step627_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) diff --git a/editor/src/AgentChatPanelModel.h b/editor/src/AgentChatPanelModel.h index ea5a0a5..b004ea0 100644 --- a/editor/src/AgentChatPanelModel.h +++ b/editor/src/AgentChatPanelModel.h @@ -3,6 +3,7 @@ #include "AgentToolCallVisualization.h" #include "AgentMutationPreview.h" +#include "AgentMutationApproval.h" #include #include @@ -24,6 +25,7 @@ struct AgentChatState { std::vector messages; std::vector toolCalls; std::vector mutationPreviews; + std::vector mutationApprovals; std::string draftInput; bool autoScroll = true; bool open = false; diff --git a/editor/src/AgentMutationApproval.h b/editor/src/AgentMutationApproval.h new file mode 100644 index 0000000..2698251 --- /dev/null +++ b/editor/src/AgentMutationApproval.h @@ -0,0 +1,95 @@ +#pragma once +// Step 627: Inline accept/reject/modify controls + +#include +#include + +enum class MutationDecision { + Pending, + Accepted, + Rejected, + Modified +}; + +struct MutationApprovalRecord { + std::string previewId; + MutationDecision decision = MutationDecision::Pending; + std::string reason; + std::string modifiedMutationJson; +}; + +class AgentMutationApproval { +public: + static void ensureRecord(std::vector* records, + const std::string& previewId) { + if (!records) return; + if (findMutable(records, previewId)) return; + records->push_back({previewId, MutationDecision::Pending, "", ""}); + } + + static bool accept(std::vector* records, + const std::string& previewId) { + auto* record = ensureAndGet(records, previewId); + if (!record) return false; + record->decision = MutationDecision::Accepted; + record->reason.clear(); + return true; + } + + static bool reject(std::vector* records, + const std::string& previewId, + const std::string& reason) { + auto* record = ensureAndGet(records, previewId); + if (!record || reason.empty()) return false; + record->decision = MutationDecision::Rejected; + record->reason = reason; + return true; + } + + static bool modify(std::vector* records, + const std::string& previewId, + const std::string& modifiedMutationJson) { + auto* record = ensureAndGet(records, previewId); + if (!record || modifiedMutationJson.empty()) return false; + record->decision = MutationDecision::Modified; + record->modifiedMutationJson = modifiedMutationJson; + record->reason.clear(); + return true; + } + + static const MutationApprovalRecord* find(const std::vector& records, + const std::string& previewId) { + for (const auto& record : records) { + if (record.previewId == previewId) return &record; + } + return nullptr; + } + + static std::string decisionText(MutationDecision decision) { + if (decision == MutationDecision::Pending) return "pending"; + if (decision == MutationDecision::Accepted) return "accepted"; + if (decision == MutationDecision::Rejected) return "rejected"; + return "modified"; + } + + static std::string rejectionMessage(const MutationApprovalRecord& record) { + return "rejected: " + record.reason; + } + +private: + static MutationApprovalRecord* ensureAndGet(std::vector* records, + const std::string& previewId) { + if (!records || previewId.empty()) return nullptr; + ensureRecord(records, previewId); + return findMutable(records, previewId); + } + + static MutationApprovalRecord* findMutable(std::vector* records, + const std::string& previewId) { + if (!records) return nullptr; + for (auto& record : *records) { + if (record.previewId == previewId) return &record; + } + return nullptr; + } +}; diff --git a/editor/src/panels/AgentChatPanel.h b/editor/src/panels/AgentChatPanel.h index 17e2991..2f59a31 100644 --- a/editor/src/panels/AgentChatPanel.h +++ b/editor/src/panels/AgentChatPanel.h @@ -52,6 +52,9 @@ static void renderAgentChatPanel(EditorState& state) { ImGui::TextUnformatted("Mutation Previews"); for (std::size_t i = 0; i < chat.mutationPreviews.size(); ++i) { const auto& preview = chat.mutationPreviews[i]; + AgentMutationApproval::ensureRecord(&chat.mutationApprovals, preview.previewId); + const MutationApprovalRecord* approval = + AgentMutationApproval::find(chat.mutationApprovals, preview.previewId); std::string header = preview.previewId + (preview.hasChanges ? " (changes)" : " (no changes)"); if (ImGui::CollapsingHeader((header + "##preview_" + std::to_string(i)).c_str())) { ImGui::TextUnformatted("Mutation JSON"); @@ -70,6 +73,24 @@ static void renderAgentChatPanel(EditorState& state) { ImGui::TextUnformatted(preview.afterCode.c_str()); ImGui::EndChild(); ImGui::Columns(1); + + ImGui::Separator(); + if (approval) { + ImGui::Text("Decision: %s", AgentMutationApproval::decisionText(approval->decision).c_str()); + } + if (ImGui::Button(("Accept##" + preview.previewId).c_str())) { + AgentMutationApproval::accept(&chat.mutationApprovals, preview.previewId); + } + ImGui::SameLine(); + if (ImGui::Button(("Reject##" + preview.previewId).c_str())) { + AgentMutationApproval::reject(&chat.mutationApprovals, preview.previewId, "user_rejected"); + } + ImGui::SameLine(); + if (ImGui::Button(("Modify##" + preview.previewId).c_str())) { + AgentMutationApproval::modify(&chat.mutationApprovals, + preview.previewId, + preview.mutationJson); + } } } } diff --git a/editor/tests/step627_test.cpp b/editor/tests/step627_test.cpp new file mode 100644 index 0000000..cc36727 --- /dev/null +++ b/editor/tests/step627_test.cpp @@ -0,0 +1,137 @@ +// Step 627: Inline accept/reject controls (12 tests) + +#include "AgentMutationApproval.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 {} + +void test_ensure_record_creates_pending_entry() { + TEST(ensure_record_creates_pending_entry); + std::vector records; + AgentMutationApproval::ensureRecord(&records, "p1"); + CHECK(records.size() == 1, "record should be created"); + CHECK(records[0].decision == MutationDecision::Pending, "initial decision should be pending"); + PASS(); +} + +void test_ensure_record_is_idempotent() { + TEST(ensure_record_is_idempotent); + std::vector records; + AgentMutationApproval::ensureRecord(&records, "p1"); + AgentMutationApproval::ensureRecord(&records, "p1"); + CHECK(records.size() == 1, "record should not duplicate"); + PASS(); +} + +void test_accept_sets_decision_accepted() { + TEST(accept_sets_decision_accepted); + std::vector records; + CHECK(AgentMutationApproval::accept(&records, "p1"), "accept should succeed"); + auto* record = AgentMutationApproval::find(records, "p1"); + CHECK(record && record->decision == MutationDecision::Accepted, "decision mismatch"); + PASS(); +} + +void test_reject_requires_reason() { + TEST(reject_requires_reason); + std::vector records; + CHECK(!AgentMutationApproval::reject(&records, "p1", ""), "reject should fail without reason"); + PASS(); +} + +void test_reject_sets_decision_and_reason() { + TEST(reject_sets_decision_and_reason); + std::vector records; + CHECK(AgentMutationApproval::reject(&records, "p1", "unsafe mutation"), "reject should succeed"); + auto* record = AgentMutationApproval::find(records, "p1"); + CHECK(record && record->decision == MutationDecision::Rejected, "decision mismatch"); + CHECK(record && record->reason == "unsafe mutation", "reason mismatch"); + PASS(); +} + +void test_modify_requires_payload() { + TEST(modify_requires_payload); + std::vector records; + CHECK(!AgentMutationApproval::modify(&records, "p1", ""), "modify should fail without payload"); + PASS(); +} + +void test_modify_sets_decision_and_payload() { + TEST(modify_sets_decision_and_payload); + std::vector records; + CHECK(AgentMutationApproval::modify(&records, "p1", "{\"type\":\"setProperty\"}"), + "modify should succeed"); + auto* record = AgentMutationApproval::find(records, "p1"); + CHECK(record && record->decision == MutationDecision::Modified, "decision mismatch"); + CHECK(record && record->modifiedMutationJson.find("setProperty") != std::string::npos, + "modified payload mismatch"); + PASS(); +} + +void test_accept_clears_rejection_reason() { + TEST(accept_clears_rejection_reason); + std::vector records; + (void)AgentMutationApproval::reject(&records, "p1", "need review"); + (void)AgentMutationApproval::accept(&records, "p1"); + auto* record = AgentMutationApproval::find(records, "p1"); + CHECK(record && record->reason.empty(), "reason should be cleared on accept"); + PASS(); +} + +void test_decision_text_mapping_pending() { + TEST(decision_text_mapping_pending); + CHECK(AgentMutationApproval::decisionText(MutationDecision::Pending) == "pending", + "pending label mismatch"); + PASS(); +} + +void test_decision_text_mapping_modified() { + TEST(decision_text_mapping_modified); + CHECK(AgentMutationApproval::decisionText(MutationDecision::Modified) == "modified", + "modified label mismatch"); + PASS(); +} + +void test_rejection_message_format() { + TEST(rejection_message_format); + MutationApprovalRecord record; + record.previewId = "p1"; + record.decision = MutationDecision::Rejected; + record.reason = "invalid args"; + CHECK(AgentMutationApproval::rejectionMessage(record) == "rejected: invalid args", + "rejection message mismatch"); + PASS(); +} + +void test_find_returns_null_for_missing_preview() { + TEST(find_returns_null_for_missing_preview); + std::vector records; + CHECK(AgentMutationApproval::find(records, "missing") == nullptr, "find should return null"); + PASS(); +} + +int main() { + std::cout << "Step 627: Inline accept/reject controls\n"; + + test_ensure_record_creates_pending_entry(); // 1 + test_ensure_record_is_idempotent(); // 2 + test_accept_sets_decision_accepted(); // 3 + test_reject_requires_reason(); // 4 + test_reject_sets_decision_and_reason(); // 5 + test_modify_requires_payload(); // 6 + test_modify_sets_decision_and_payload(); // 7 + test_accept_clears_rejection_reason(); // 8 + test_decision_text_mapping_pending(); // 9 + test_decision_text_mapping_modified(); // 10 + test_rejection_message_format(); // 11 + test_find_returns_null_for_missing_preview();// 12 + + std::cout << "\nResults: " << passed << "/" << (passed + failed) << " passed\n"; + return failed == 0 ? 0 : 1; +} diff --git a/progress.md b/progress.md index 3554e0c..0b53300 100644 --- a/progress.md +++ b/progress.md @@ -12527,3 +12527,39 @@ diff metadata so pending agent edits can be inspected before application. - `editor/src/panels/AgentChatPanel.h` (`85` <= `600`) - `editor/tests/step626_test.cpp` within test-file size guidance (`135` lines) - Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md` + +### Step 627: Inline accept/reject controls +**Status:** PASS (12/12 tests) + +Adds human-in-the-loop mutation decisions with explicit accept/reject/modify +state tracking and inline control wiring for each preview item. + +**Files added:** +- `editor/src/AgentMutationApproval.h` - approval model: + - pending/accepted/rejected/modified decision states + - accept/reject/modify transitions with validation + - rejection feedback message shaping +- `editor/tests/step627_test.cpp` - 12 tests covering: + - record creation/idempotency + - decision transition behavior + - validation for reject/modify payloads + - decision/rejection text formatting and lookup behavior + +**Files modified:** +- `editor/src/AgentChatPanelModel.h` - added mutation approval state storage +- `editor/src/panels/AgentChatPanel.h` - added inline `[Accept] [Reject] [Modify]` controls + and per-preview decision status rendering +- `editor/CMakeLists.txt` - `step627_test` target + +**Verification run:** +- `cmake -S editor -B editor/build-native` - PASS +- `cmake --build editor/build-native --target step627_test step626_test` - PASS +- `./editor/build-native/step627_test` - PASS (12/12) +- `./editor/build-native/step626_test` - PASS (12/12) regression coverage + +**Architecture gate check:** +- `editor/src/AgentMutationApproval.h` (`95` <= `600`) +- `editor/src/AgentChatPanelModel.h` (`111` <= `600`) +- `editor/src/panels/AgentChatPanel.h` (`106` <= `600`) +- `editor/tests/step627_test.cpp` within test-file size guidance (`137` lines) +- Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md`