Step 577: add architect review surface
This commit is contained in:
Bill
@@ -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)
|
||||
|
||||
126
editor/src/ArchitectReviewSurface.h
Normal file
126
editor/src/ArchitectReviewSurface.h
Normal file
@@ -0,0 +1,126 @@
|
||||
#pragma once
|
||||
// Step 577: Architect Review Surface for Intake
|
||||
|
||||
#include "RequirementNormalizationConflictDetector.h"
|
||||
|
||||
#include <map>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
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<NormalizedRequirement>& 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<ConstraintReviewItem> items() const {
|
||||
std::vector<ConstraintReviewItem> out;
|
||||
for (const auto& kv : items_) out.push_back(kv.second);
|
||||
return out;
|
||||
}
|
||||
|
||||
private:
|
||||
std::map<std::string, ConstraintReviewItem> 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;
|
||||
}
|
||||
};
|
||||
178
editor/tests/step577_test.cpp
Normal file
178
editor/tests/step577_test.cpp
Normal file
@@ -0,0 +1,178 @@
|
||||
// Step 577: Architect Review Surface for Intake (12 tests)
|
||||
|
||||
#include "ArchitectReviewSurface.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 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;
|
||||
}
|
||||
35
progress.md
35
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`
|
||||
|
||||
Reference in New Issue
Block a user