From 820b2743fdf96240fe616c3bc529e56b039df739 Mon Sep 17 00:00:00 2001 From: Bill Date: Tue, 17 Feb 2026 11:07:22 -0700 Subject: [PATCH] Step 577: add architect review surface --- editor/CMakeLists.txt | 9 ++ editor/src/ArchitectReviewSurface.h | 126 ++++++++++++++++++++ editor/tests/step577_test.cpp | 178 ++++++++++++++++++++++++++++ progress.md | 35 ++++++ 4 files changed, 348 insertions(+) create mode 100644 editor/src/ArchitectReviewSurface.h create mode 100644 editor/tests/step577_test.cpp diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index 7b6a72f..caceff7 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -4072,4 +4072,13 @@ target_link_libraries(step576_test PRIVATE tree_sitter_javascript tree_sitter_typescript tree_sitter_java tree_sitter_rust tree_sitter_go) +add_executable(step577_test tests/step577_test.cpp) +target_include_directories(step577_test PRIVATE src) +target_link_libraries(step577_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/ArchitectReviewSurface.h b/editor/src/ArchitectReviewSurface.h new file mode 100644 index 0000000..c4784fa --- /dev/null +++ b/editor/src/ArchitectReviewSurface.h @@ -0,0 +1,126 @@ +#pragma once +// Step 577: Architect Review Surface for Intake + +#include "RequirementNormalizationConflictDetector.h" + +#include +#include +#include + +enum class ReviewDecision { + Pending, + Approved, + Modified, + Rejected +}; + +struct ConstraintReviewItem { + std::string requirementId; + std::string originalText; + std::string reviewedText; + ReviewDecision decision = ReviewDecision::Pending; + std::string reviewer; +}; + +struct ConstraintReviewSummary { + int pending = 0; + int approved = 0; + int modified = 0; + int rejected = 0; + bool allDecided = false; +}; + +class ArchitectReviewSurface { +public: + bool loadConstraints(const std::vector& requirements, std::string* error) { + if (!error) return false; + error->clear(); + items_.clear(); + for (const auto& requirement : requirements) { + if (requirement.kind != NormalizedRequirementKind::Constraint) continue; + ConstraintReviewItem item; + item.requirementId = requirement.requirementId; + item.originalText = requirement.normalizedText; + item.reviewedText = requirement.normalizedText; + items_[item.requirementId] = item; + } + if (items_.empty()) return fail(error, "no_constraints_to_review"); + return true; + } + + bool approve(const std::string& requirementId, + const std::string& reviewer, + std::string* error) { + if (!error) return false; + error->clear(); + auto* item = find(requirementId); + if (!item) return fail(error, "constraint_not_found"); + if (reviewer.empty()) return fail(error, "reviewer_missing"); + item->decision = ReviewDecision::Approved; + item->reviewer = reviewer; + item->reviewedText = item->originalText; + return true; + } + + bool modify(const std::string& requirementId, + const std::string& reviewedText, + const std::string& reviewer, + std::string* error) { + if (!error) return false; + error->clear(); + auto* item = find(requirementId); + if (!item) return fail(error, "constraint_not_found"); + if (reviewer.empty()) return fail(error, "reviewer_missing"); + if (reviewedText.empty()) return fail(error, "reviewed_text_missing"); + item->decision = ReviewDecision::Modified; + item->reviewer = reviewer; + item->reviewedText = reviewedText; + return true; + } + + bool reject(const std::string& requirementId, + const std::string& reviewer, + std::string* error) { + if (!error) return false; + error->clear(); + auto* item = find(requirementId); + if (!item) return fail(error, "constraint_not_found"); + if (reviewer.empty()) return fail(error, "reviewer_missing"); + item->decision = ReviewDecision::Rejected; + item->reviewer = reviewer; + return true; + } + + ConstraintReviewSummary summarize() const { + ConstraintReviewSummary summary; + for (const auto& kv : items_) { + const auto decision = kv.second.decision; + if (decision == ReviewDecision::Pending) ++summary.pending; + if (decision == ReviewDecision::Approved) ++summary.approved; + if (decision == ReviewDecision::Modified) ++summary.modified; + if (decision == ReviewDecision::Rejected) ++summary.rejected; + } + summary.allDecided = summary.pending == 0 && !items_.empty(); + return summary; + } + + std::vector items() const { + std::vector out; + for (const auto& kv : items_) out.push_back(kv.second); + return out; + } + +private: + std::map items_; + + static bool fail(std::string* error, const char* code) { + *error = code; + return false; + } + + ConstraintReviewItem* find(const std::string& requirementId) { + auto it = items_.find(requirementId); + if (it == items_.end()) return nullptr; + return &it->second; + } +}; diff --git a/editor/tests/step577_test.cpp b/editor/tests/step577_test.cpp new file mode 100644 index 0000000..25f18cb --- /dev/null +++ b/editor/tests/step577_test.cpp @@ -0,0 +1,178 @@ +// Step 577: Architect Review Surface for Intake (12 tests) + +#include "ArchitectReviewSurface.h" + +#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 NormalizedRequirement c(const std::string& id, const std::string& text) { + NormalizedRequirement r; + r.requirementId = id; + r.kind = NormalizedRequirementKind::Constraint; + r.normalizedText = text; + r.anchor = "constraints"; + r.sourceLine = 1; + return r; +} + +void test_load_constraints_success() { + TEST(load_constraints_success); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load should succeed"); + CHECK(review.items().size() == 1, "one review item expected"); + PASS(); +} + +void test_load_constraints_rejects_empty_constraint_set() { + TEST(load_constraints_rejects_empty_constraint_set); + ArchitectReviewSurface review; + std::string error; + CHECK(!review.loadConstraints({}, &error), "load should fail"); + CHECK(error == "no_constraints_to_review", "wrong error"); + PASS(); +} + +void test_approve_sets_decision_and_reviewer() { + TEST(approve_sets_decision_and_reviewer); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load failed"); + CHECK(review.approve("constraint-1", "architect-a", &error), "approve failed"); + const auto items = review.items(); + CHECK(items[0].decision == ReviewDecision::Approved, "decision mismatch"); + CHECK(items[0].reviewer == "architect-a", "reviewer mismatch"); + PASS(); +} + +void test_modify_sets_reviewed_text() { + TEST(modify_sets_reviewed_text); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load failed"); + CHECK(review.modify("constraint-1", "must be deterministic and offline", "architect-a", &error), "modify failed"); + const auto items = review.items(); + CHECK(items[0].decision == ReviewDecision::Modified, "decision mismatch"); + CHECK(items[0].reviewedText == "must be deterministic and offline", "reviewed text mismatch"); + PASS(); +} + +void test_reject_sets_decision() { + TEST(reject_sets_decision); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load failed"); + CHECK(review.reject("constraint-1", "architect-a", &error), "reject failed"); + const auto items = review.items(); + CHECK(items[0].decision == ReviewDecision::Rejected, "decision mismatch"); + PASS(); +} + +void test_approve_fails_for_unknown_requirement() { + TEST(approve_fails_for_unknown_requirement); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load failed"); + CHECK(!review.approve("constraint-404", "architect-a", &error), "approve should fail"); + CHECK(error == "constraint_not_found", "wrong error"); + PASS(); +} + +void test_modify_fails_for_missing_reviewed_text() { + TEST(modify_fails_for_missing_reviewed_text); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load failed"); + CHECK(!review.modify("constraint-1", "", "architect-a", &error), "modify should fail"); + CHECK(error == "reviewed_text_missing", "wrong error"); + PASS(); +} + +void test_decisions_require_reviewer() { + TEST(decisions_require_reviewer); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "must be deterministic")}, &error), "load failed"); + CHECK(!review.approve("constraint-1", "", &error), "approve should fail"); + CHECK(error == "reviewer_missing", "wrong approve error"); + CHECK(!review.reject("constraint-1", "", &error), "reject should fail"); + CHECK(error == "reviewer_missing", "wrong reject error"); + PASS(); +} + +void test_summary_counts_pending_and_decided() { + TEST(summary_counts_pending_and_decided); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "a"), c("constraint-2", "b")}, &error), "load failed"); + CHECK(review.approve("constraint-1", "architect-a", &error), "approve failed"); + const auto summary = review.summarize(); + CHECK(summary.approved == 1, "approved count mismatch"); + CHECK(summary.pending == 1, "pending count mismatch"); + CHECK(!summary.allDecided, "allDecided should be false"); + PASS(); +} + +void test_summary_all_decided_when_no_pending() { + TEST(summary_all_decided_when_no_pending); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "a"), c("constraint-2", "b")}, &error), "load failed"); + CHECK(review.approve("constraint-1", "architect-a", &error), "approve failed"); + CHECK(review.modify("constraint-2", "b reviewed", "architect-a", &error), "modify failed"); + const auto summary = review.summarize(); + CHECK(summary.pending == 0, "pending should be zero"); + CHECK(summary.allDecided, "allDecided should be true"); + PASS(); +} + +void test_modify_overrides_prior_approval() { + TEST(modify_overrides_prior_approval); + ArchitectReviewSurface review; + std::string error; + CHECK(review.loadConstraints({c("constraint-1", "a")}, &error), "load failed"); + CHECK(review.approve("constraint-1", "architect-a", &error), "approve failed"); + CHECK(review.modify("constraint-1", "a updated", "architect-b", &error), "modify failed"); + const auto items = review.items(); + CHECK(items[0].decision == ReviewDecision::Modified, "decision should be modified"); + CHECK(items[0].reviewer == "architect-b", "reviewer should update"); + PASS(); +} + +void test_load_ignores_non_constraint_requirements() { + TEST(load_ignores_non_constraint_requirements); + ArchitectReviewSurface review; + std::string error; + NormalizedRequirement goal; + goal.requirementId = "goal-1"; + goal.kind = NormalizedRequirementKind::Goal; + goal.normalizedText = "goal"; + CHECK(!review.loadConstraints({goal}, &error), "load should fail because no constraints"); + CHECK(error == "no_constraints_to_review", "wrong error"); + PASS(); +} + +int main() { + std::cout << "Step 577: Architect Review Surface for Intake\n"; + + test_load_constraints_success(); // 1 + test_load_constraints_rejects_empty_constraint_set();// 2 + test_approve_sets_decision_and_reviewer(); // 3 + test_modify_sets_reviewed_text(); // 4 + test_reject_sets_decision(); // 5 + test_approve_fails_for_unknown_requirement(); // 6 + test_modify_fails_for_missing_reviewed_text(); // 7 + test_decisions_require_reviewer(); // 8 + test_summary_counts_pending_and_decided(); // 9 + test_summary_all_decided_when_no_pending(); // 10 + test_modify_overrides_prior_approval(); // 11 + test_load_ignores_non_constraint_requirements(); // 12 + + std::cout << "\nResults: " << passed << "/" << (passed + failed) << " passed\n"; + return failed == 0 ? 0 : 1; +} diff --git a/progress.md b/progress.md index c46b2f4..319ba7c 100644 --- a/progress.md +++ b/progress.md @@ -10661,3 +10661,38 @@ including requirement mapping and uncertainty scoring for architect intake. - `editor/src/ScopeMilestoneDecomposer.h` within header-size limit (`147` <= `600`) - `editor/tests/step576_test.cpp` within test-file size guidance (`201` lines) - Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md` + +### Step 577: Architect Review Surface for Intake +**Status:** PASS (12/12 tests) + +Implements an architect review control surface for extracted constraints, +enabling approve/modify/reject decisions before task generation. + +**Files added:** +- `editor/src/ArchitectReviewSurface.h` - review surface module: + - constraint review item loading from normalized requirements + - approve/modify/reject decision lifecycle with reviewer attribution + - reviewed text override support for modified constraints + - decision summary counters and completeness gating + - guard rails for missing reviewer/text/unknown requirement ids +- `editor/tests/step577_test.cpp` - 12 tests covering: + - review-load success/failure behavior + - approve/modify/reject behavior + - unknown-id and missing-input guard behavior + - review summary count/completeness behavior + - decision override behavior + - non-constraint filtering behavior + +**Files modified:** +- `editor/CMakeLists.txt` - `step577_test` target + +**Verification run:** +- `cmake -S editor -B editor/build-native` - PASS +- `cmake --build editor/build-native --target step577_test step576_test` - PASS +- `./editor/build-native/step577_test` - PASS (12/12) +- `./editor/build-native/step576_test` - PASS (12/12) regression coverage + +**Architecture gate check:** +- `editor/src/ArchitectReviewSurface.h` within header-size limit (`126` <= `600`) +- `editor/tests/step577_test.cpp` within test-file size guidance (`178` lines) +- Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md`