chore(project): quality improvements (#242)

* chore(docs): update review pr command

* chore(quality): add coverage enforcement and check drifts

* fix(project): fix build and implement reviewer feedback

---------

Co-authored-by: bgagent <bgagent@noreply.github.com>

Author: krokoko

.abca/commands/review_pr.md

@@ -78,6 +78,24 @@ Then apply principal-architect judgment over the diff:
 - **Routing** — Changes should land in the right package per the AGENTS.md routing table
   (agent runtime in `agent/`, API/Lambdas in `cdk/`, CLI in `cli/`).
 
+**Human review heuristics (non-automatable)** — After automated agents, apply these four
+dimensions. They are where agent output often looks plausible but isn't. Flag blocking issues when
+a dimension is clearly violated:
+
+- **Proportionality** — Does complexity match the problem? (new abstraction/factory/"engine"
+  for a one-off → AI002/AI003; files >800 lines — is size essential or accreted?)
+- **Coherence** — Does it belong here? Same concept = same term across the repo? Parallel
+  structure with real substance, not copy-paste boilerplate (AI006)?
+- **Clarity** — Do names communicate intent? Does error handling surface failures or hide
+  them behind plausible defaults (AI004)? Magic values that should live in
+  `contracts/constants.json` (AI007)?
+- **Appropriateness** — Maintainable by this team? Integration code verified against *real*
+  API behavior, not only self-written mocks (AI001)? Tests assert what code *should* do,
+  not merely what it *does* (AI005)?
+
+Include a **Human heuristics** subsection in Stage 5 output: one bullet per dimension
+(pass or concern with `file:line` when applicable).
+
 ### Stage 4: Documentation — Did We Update It Where Needed?
 
 Documentation drift is a blocking concern on this repo. Check:
@@ -112,5 +130,7 @@ Summarize as a principal architect would in a PR review. Structure the output:
 7. **Review agents run** — List each plugin/agent you invoked (Stage 3) and, for any in-scope
    agent you omitted, the one-line reason. This section is required — its absence means the
    mandatory plugin step was skipped.
+8. **Human heuristics** — Proportionality, Coherence, Clarity, Appropriateness (pass or concern
+   per dimension; cite `file:line` when not pass).
 
 Be specific and actionable. Prefer concrete diffs over vague advice.

.github/workflows/build.yml

@@ -87,6 +87,14 @@ repos:
         files: ^(contracts/constants\.json$|agent/src/policy\.py$|cdk/src/handlers/shared/types\.ts$|cdk/src/constructs/blueprint\.ts$|scripts/check-constants-sync\.ts$)
         stages: [pre-commit]
 
+      - id: coverage-thresholds-sync
+        name: coverage threshold drift (contracts/coverage-thresholds.json)
+        entry: bash -lc 'cd "$(git rev-parse --show-toplevel)" && mise run check:coverage-thresholds-sync'
+        language: system
+        pass_filenames: false
+        files: ^(contracts/coverage-thresholds\.json$|cdk/package\.json$|cli/package\.json$|agent/(pyproject\.toml|mise\.toml)$|scripts/check-coverage-thresholds-sync\.ts$)
+        stages: [pre-commit]
+
       - id: monorepo-security-pre-push
         name: security scans (pre-push)
         entry: bash -lc 'cd "$(git rev-parse --show-toplevel)" && mise run hooks:pre-push:security'

.gitignore

@@ -54,6 +54,7 @@ Rules:
 - The PR title and description become the squash commit message, so keep them accurate throughout the review.
 - The CI workflow runs `mise run install` then `mise run build` (compile + lint + test + synth + security scans for all packages).
 - Iterate on review feedback by pushing new commits to the same branch. Maintainers squash-merge when approved.
+- For structured reviews (human or agent), use the [`review_pr` command](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/blob/main/.abca/commands/review_pr.md) — including the **human review heuristics** (Proportionality, Coherence, Clarity, Appropriateness) for smell dimensions automation cannot catch.
 
 ### PR checklist
 

.pre-commit-config.yaml

@@ -41,8 +41,8 @@ description = "Type check with ty"
 run = "uv run ty check"
 
 [tasks.test]
-description = "Run tests with pytest"
-run = "uv run pytest"
+description = "Run tests with pytest (coverage floor enforced; use `uv run pytest --no-cov` for focused runs)"
+run = "uv run pytest --cov=src --cov-branch --cov-fail-under=72 --cov-report=term-missing:skip-covered --cov-report=lcov:coverage/lcov.info --cov-report=json:coverage/coverage.json"
 
 # SECURITY
 

CONTRIBUTING.md

@@ -53,6 +53,7 @@ dev = [
     "ty",
     "pytest",
     "pygments==2.20.0",
+    "pytest-cov==7.1.0",
 ]
 
 [tool.ruff]
@@ -87,6 +88,17 @@ ignore = [
 testpaths = ["tests"]
 pythonpath = ["src"]
 
+[tool.coverage.run]
+branch = true
+source = ["src"]
+relative_files = true
+
+[tool.coverage.report]
+fail_under = 72
+precision = 2
+show_missing = true
+skip_covered = true
+
 [tool.ty.environment]
 python-version = "3.13"
 extra-paths = ["src"]