feat: add computed review-verdict ledger for MAP review outputs (#406) - #407
Merged
Conversation
Implements `write_review_verdict_ledger` step in map_step_runner — a pure closed decision table (`review_verdict_table.v1`) that normalizes Monitor/Predictor/Evaluator outputs into PROCEED/REVISE/BLOCK verdicts and writes `.map/<branch>/review-verdict-ledger.json` + `.md`. Key behaviour: - CRITICAL findings → BLOCK regardless of category - Security/correctness important findings → BLOCK - Other important/needs_investigation → REVISE - Pre-existing findings (`was_present_before_pr=true`) are tombstoned and excluded from computed_verdict - MEDIUM/HIGH without reach_evidence are downgraded to status="downgraded" (not active, not blocking) - Adversarial and compare_orderings review modes supported - Previous verdict carried forward into the journal entry Also: - Added `write_review_verdict_ledger` section to map-review SKILL.md (budget bumped 560→570; reference detail in review-reference.md) - Fixed 30 pre-existing E402 lint errors via pyproject.toml per-file-ignores - Fixed 3 F541 f-string-without-placeholders in _apply_verdict_table - Widened dict[str,object] → dict[str,Any] in all new functions to unblock Pyright iteration/indexing (0 errors, 0 warnings) - 30 new tests in tests/test_review_verdict_ledger.py (all passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLmcNUqk3b7Dp9tquhiHBo
Adds the Review Verdict Ledger entry to the main component catalog and the changelog delta section so future sessions see the feature before researching. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLmcNUqk3b7Dp9tquhiHBo
|
Warning Review limit reached
Next review available in: 38 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughChangesThe PR adds a Review Verdict Ledger for Review Verdict Ledger
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ReviewWorkflow
participant ReviewerEnvelopes
participant write_review_verdict_ledger
participant normalize_review_verdict
participant write_stage_gate
ReviewWorkflow->>ReviewerEnvelopes: persist Monitor, Predictor, and Evaluator JSON
ReviewWorkflow->>write_review_verdict_ledger: provide envelope files and review mode
write_review_verdict_ledger->>normalize_review_verdict: ingest findings and compute verdict
normalize_review_verdict-->>write_review_verdict_ledger: return computed_verdict and escalation status
write_review_verdict_ledger-->>ReviewWorkflow: return ledger result
ReviewWorkflow->>write_stage_gate: request computed_verdict
write_stage_gate->>write_review_verdict_ledger: verify ledger binding
write_stage_gate-->>ReviewWorkflow: write or reject the stage gate
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja (1)
7593-7596: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
_write_json_filefor the ledger JSON artifact.Every other JSON artifact in this module is written through
_write_json_file, which creates the parent directory and performs an atomic temp-file write plusreplace. The directwrite_textcall can leave a partially writtenreview-verdict-ledger.jsonif the process is interrupted, and a downstream reader then fails to parse it.♻️ Proposed refactor
json_path = branch_dir / "review-verdict-ledger.json" - json_path.write_text( - json.dumps(ledger, indent=2, ensure_ascii=True) + "\n", encoding="utf-8" - ) + _write_json_file(json_path, ledger)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja` around lines 7593 - 7596, Replace the direct json_path.write_text call in the ledger-writing flow with the module’s _write_json_file helper, passing the existing ledger data and JSON formatting options so review-verdict-ledger.json uses the established directory creation and atomic-write behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/map-review/SKILL.md:
- Around line 483-485: Pass adversarial findings to write_review_verdict_ledger
by conditionally supplying --adversarial-json "$AGGREGATED_FINDINGS_JSON" when
REVIEW_MODE_LABEL is adversarial. Apply this invocation change in
.claude/skills/map-review/SKILL.md lines 483-485 and
src/mapify_cli/templates_src/skills/map-review/review-reference.md.jinja lines
134-143; then regenerate
src/mapify_cli/templates/skills/map-review/review-reference.md lines 134-143
with make render-templates rather than editing the generated file directly.
In @.map/scripts/map_step_runner.py:
- Around line 7466-7477: Ensure closed-enum ledger fields remain schema-valid:
in .map/scripts/map_step_runner.py lines 7466-7477, update
normalize_review_verdict to allow-list source_agent values and fall back to
"adversarial"; in lines 20846-20854, validate --previous-verdict against
PROCEED, REVISE, and BLOCK, passing None for unknown values. Since
compare_orderings is a supported source, add it to the source_agent enum in
src/mapify_cli/schemas.py lines 1928-1934 and include it in the runner
allow-list. Apply these changes to the Jinja template and re-render the
generated copy.
- Around line 7378-7408: Keep findings downgraded from important or critical due
to missing reach_evidence active in the findings registry, with severity set to
needs_investigation and transition_reason recording the downgrade; update the
active-filter logic near the verdict application so status="downgraded" entries
are not removed, allowing _apply_verdict_table to produce REVISE. Update the
affected assertions in test_review_verdict_ledger.py to expect the revised
behavior.
In `@src/mapify_cli/schemas.py`:
- Around line 2018-2029: Update write_review_verdict_ledger() to validate the
constructed ledger against REVIEW_VERDICT_LEDGER_SCHEMA using the existing
validate_artifact() or jsonschema.validate() pattern used by
REVIEW_BUNDLE_SCHEMA before writing review-verdict-ledger.json and updating the
manifest.
In `@src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja`:
- Around line 7555-7580: Update _safe_parse and _safe_parse_list to record JSON
parse failures, including which payload failed, instead of silently returning
only empty values. After normalize_review_verdict produces the ledger, append
the collected parse_failures to ledger["not_verified"] and ensure
computed_verdict cannot remain PROCEED when any parse failure exists; preserve
existing behavior for valid and intentionally empty payloads.
In `@src/mapify_cli/templates_src/skills/map-review/SKILL.md.jinja`:
- Around line 482-485: Update the ledger invocation around
write_review_verdict_ledger to derive the accepted mode label from the executed
REVIEW_MODE, rather than the undefined REVIEW_MODE_LABEL. Construct
mode-specific arguments so adversarial mode passes --adversarial-json
"$AGGREGATED_FINDINGS_JSON", lightweight mode passes only Monitor output, and
cross-AI and compare-orderings modes pass their corresponding contract inputs;
preserve normal mode’s reviewer inputs.
In `@src/mapify_cli/templates/skills/map-review/SKILL.md`:
- Around line 480-488: Clarify the verdict authority between the “Final Verdict”
section, the computed_verdict from the decision table, and the $FINAL_VERDICT
passed to write_stage_gate. Add one sentence stating which value is
authoritative when they disagree, ensuring the stage gate and persisted review
verdict ledger use the same verdict.
---
Nitpick comments:
In `@src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja`:
- Around line 7593-7596: Replace the direct json_path.write_text call in the
ledger-writing flow with the module’s _write_json_file helper, passing the
existing ledger data and JSON formatting options so review-verdict-ledger.json
uses the established directory creation and atomic-write behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2229d497-d5c2-4b8d-a94c-a0a8dd2bca79
📒 Files selected for processing (14)
.claude/skills/map-review/SKILL.md.claude/skills/map-review/review-reference.md.map/scripts/map_step_runner.pydocs/ARCHITECTURE.mdpyproject.tomlsrc/mapify_cli/schemas.pysrc/mapify_cli/templates/map/scripts/map_step_runner.pysrc/mapify_cli/templates/skills/map-review/SKILL.mdsrc/mapify_cli/templates/skills/map-review/review-reference.mdsrc/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinjasrc/mapify_cli/templates_src/skills/map-review/SKILL.md.jinjasrc/mapify_cli/templates_src/skills/map-review/review-reference.md.jinjatests/test_review_verdict_ledger.pytests/test_skills.py
Comment on lines
+2018
to
+2029
| "required": [ | ||
| "schema_version", | ||
| "branch", | ||
| "generated_at", | ||
| "findings_registry", | ||
| "not_verified", | ||
| "computed_verdict", | ||
| "verdict_table", | ||
| "journal", | ||
| ], | ||
| "additionalProperties": True, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find registration and validation usages of the new ledger schema.
set -euo pipefail
rg -n -C 4 'REVIEW_VERDICT_LEDGER_SCHEMA'
rg -n -C 3 'REVIEW_BUNDLE_SCHEMA|CONTEXT_USEFULNESS_SCHEMA' --glob '!src/mapify_cli/schemas.py'
rg -n -C 3 'validate_artifact|jsonschema|SCHEMA_REGISTRY|_SCHEMAS\b'Repository: azalio/map-framework
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== locate schemas.py =="
fd -a 'schemas\.py$' . || true
echo "== symbols in schemas.py =="
if [ -f src/mapify_cli/schemas.py ]; then
rg -n 'REVIEW_VERDICT_LEDGER_SCHEMA|REVIEW_BUNDLE_SCHEMA|CONTEXT_USEFULNESS_SCHEMA|SCHEMA_REGISTRY|_SCHEMAS|validate|rjsonschema|jsonschema|jsonschemas' src/mapify_cli/schemas.py || true
fi
echo "== schema registry usages outside schemas.py =="
rg -n -C 3 'SCHEMA_REGISTRY|_SCHEMAS\b|validate_artifact|jsonschema|jsonschemas|rjsonschema|schema_registry' . --glob '!src/mapify_cli/schemas.py' || trueRepository: azalio/map-framework
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact ledger references =="
rg -n -C 3 'REVIEW_VERDICT_LEDGER_SCHEMA|review_verdict_ledger|verdict_ledger|ledger\.json' . --glob '!src/mapify_cli/schemas.py' --glob '!tests/test_schemas.py' --glob '!tests/test_artifact_schemas.py' || true
echo "== reviewed symbols outside schemas.py =="
rg -n -C 3 'REVIEW_VERDICT_LEDGER_SCHEMA' src tests --glob '*.py' || true
rg -n -C 3 'verdict_ledger|verdict ledger|Review Verdict Ledger|review verdict ledger' . || true
echo "== schema definitions around ledger =="
sed -n '1800,1975p' src/mapify_cli/schemas.pyRepository: azalio/map-framework
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== imports and write function body =="
sed -n '7521,7645p' src/mapify_cli/templates/map/scripts/map_step_runner.py
echo "== ledger schema definition and tests =="
sed -n '1887,1945p' src/mapify_cli/schemas.py
sed -n '360,458p' tests/test_review_verdict_ledger.py
echo "== call sites with schema references =="
rg -n 'REVIEW_VERDICT_LEDGER_SCHEMA|validate_artifact|_schemas_mod|REVIEW_BUNDLE_SCHEMA|RUN_HEALTH_REPORT_SCHEMA' src/mapify_cli/templates/map/scripts/map_step_runner.py --glob '*.py'Repository: azalio/map-framework
Length of output: 11814
Validate the review verdict ledger against REVIEW_VERDICT_LEDGER_SCHEMA.
write_review_verdict_ledger() documents that the returned ledger conforms to this schema, writes review-verdict-ledger.json, and updates the manifest, but it never calls validate_artifact() or jsonschema.validate() on the ledger. The REVIEW_BUNDLE_SCHEMA path has an equivalent validation point; add one here so drift in required fields, enums, or additionalProperties does not land silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mapify_cli/schemas.py` around lines 2018 - 2029, Update
write_review_verdict_ledger() to validate the constructed ledger against
REVIEW_VERDICT_LEDGER_SCHEMA using the existing validate_artifact() or
jsonschema.validate() pattern used by REVIEW_BUNDLE_SCHEMA before writing
review-verdict-ledger.json and updating the manifest.
added 2 commits
August 10, 2026 10:55
![Mikhail [azalio] Petrov](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
The ledger computed a verdict but could not affect one, and three paths turned a CRITICAL finding into PROCEED. - Bind the gate. write_stage_gate review <verdict> is refused, and no gate file written, when <verdict> contradicts computed_verdict. On by default; MAP_REVIEW_LEDGER_ENFORCE=0 is the explicit opt-out. Other stages and the no-ledger case are untouched and reported as such. - Feed the ledger. Reviewer envelopes are captured to .map/<branch>/review-agent-<role>.json in a new Step A.2c and read via --monitor-file/--predictor-file/--evaluator-file/--adversarial-file. The previous wiring referenced $MONITOR_JSON and friends, which were never assigned, so every real run computed PROCEED on an empty registry. - Count downgraded findings. The table now consumes active AND downgraded rows; only tombstoned rows are excluded, and only minor findings may be tombstoned. A missing reach_evidence field, or a self-attested was_present_before_pr=true, downgrades severity to needs_investigation instead of deleting the finding. Neutralising a CRITICAL this way sets escalation_required and names the unproven claim in not_verified. - Treat missing or malformed reviewer output as a finding rather than as an absence of findings, so an unobserved review cannot read as a clean one. - Recover journal.previous_verdict from the ledger on disk when the caller omits it; a journal retyped each run is not a journal. - Drop the E402 per-file-ignores from pyproject.toml: E402 is not in the selected rule set, so they suppressed nothing and were unrelated to #406. Tests rewritten where they encoded the fail-open behaviour, plus coverage for file input, journal continuity and gate refusal. map-review's body budget moves to 605; the status table, heredoc example and enforcement hatch live in review-reference.md.
…refusal Closes the acceptance criteria left open by the previous commit. - Objection channels. `record_review_objection --finding-id --channel [--evidence]` is now the only supported way to contest a finding. quote_absent / wrong_category / different_version are checkable against the change and tombstone the row, but require evidence — naming a reason is not enough. unverifiable_context retains the finding and escalates to a human, so PROCEED becomes unavailable. no_new_fact retains it and repeats the previous verdict (`journal.repeated_verbatim`). Objections live in .map/<branch>/review-objections.json, are bound to the claim they were raised against so a stale one cannot drift onto another finding, and are one per finding so a registry cannot be worn down by repetition. - Escalation ceiling: escalation_required can no longer coexist with PROCEED. Reachable when only minor findings remain and one is contested on unverifiable context. - input_classification stops being constants. destination and executor_class are accepted and recorded; evidence_mode is derived from the run (independent_run when adversarial or cross-AI findings took part, structural otherwise). destination is deliberately NOT a table argument: no reachable branch turns on it, and an unreachable rule is dead code in a gate. - A review gate with no ledger at all is now refused rather than recorded as a gap. /map-review is the only writer of a review-stage gate and it always writes the ledger first, so a missing ledger means the closeout was skipped — the exact case a gate must not wave through. - CHANGELOG.md and docs/USAGE.md document the ledger, the binding and the channels; review-reference.md gains the channel table. The #388 spelling test opts out of the binding explicitly: it writes all three verdicts for one branch, which no single ledger could agree with.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
tests/test_review_verdict_ledger.py (3)
726-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a helper for the ledger path.
The literal
Path(".map/test-branch") / "review-verdict-ledger.json"repeats in several tests (also at lines 904-906, 996-998, 1011-1013, 1023-1025, 1035-1037). The path depends on thebranch_workspacefixture'schdirand on the branch nametest-branch. A small helper keeps the coupling in one place.♻️ Suggested helper
def _ledger_payload() -> dict: return json.loads( (Path(".map/test-branch") / "review-verdict-ledger.json").read_text(encoding="utf-8") )Then call
payload = _ledger_payload()in each test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_review_verdict_ledger.py` around lines 726 - 731, Introduce a shared _ledger_payload helper in the test module that reads and parses the ledger at the existing test-branch path. Replace each repeated Path(".map/test-branch") / "review-verdict-ledger.json" loading block, including the tests around the current assertions and the other identified locations, with calls to _ledger_payload(), preserving the existing payload assertions.
766-776: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a case for an unrecognized
MAP_REVIEW_LEDGER_ENFORCEvalue.The tests cover only
"0". The gate is documented as fail-closed. An operator can setMAP_REVIEW_LEDGER_ENFORCE=falseorMAP_REVIEW_LEDGER_ENFORCE=noand expect enforcement to stay on, or expect it to turn off. The current tests do not pin either behavior, so a future parser change can silently disable the gate.Add one test that sets a value other than
"0"and asserts the resultingledger_enforcement.💚 Suggested test
def test_review_gate_binding_stays_enforced_for_unrecognized_env( branch_workspace, monkeypatch ): monkeypatch.setenv("MAP_REVIEW_LEDGER_ENFORCE", "false") map_step_runner.write_review_verdict_ledger( monitor_json=json.dumps(_monitor_with_critical()), ) result = map_step_runner.write_stage_gate("review", "ready", "code-review-001.md", "") assert result["status"] == "error" assert not (branch_workspace / "review-gate.json").exists()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_review_verdict_ledger.py` around lines 766 - 776, Extend the review-gate tests around test_review_gate_binding_can_be_disabled_explicitly with a case using an unrecognized MAP_REVIEW_LEDGER_ENFORCE value such as "false". Assert fail-closed behavior: write_stage_gate returns status "error", reports enforcement as enabled if exposed, and does not create review-gate.json.
1028-1038: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe test does not prove derivation.
The test name states that
evidence_modeis derived and not asserted. The call passesreview_mode="adversarial"together withadversarial_json. If the runner readsreview_modedirectly, the assertion still passes. The test therefore cannot distinguish derivation from a caller-asserted mode.Drop
review_modefrom this call, or add a second call that passesreview_mode="adversarial"with no adversarial input and assertsevidence_modestays"structural".💚 Suggested change
map_step_runner.write_review_verdict_ledger( monitor_json=json.dumps(_monitor_no_issues()), adversarial_json=json.dumps( [{"severity": "minor", "category": "tests", "claim": "second opinion"}] ), - review_mode="adversarial", )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_review_verdict_ledger.py` around lines 1028 - 1038, Update the test around write_review_verdict_ledger to prove evidence_mode is derived from adversarial_json rather than copied from review_mode: remove review_mode="adversarial" from the existing call, or add a separate call using review_mode="adversarial" without adversarial input and assert evidence_mode remains "structural"..map/scripts/map_step_runner.py (1)
7905-7914: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument
destinationandexecutor_classin the docstring.Both parameters are new, both are exposed as CLI flags at lines 21389-21390, and both are recorded in
input_classification. TheArgs:block does not list them, so the accepted values fordestination(pre_commit|pr_review|ci|unknown) are not discoverable from the function.♻️ Proposed docstring addition
previous_verdict: Prior PROCEED|REVISE|BLOCK verdict string. When empty, it is recovered from the ledger already written for this branch. + destination: Where the reviewed change is headed + (pre_commit|pr_review|ci|unknown). Any other value is + recorded as "unknown". + executor_class: Model tier that produced the reviewer output, when known. + Recorded only. branch: Branch name override.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.map/scripts/map_step_runner.py around lines 7905 - 7914, Update the relevant function docstring’s Args section to document the destination and executor_class parameters, including destination’s accepted values (pre_commit|pr_review|ci|unknown), matching the CLI flags and input_classification usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.map/scripts/map_step_runner.py:
- Around line 7665-7668: The retention floor must cover both removal paths: in
.map/scripts/map_step_runner.py lines 7665-7668, update the operator objection
handling to escalate rather than tombstone when severity is in
_NON_TOMBSTONABLE_SEVERITIES; in .map/scripts/map_step_runner.py lines
7439-7479, add needs_investigation to that set. Apply both changes to
src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja at lines
7439-7479 and 7665-7668, then run make render-templates; do not directly edit
the generated src/mapify_cli/templates/map/scripts/map_step_runner.py sites at
lines 7439-7479 and 7665-7668.
In `@src/mapify_cli/templates_src/skills/map-review/review-reference.md.jinja`:
- Around line 213-222: Update the review-reference invocation in
src/mapify_cli/templates_src/skills/map-review/review-reference.md.jinja at
lines 213-222 so both adversarial and compare_orderings modes pass
--adversarial-file pointing to review-agent-adversarial.json; then re-render the
corresponding sections in .claude/skills/map-review/review-reference.md at lines
213-222 and src/mapify_cli/templates/skills/map-review/review-reference.md at
lines 213-222.
- Around line 146-180: Define and apply one consistent tombstone eligibility
rule in the review documentation: explicitly state whether evidence-backed
objection channels may tombstone important or critical findings, then align the
computed_verdict table and Contesting a finding channel effects accordingly.
Update the template source at
src/mapify_cli/templates_src/skills/map-review/review-reference.md.jinja:146-180,
re-render the corresponding sections in
.claude/skills/map-review/review-reference.md:146-180 and
src/mapify_cli/templates/skills/map-review/review-reference.md:146-180, and
document the same rule in CHANGELOG.md:11-12 and docs/USAGE.md:142-144.
In `@src/mapify_cli/templates_src/skills/map-review/SKILL.md.jinja`:
- Around line 349-364: Step A.2c does not create the reviewer envelope files
required by the verdict ledger. In
src/mapify_cli/templates_src/skills/map-review/SKILL.md.jinja lines 349-364, add
mkdir -p "$BRANCH_DIR" and a quoted-heredoc example that writes a reviewer
envelope to review-agent-<role>.json, then apply the same addition in
.claude/skills/map-review/SKILL.md lines 349-364. Do not edit
src/mapify_cli/templates/skills/map-review/SKILL.md lines 349-364 directly;
regenerate it with make render-templates.
- Around line 486-497: Reconcile the verdict flow across the cross-AI,
adversarial, and ledger phases: explicitly state which phases provide findings
to the ledger and how they affect computed_verdict. If cross-AI output cannot be
normalized into ledger findings, have that phase write its own gate and skip
ledger-based verdict computation, then update write_stage_gate and the
surrounding FINAL_VERDICT control flow so the documented enforcement matches
this behavior and no verdict is silently discarded.
- Around line 508-516: The mandatory ledger invocation must use the files and
review phase for the current run. In
src/mapify_cli/templates_src/skills/map-review/SKILL.md.jinja lines 508-516, add
the adversarial file when its envelope exists and derive --review-mode from the
phase that ran; apply the same change in .claude/skills/map-review/SKILL.md
lines 508-516. Do not edit src/mapify_cli/templates/skills/map-review/SKILL.md
lines 508-516 directly; regenerate it with make render-templates.
---
Nitpick comments:
In @.map/scripts/map_step_runner.py:
- Around line 7905-7914: Update the relevant function docstring’s Args section
to document the destination and executor_class parameters, including
destination’s accepted values (pre_commit|pr_review|ci|unknown), matching the
CLI flags and input_classification usage.
In `@tests/test_review_verdict_ledger.py`:
- Around line 726-731: Introduce a shared _ledger_payload helper in the test
module that reads and parses the ledger at the existing test-branch path.
Replace each repeated Path(".map/test-branch") / "review-verdict-ledger.json"
loading block, including the tests around the current assertions and the other
identified locations, with calls to _ledger_payload(), preserving the existing
payload assertions.
- Around line 766-776: Extend the review-gate tests around
test_review_gate_binding_can_be_disabled_explicitly with a case using an
unrecognized MAP_REVIEW_LEDGER_ENFORCE value such as "false". Assert fail-closed
behavior: write_stage_gate returns status "error", reports enforcement as
enabled if exposed, and does not create review-gate.json.
- Around line 1028-1038: Update the test around write_review_verdict_ledger to
prove evidence_mode is derived from adversarial_json rather than copied from
review_mode: remove review_mode="adversarial" from the existing call, or add a
separate call using review_mode="adversarial" without adversarial input and
assert evidence_mode remains "structural".
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f539998-27c7-47e0-a91f-1e8cb25dc44b
📒 Files selected for processing (15)
.claude/skills/map-review/SKILL.md.claude/skills/map-review/review-reference.md.map/scripts/map_step_runner.pyCHANGELOG.mddocs/ARCHITECTURE.mddocs/USAGE.mdsrc/mapify_cli/templates/map/scripts/map_step_runner.pysrc/mapify_cli/templates/skills/map-review/SKILL.mdsrc/mapify_cli/templates/skills/map-review/review-reference.mdsrc/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinjasrc/mapify_cli/templates_src/skills/map-review/SKILL.md.jinjasrc/mapify_cli/templates_src/skills/map-review/review-reference.md.jinjatests/test_map_step_runner.pytests/test_review_verdict_ledger.pytests/test_skills.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_skills.py
- docs/ARCHITECTURE.md
Six findings, all reproduced against the code before fixing. - One retention floor, enforced at both removal sites. The floor guarded the reviewer self-attestation path but not the operator objection path, so an evidence-backed objection could tombstone a CRITICAL and turn BLOCK into PROCEED. Objection evidence is free text that nothing verifies, so above `minor` a checkable channel now downgrades and sets escalation_required instead of removing. `needs_investigation` joins the floor: it means "severity not established", not "low severity", which is exactly the finding that must not be dropped. - Closed enums are enforced instead of copied through. An adversarial finding's `source_agent` is mapped onto the schema's own vocabulary (`compare_orderings` -> `ordering`, unknown -> `adversarial`) and an unrecognized `--previous-verdict` is recorded as none. The ledger the runner writes previously violated REVIEW_VERDICT_LEDGER_SCHEMA: `input_integrity` was missing from the transition_reason enum, and the PR's own compare-orderings test asserted a source_agent the schema rejects. - The schema is now verified rather than asserted. A test validates a produced ledger — exercising the pre-existing, no-reach-evidence, parse-failure and objection paths — against REVIEW_VERDICT_LEDGER_SCHEMA, and declares the fields added since: escalation_required, escalation_reasons, downgraded_count, downgraded_from, journal.repeated_verbatim. - Step A.2c had no command. It named a quoted heredoc without showing one and never created $BRANCH_DIR, so an agent following it literally wrote no envelopes and every ledger run recorded an unobserved review. It now carries mkdir plus a real heredoc. - The ledger invocation was wired for normal mode only: three fixed paths, no --adversarial-file, REVIEW_MODE_LABEL pinned to normal. Adversarial, cross-AI and compare-orderings runs produced three read errors and lost their findings. The call now builds its argument list from the envelopes that exist and takes the mode from the phase that ran. - No phase assigns a verdict any more. The adversarial phase writes its aggregated findings to review-agent-adversarial.json and lets the ledger compute; cross-AI presents its opinion and falls through to the in-session review, which matches ARCHITECTURE.md's "supplement, never a gate" rather than the skill's previous "set FINAL_VERDICT and skip". The tombstone rule is now stated once and identically in review-reference.md, docs/USAGE.md and CHANGELOG.md. map-review's body budget moves to 630.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
write_review_verdict_ledgerinmap_step_runner.py— a closed decision table (review_verdict_table.v1) that normalizes Monitor/Predictor/Evaluator outputs intoPROCEED/REVISE/BLOCKverdicts.map/<branch>/review-verdict-ledger.json(machine-readable audit trail) and.map/<branch>/review-verdict-ledger.md(human summary)## Write Review Verdict Ledgerstep to themap-reviewskill (SKILL.md), with full decision-table docs inreview-reference.md § Verdict Ledgerdocs/ARCHITECTURE.mdwith the new componentDecision table
Pre-existing findings (
was_present_before_pr=true) are tombstoned and excluded fromcomputed_verdict. MEDIUM/HIGH findings withoutreach_evidenceare downgraded tostatus="downgraded"(non-active, non-blocking).Modes
normal— Monitor + Predictor + Evaluator inputsadversarial— accepts--adversarial-jsonaggregated findingscross_ai— same schema, source labelled accordinglycompare_orderings— records both ordering runs in the journalChanges
src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja—normalize_review_verdict(),write_review_verdict_ledger(),_apply_verdict_table()(all new), plusdict[str,Any]type widening (0 Pyright errors) and F541 fixessrc/mapify_cli/templates_src/skills/map-review/SKILL.md.jinja— new mandatory section (budget bumped 560→570)src/mapify_cli/templates_src/skills/map-review/review-reference.md.jinja—§ Verdict Ledgerreference detailpyproject.toml— per-file-ignores for 30 pre-existing E402 errorstests/test_review_verdict_ledger.py— 30 new tests (all passing)tests/test_skills.py— budget constant updatedocs/ARCHITECTURE.md— component entry + changelog deltamake render-templatesTest plan
uv run pytest tests/ -q --ignore=tests/skills_eval --ignore=tests/integration— 4317 passed, 4 skippedmake check— ruff, mypy, pyright (0/0/0), lint-hooks, full pytest, check-render all greentests/test_review_verdict_ledger.py— 30 tests covering: CRITICAL→BLOCK, security/correctness important→BLOCK, pre-existing tombstoned, MEDIUM without reach_evidence downgraded, no-issues→PROCEED, minor→PROCEED, evaluator contradiction, adversarial mode, compare_orderings mode, previous verdict journal, schema fields, write functions for JSON/MD/manifestGenerated by Claude Code
Summary by CodeRabbit
New Features
/map-reviewbased on all reviewer findings and evidence.Workflow Improvements
PROCEED,REVISE, orBLOCKverdict by default.Documentation