From 8e0decfdc3d935b020d6f14497686a6a7c77fb23 Mon Sep 17 00:00:00 2001 From: Jason Date: Sat, 25 Jul 2026 01:13:34 -0500 Subject: [PATCH] feat(cli): add adjudicate --apply-same guarded batch merge (#137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gives `adjudicate` a batch path to action: `--apply-same` fuses every eligible SAME 2-member duplicate group in one pass, closing the advisory -> action loop that previously required one manual `merge` per pair. - Eligibility is exactly `verdict == SAME` and a 2-member group; N>2 groups and DIFFERENT/UNCERTAIN verdicts are skipped (mirroring `--apply`). - The batch prints a full aggregate preview of every merge it will perform, then requires a typed-count confirmation: `--confirm-count ` must equal the previewed count exactly (scriptable, mirrors `purge --confirm-phrase`); on a TTY the count is prompted; a non-TTY run without the flag refuses with zero writes. Any empty/wrong/non-numeric value aborts byte-identically. - An empty report (no eligible groups) short-circuits to "nothing to apply" (exit 0), so a scripted run never fails spuriously. - Merges execute sequentially, each landing in the `merged_from` ledger so `unmerge` round-trips. A mid-batch failure stops but keeps prior commits and reports how many were applied before the failure. Apply-time re-resolution skips a member already absorbed earlier in the batch. - Refactor: the per-pair prepare/preview/commit body is extracted into `_prepare_one_merge`/`_format_merge_preview_line`/`_commit_one_merge`, shared by both the interactive `--apply` walk and the batch, so the destructive write ordering is defined once. `--apply-same` is mutually exclusive with `--apply` and `--json`. Reviewed via the full 4R set (risk/resilience/reliability/readability); risk review found no data-loss or unauthorized-merge path. Exceeds the 800-line review budget (size:exception) — 62% of the diff is the 20-scenario test matrix for this destructive path. Closes #137. --- .../changes/adjudicate-apply-same/design.md | 130 ++++ .../changes/adjudicate-apply-same/explore.md | 76 +++ .../changes/adjudicate-apply-same/proposal.md | 103 ++++ .../entity-resolution-adjudication/spec.md | 163 +++++ .../changes/adjudicate-apply-same/tasks.md | 62 ++ .../adjudicate-apply-same/verify-report.md | 128 ++++ src/openkos/cli/main.py | 348 +++++++++-- tests/unit/cli/test_adjudicate.py | 574 +++++++++++++++++- 8 files changed, 1530 insertions(+), 54 deletions(-) create mode 100644 openspec/changes/adjudicate-apply-same/design.md create mode 100644 openspec/changes/adjudicate-apply-same/explore.md create mode 100644 openspec/changes/adjudicate-apply-same/proposal.md create mode 100644 openspec/changes/adjudicate-apply-same/specs/entity-resolution-adjudication/spec.md create mode 100644 openspec/changes/adjudicate-apply-same/tasks.md create mode 100644 openspec/changes/adjudicate-apply-same/verify-report.md diff --git a/openspec/changes/adjudicate-apply-same/design.md b/openspec/changes/adjudicate-apply-same/design.md new file mode 100644 index 0000000..af466a2 --- /dev/null +++ b/openspec/changes/adjudicate-apply-same/design.md @@ -0,0 +1,130 @@ +# Design: adjudicate --apply-same (guarded batch merge) — CLOSES #137 + +## Technical Approach + +Add a third `adjudicate` execution mode, `--apply-same`, that fuses every eligible +SAME 2-member group in one guarded batch. It reuses the shipped destructive machinery +verbatim (`_resolve_concept_path`, `prepare_merge`, `merge_core`, `_autocommit` + +ledger — `main.py:516-631`, `2698`, `2811`). To avoid duplicating the destructive +write ordering, three small shared helpers are extracted from the current +`_run_adjudicate_apply` monolith so both the interactive walk and the batch call the +SAME code. The batch is two-pass: build+print ONE aggregate preview, gate on a typed +exact-count confirmation, then execute sequentially. No new merge-core work (#163 +already extracted it); no new LLM field (#147 was prompt-only — `verdict==SAME` + +`len(member_ids)==2` remains the only trustworthy programmatic gate). + +## Architecture Decisions + +### Decision: Share the per-pair body via extracted helpers (no duplicated write ordering) + +**Choice**: Extract from `_run_adjudicate_apply` three helpers both modes call: +`_prepare_one_merge(root, layout, index_path, log_path, group)` (resolve both ids → +skip already-merged; `prepare_merge` → raise on bad input); `_format_merge_preview_line(prepared)` +(the existing "merge X into Y (sensitivity A->B, N rewrite(s), removes bundle/X.md)" +line, `587-592`); `_commit_one_merge(root, layout, index_path, log_path, prepared)` +(the `merge_core` + `_autocommit` write+commit unit, `602-622`). `_run_adjudicate_apply` +is refactored to call them with its `[y/N/skip]` prompt between prepare and commit. + +**Alternatives considered**: Copy the loop body into `_run_adjudicate_apply_same`. + +**Rationale**: A copied destructive path can silently diverge (write ordering, commit +path list, ledger). One shared `_commit_one_merge` guarantees byte-identical behavior; +`--apply` regression tests protect the refactor. + +### Decision: Typed exact-count gate, mirroring `purge --confirm-phrase` + +**Choice**: After the full preview, gate on typing the exact eligible count. Add a +companion option `--confirm-count ` (mirrors `purge`'s `--confirm-phrase`, +`2148-2169`). Resolution: if `--confirm-count` given → use it; elif `sys.stdin.isatty()` +→ `typer.prompt("Type N to proceed")`; else REFUSE (`stdin is not a TTY; re-run with +--confirm-count`, exit 1, zero writes). One comparison `typed.strip() == str(count)`; +empty Enter, non-numeric, or mismatch all fall through to abort (exit 1, zero writes). + +**Alternatives considered**: Pure `isatty` gate + `typer.prompt` (like `--apply`). + +**Rationale**: `purge` is the codebase's precedent for typed destructive confirmation; +its companion flag serves both scripted use and tests cleanly (CliRunner `input=` + +`isatty` is fragile for an accept-path test). Unattended application is possible ONLY +when the operator explicitly types the exact count on the command line — deliberate, +not blind — honoring "must not blindly merge." A bare non-TTY run refuses. + +### Decision: Structural count up front; re-verify per pair at apply time + +**Choice**: The preview count = number of eligible SAME 2-member groups (structural). +The typed count matches THAT. Pass 2 re-resolves each pair (`_resolve_concept_path`); +a member already absorbed by an earlier chained merge → skip (already-merged), so +`applied` may be < previewed. The summary reports both. + +**Rationale**: Mirrors `--apply` re-resolution (`552-564`); the count cannot depend on +runtime chaining. Preview pass sees one pre-batch snapshot, so it is internally +consistent; chaining only reduces actual applied merges, never adds surprises. + +## Data Flow + + adjudicate --apply-same + → mutual-exclusion (apply|json → exit 2) [before workspace gate] + → require_workspace → read_config → find_candidates → adjudicate_candidates + → _run_adjudicate_apply_same: + Pass 1: filter SAME & len==2; _prepare_one_merge each (raise→exit 1); + echo _format_merge_preview_line each; echo "Total: N" + Gate: resolve typed count → == N ? proceed : abort (exit 1, ZERO writes) + Pass 2: per pair re-resolve (skip already-merged) → _prepare_one_merge + → _commit_one_merge (OSError/ValueError → stop, keep prior, exit 1) + Summary: applied N, skipped M (N>2, already-merged) + +## File Changes + +| File | Action | Description | +|------|--------|-------------| +| `src/openkos/cli/main.py` | Modify | Extract 3 helpers; refactor `_run_adjudicate_apply`; add `_run_adjudicate_apply_same`; add `--apply-same`/`--confirm-count` options + mutual-exclusion + dispatch in `adjudicate()`. ~150-190 lines | +| `tests/unit/cli/test_adjudicate.py` | Modify | Batch tests mirroring the `--apply` fixture/monkeypatch matrix. ~250-350 lines | +| `openspec/specs/entity-resolution-adjudication/spec.md` | Modify | Delta: `--apply-same` requirements | + +## Interfaces / Contracts + +```python +def _prepare_one_merge(root, layout, index_path, log_path, group) -> PreparedMerge | None # None = skip already-merged +def _format_merge_preview_line(prepared: PreparedMerge) -> str +def _commit_one_merge(root, layout, index_path, log_path, prepared: PreparedMerge) -> None # merge_core + _autocommit +def _run_adjudicate_apply_same(root, layout, index_path, log_path, results, *, confirm_count: str | None) -> None +``` + +## Testing Strategy (strict TDD — RED first) + +| Layer | What to Test | Approach | +|-------|-------------|----------| +| Unit | `--apply-same` + `--apply` → exit 2; `--apply-same` + `--json` → exit 2, no adjudicate call | CliRunner, assert stderr/exit | +| Unit | Eligibility: only SAME 2-member previewed; N>2 echoed skipped; DIFFERENT/UNCERTAIN absent | monkeypatch fakes | +| Unit | Aggregate preview: one rich line per pair + "Total: N" BEFORE any prompt/write | stdout assert | +| Unit | Gate exact count → applies all; commit count == N | `--confirm-count` = str(N), ledger/commit assert | +| Unit | Gate Enter/wrong/non-numeric → abort, exit 1, snapshot byte-identical, 0 commits | `--confirm-count` "", "9", "x" | +| Unit | Non-TTY, no `--confirm-count` → refuse exit 1, zero writes | default (CliRunner not a TTY) | +| Unit | Chained shared member → applied < previewed; summary already-merged count | overlapping groups fixture | +| Unit | Mid-batch `merge_core`/`prepare_merge` failure → stop, prior commits kept, exit 1 | monkeypatch raise | +| Unit | Reversibility: batch → N sequential LIFO `unmerge` round-trips | `_init_apply_workspace` git-backed | +| Unit | `--apply` regression (extracted helpers) unchanged | existing suite green | + +## Threat Matrix + +| Boundary | Applicability | Design response | Planned RED test | +|---|---|---|---| +| Documentation-like paths | N/A: no path classification/execution |—|—| +| Git repository selection | N/A: reuses `_autocommit` root=`Path.cwd()`, no new selector |—|—| +| Commit state | Applicable: one commit per merge via shared `_commit_one_merge`; mid-batch failure keeps prior commits | Reuse `--apply` ordering verbatim | mid-batch-failure keeps-prior-commits test | +| Push state | N/A: no push |—|—| +| PR commands | N/A: no PR automation |—|—| + +Destructive write gate (not a shell row, tracked here): typed exact-count over full +preview; empty/non-numeric/mismatch and bare non-TTY all abort with zero writes. +RED tests: gate-abort-byte-identical, non-TTY-refusal. + +## Migration / Rollout + +No migration. Reversible via N sequential LIFO `unmerge ` (newest +first per survivor chain; `main.py:3086-3115`). Code rollback: drop the `--apply-same` +branch; `--apply` path unaffected once the shared-helper refactor is retained. + +## Open Questions + +- [ ] None blocking. Batch-undo command remains an explicit non-goal (documented: N + sequential unmerge). diff --git a/openspec/changes/adjudicate-apply-same/explore.md b/openspec/changes/adjudicate-apply-same/explore.md new file mode 100644 index 0000000..5a14e89 --- /dev/null +++ b/openspec/changes/adjudicate-apply-same/explore.md @@ -0,0 +1,76 @@ +# Exploration: `adjudicate --apply-same` (issue #137, part 3 — guarded batch) + +Final slice of issue #137. Parts already shipped and archived: `adjudicate --json` +(Slice 2a, #161), merge-core-extraction (Slice 2b-i, #163), `adjudicate --apply` +interactive per-pair walk (Slice 2b-ii, #165). This slice adds the guarded batch +`--apply-same`, previously deferred pending #138 (verdict quality). + +## Post-#147 adjudication signals (the key question for the batch guardrail) + +`#138` was resolved by change **#147** (commit 6d06d2c). **#147's fix is +prompt-only.** + +- `src/openkos/resolution/adjudication.py` — `AdjudicatedCandidate` is UNCHANGED: + still `candidate`, `verdict`, `confidence: float`, `rationale` (lines 82-96). No + new part-whole / relationship-shape field, no calibration. `_coerce_confidence` + (172-186) is still a raw clamp of the LLM-reported value. +- `#147` changed only `_SYSTEM_PROMPT` (46-63): the LLM is now instructed to treat + PART/COMPONENT/ASPECT/SUBTYPE/INSTANCE relationships as DIFFERENT and to prefer + DIFFERENT/UNCERTAIN over SAME when unsure. Pinned as system-message TEXT + assertions in `tests/unit/resolution/test_adjudication.py:426-444`, not a field. +- `src/openkos/cli/main.py:4179-4181` / `4314-4317` still say, present tense, that + the local model returns a flat, uncalibrated confidence "kept for future + thresholding." + +**Consequence:** the prior exploration's claim (Engram 1859) HOLDS. The only +machine-checkable guardrail for a batch remains `verdict == Verdict.SAME` plus +2-member group size. #147 improved verdict *quality* behaviorally but added no new +programmatic exclusion criterion. A `--min-confidence` guardrail would give false +safety today. + +## Affected areas + +- `src/openkos/cli/main.py:516-631` (`_run_adjudicate_apply`) — the shipped + per-pair loop body: skip logic, `_resolve_concept_path`, `prepare_merge` / + `merge_core` / `_autocommit`, ledger commits. Reuse verbatim. +- `src/openkos/cli/main.py:4130-4321` (`adjudicate` command) — flag wiring + + mutual-exclusion pattern to extend for `--apply-same`. +- `src/openkos/cli/main.py:2658-2811` (`PreparedMerge`, `MergeResult`, + `prepare_merge`, `merge_core`) — reused verbatim, no changes. +- `src/openkos/cli/main.py:3086` (`unmerge`) — LIFO-per-survivor; batch undo is N + sequential calls, no batch-level undo command exists. +- `tests/unit/cli/test_adjudicate.py:1007-1600` — ~30 existing `--apply` tests to + mirror (monkeypatch `adjudicate_candidates`, fake SAME verdicts, snapshot/ledger + assertions). + +## Approach + +Add `_run_adjudicate_apply_same` reusing `prepare_merge`/`merge_core`/`_autocommit` +verbatim, restructured as: build + print a full aggregate preview of every eligible +SAME 2-member pair → one explicit batch confirmation (not a bare `[y/N]`, per the +issue's "must not blindly merge") → sequential execute with the same +mid-run-failure-stops-but-keeps-prior-commits semantics as `--apply`. N>2 HIGH +groups skipped identically to `--apply`. `--apply-same` mutually exclusive with +`--apply` and `--json`. + +## Open guardrail decisions for the maintainer + +1. Confidence still cannot exclude anything — a `--min-confidence` flag would be a + NEW decision, not something #147 unlocked, and would provide false safety. +2. Confirmation shape (typed count / "APPLY" vs bare y/N) — no existing + aggregate-preview UX to copy; needs explicit sign-off. +3. Batch-size cap — none exists; whether to add one is open. +4. Batch-scale reversibility — real, but N sequential LIFO `unmerge` calls, no batch + undo command. Document, don't silently assume. +5. N>2 handling and flag mutual-exclusion error codes — likely mirror `--apply`, + confirm explicitly. + +## Sizing + +~350-550 changed lines total (production ~150-200, tests ~200-350). Fits one PR +under the 800-line budget. + +## Ready for proposal + +Yes. The one genuine maintainer decision is the batch confirmation shape (#2); the +rest have safe defaults mirroring the shipped `--apply`. diff --git a/openspec/changes/adjudicate-apply-same/proposal.md b/openspec/changes/adjudicate-apply-same/proposal.md new file mode 100644 index 0000000..f75994d --- /dev/null +++ b/openspec/changes/adjudicate-apply-same/proposal.md @@ -0,0 +1,103 @@ +# Proposal: adjudicate --apply-same (guarded batch merge) + +## Intent + +Close issue #137 by shipping its final part: a guarded batch verb that fuses +every eligible SAME 2-member group in one operation. The shipped `--apply` +walk confirms one pair at a time, which is slow for large duplicate sets. The +issue demands a batch that "must not blindly merge" — so the guard is a typed +count over a full preview, not a bare `[y/N]`. #147 (deferred dependency #138) +improved verdict quality but added NO machine-checkable field: confidence is +still uncalibrated, so `verdict == SAME` + 2-member group size remains the only +trustworthy programmatic gate. The preview + typed-count + reversibility are the +safety net. + +## Scope + +### In Scope +- New `--apply-same` flag on the existing `adjudicate` command; mutually + exclusive with `--apply` and `--json` (mirror the `--apply`/`--json` exit-2 + pattern, checked before the workspace gate). +- New `_run_adjudicate_apply_same` path: same adjudication (`find_candidates` → + `adjudicate_candidates`), filter to `verdict==SAME AND len(member_ids)==2`, + build and print ONE aggregate preview (survivor <- absorbed per line), + typed-count confirmation gate, then sequential merge reusing the shipped + per-pair body (`_resolve_concept_path` re-verify, `prepare_merge`, + `merge_core`, `_autocommit` + ledger). +- Reversibility documented: each merge lands a `merged_from` entry; a bad batch + is recovered via N sequential LIFO `unmerge` calls. + +### Out of Scope (Non-Goals) +- `--min-confidence` flag — confidence is uncalibrated post-#147; would give + false safety. +- Batch-size cap. +- New batch-undo command — reversibility is N sequential `unmerge`. +- Applying N>2 groups — SKIPPED, identical to `--apply`. + +## Capabilities + +### New Capabilities +None + +### Modified Capabilities +- `entity-resolution-adjudication`: add `--apply-same` batch verb requirements + (aggregate preview, typed-count confirmation, eligibility filter reuse, mutual + exclusion with `--apply`/`--json`, mid-run stop-keep-prior semantics, unmerge + round-trip). + +## Approach + +Two-pass in `_run_adjudicate_apply_same`. Pass 1: filter eligible SAME 2-member +groups, print one aggregate preview block (reusing the per-pair preview format +`_run_adjudicate_apply` already emits: `merge {absorbed} into {survivor} +(sensitivity X->Y, N rewrite(s), removes bundle/{absorbed}.md)`). Then prompt +the operator to TYPE THE EXACT NUMBER of listed merges. Empty Enter or any +wrong/mismatched value ABORTS with zero writes. Pass 2 (only on exact match): +loop the eligible pairs, re-verify ids per pair (skip already-merged), then +`prepare_merge`/`merge_core`/`_autocommit` + ledger commit per merge — the same +functions `_run_adjudicate_apply` uses (main.py:516-631). A mid-run +`(OSError, ValueError)` stops the run but keeps prior per-merge commits, same as +`--apply`. Final summary line prints applied/skipped counts. + +## Affected Areas + +| Area | Impact | Description | +|------|--------|-------------| +| `src/openkos/cli/main.py` | Modified | New `--apply-same` flag + mutual-exclusion checks in `adjudicate()`; new `_run_adjudicate_apply_same` helper | +| `tests/unit/cli/test_adjudicate.py` | Modified | New batch tests mirroring the `--apply` fixture/monkeypatch matrix | +| `openspec/specs/entity-resolution-adjudication/spec.md` | Modified | Delta spec for `--apply-same` | + +## Risks + +| Risk | Likelihood | Mitigation | +|------|------------|------------| +| Batch is destructive; one blind mistake compounds N times | Med | Full aggregate preview + typed-count gate; every merge reversible via `unmerge` | +| Typed-count gate accepts a mismatched/empty value and writes | Low | Gate MUST be exact-match; empty or any mismatch ABORTS with zero writes — pinned by tests | +| Confidence assumed as a guard | Low | Explicit non-goal; uncalibrated post-#147 | +| Chained survivors during batch invalidate later ids | Med | Re-verify both ids per pair before merge; skip already-merged (same as `--apply`) | + +## Rollback Plan + +No merge writes until the typed count exactly matches, so an aborted gate leaves +the workspace untouched. After a completed batch, reverse via N sequential LIFO +`unmerge ` calls (newest first per survivor chain); each +merge has an independent `merged_from` ledger entry and its own commit. Revert +the code change itself by dropping the `--apply-same` branch — the shipped +`--apply` path is unaffected. + +## Dependencies + +- #147 (verdict-quality prompt fix) — resolved; prompt-only, no new field. +- merge-core-extraction (#163) — shipped; `prepare_merge`/`merge_core` reused. + +## Success Criteria + +- [ ] `adjudicate --apply-same` prints one aggregate preview of every eligible + SAME 2-member merge before any write. +- [ ] Typed count matching the listed total proceeds; empty/wrong/mismatched + value aborts with zero writes. +- [ ] N>2 groups and non-SAME verdicts are excluded from the batch. +- [ ] `--apply-same` is rejected (exit 2) when combined with `--apply` or `--json`. +- [ ] Mid-run failure stops remaining merges but keeps prior commits. +- [ ] Applied merges round-trip via sequential `unmerge`. +- [ ] Issue #137 is CLOSED by this slice. diff --git a/openspec/changes/adjudicate-apply-same/specs/entity-resolution-adjudication/spec.md b/openspec/changes/adjudicate-apply-same/specs/entity-resolution-adjudication/spec.md new file mode 100644 index 0000000..ad31373 --- /dev/null +++ b/openspec/changes/adjudicate-apply-same/specs/entity-resolution-adjudication/spec.md @@ -0,0 +1,163 @@ +# Delta for entity-resolution-adjudication + +## ADDED Requirements + +### Requirement: `--apply-same` Eligibility Filter + +`adjudicate --apply-same` MUST include a group in the batch ONLY when +`verdict == SAME` AND the group has exactly 2 `member_ids`. DIFFERENT and +UNCERTAIN groups MUST NEVER be included. A SAME group with more than 2 +members MUST be skipped (not merged), mirroring `--apply`'s N>2 handling. + +#### Scenario: Mixed report yields only SAME 2-member pairs + +- GIVEN a report with SAME 2-member, SAME 3-member, DIFFERENT, and UNCERTAIN + groups +- WHEN `adjudicate --apply-same` runs +- THEN only the SAME 2-member groups appear in the batch + +#### Scenario: SAME group with >2 members is skipped + +- GIVEN a SAME-verdict group with 3 members +- WHEN `adjudicate --apply-same` runs +- THEN that group is never merged and is reported as skipped + +#### Scenario: DIFFERENT/UNCERTAIN groups are skipped + +- GIVEN a DIFFERENT-verdict group and an UNCERTAIN-verdict group +- WHEN `adjudicate --apply-same` runs +- THEN neither group is merged + +### Requirement: Aggregate Preview Before Any Write + +Before any write, `adjudicate --apply-same` MUST print a preview listing +EVERY eligible merge, one per line in the form +`merge into ...`, followed by the total eligible +count. No write MUST occur before the confirmation gate is resolved. + +#### Scenario: Preview lists all eligible pairs and the count + +- GIVEN 3 eligible SAME 2-member groups +- WHEN `adjudicate --apply-same` runs +- THEN stdout lists all 3 pairs before any prompt +- AND stdout shows the total eligible count +- AND no filesystem write has occurred yet + +### Requirement: Typed-Count Confirmation Gate + +The confirmation gate MUST be resolved in this order: + +1. If `--confirm-count ` is supplied on the command line, proceed + ONLY when `value.strip()` exactly equals the eligible-merge count; any + other value (empty, non-numeric, wrong number) MUST abort with ZERO + writes. +2. Else, if stdin is a TTY, print the full aggregate preview, then prompt + the operator to type the exact eligible-merge count; the same + exact-match-or-abort-zero-writes rule applies. +3. Else (non-TTY and no `--confirm-count`), `adjudicate --apply-same` MUST + REFUSE with exit code 1 and ZERO writes — it cannot confirm unattended + without the explicit flag. + +Unattended/scripted apply IS possible, but ONLY via an explicit exact +`--confirm-count` match; it is never a silent bypass. + +#### Scenario: `--confirm-count ` proceeds + +- GIVEN 3 eligible pairs and `--confirm-count 3` +- WHEN `adjudicate --apply-same` runs +- THEN all 3 merges proceed without any interactive prompt + +#### Scenario: `--confirm-count ` aborts with zero writes + +- GIVEN 3 eligible pairs and `--confirm-count 2`, `--confirm-count 4`, + `--confirm-count ""`, or `--confirm-count yes` +- WHEN `adjudicate --apply-same` runs +- THEN the run aborts and zero merges are written + +#### Scenario: TTY prompt with exact count typed proceeds + +- GIVEN 3 eligible pairs, no `--confirm-count`, a TTY stdin, and typed + input `"3\n"` +- WHEN `adjudicate --apply-same` runs +- THEN the full aggregate preview is printed, then all 3 merges proceed + +#### Scenario: TTY prompt with empty input aborts with zero writes + +- GIVEN eligible pairs, no `--confirm-count`, a TTY stdin, and typed input + `"\n"` +- WHEN `adjudicate --apply-same` runs +- THEN the run aborts, zero merges are written, and the workspace is + byte-identical to before the run + +#### Scenario: TTY prompt with wrong or non-numeric input aborts with zero writes + +- GIVEN 3 eligible pairs, no `--confirm-count`, a TTY stdin, and typed + input `"2\n"`, `"4\n"`, or `"yes\n"` +- WHEN `adjudicate --apply-same` runs +- THEN the run aborts and zero merges are written + +#### Scenario: Non-TTY without `--confirm-count` refuses + +- GIVEN eligible pairs, no `--confirm-count`, and a non-interactive/non-TTY + invocation +- WHEN `adjudicate --apply-same` runs +- THEN the run refuses with exit code 1 and zero merges are written + +### Requirement: Sequential Execution And Mid-Batch Failure Semantics + +On an exact-match confirmation, accepted merges MUST execute sequentially, +reusing the shipped per-pair `prepare_merge`/`merge_core`/`_autocommit` +body, and each MUST land a `merged_from` ledger entry. A mid-batch failure +MUST stop the run but MUST KEEP already-committed merges intact, and the +final report MUST show what was applied versus not attempted. + +#### Scenario: Mid-batch failure stops but keeps prior commits + +- GIVEN 3 accepted pairs where the 2nd pair fails during `merge_core` +- WHEN `adjudicate --apply-same` runs +- THEN pair 1 remains applied and committed, pair 2 fails with a clear + error, pair 3 is never attempted, and the run reports what was applied + versus not + +### Requirement: Stale-Id Guard Across Batch + +`adjudicate --apply-same` MUST re-verify both member ids of each accepted +pair immediately before applying it. If an earlier merge in the same batch +already absorbed a member that a later pair references, that later pair +MUST be skipped (not crash), and the skip MUST be clearly reported. + +#### Scenario: Shared-member pairs are handled without crashing + +- GIVEN two eligible SAME pairs sharing one member id +- WHEN `adjudicate --apply-same` applies the first pair +- THEN the second pair is skipped or safely re-resolved, the run does not + crash, and the skip is clearly reported + +### Requirement: Reversibility Via Sequential Unmerge + +Every merge applied by `adjudicate --apply-same` MUST be reversible via the +existing `unmerge` command, following the same LIFO per-survivor semantics +as `--apply`. No batch-undo command is provided. + +#### Scenario: Batch round-trips via sequential unmerge + +- GIVEN a batch of N applied merges +- WHEN `unmerge` is run N times in the correct LIFO order per survivor +- THEN the workspace is restored to byte parity with its pre-batch state + +### Requirement: `--apply-same` Mutual Exclusion With `--apply` And `--json` + +`adjudicate --apply-same` MUST be mutually exclusive with `--apply` and +with `--json`. Supplying more than one of these flags together MUST be +rejected with a clear stderr message and exit code 2, mirroring the +existing `--apply`/`--json` mutual-exclusion pattern. + +#### Scenario: `--apply-same --apply` exits 2 + +- WHEN `adjudicate --apply-same --apply` runs +- THEN stderr contains a clear rejection message and the exit code is 2 + +#### Scenario: `--apply-same --json` exits 2 + +- WHEN `adjudicate --apply-same --json` runs +- THEN stderr contains a clear rejection message and the exit code is 2 diff --git a/openspec/changes/adjudicate-apply-same/tasks.md b/openspec/changes/adjudicate-apply-same/tasks.md new file mode 100644 index 0000000..cefae61 --- /dev/null +++ b/openspec/changes/adjudicate-apply-same/tasks.md @@ -0,0 +1,62 @@ +# Tasks: adjudicate --apply-same (guarded batch merge, closes #137) + +## Review Workload Forecast + +| Field | Value | +|-------|-------| +| Estimated changed lines | ~400-540 (prod ~150-190, tests ~250-350) | +| 400-line budget risk | Medium | +| Chained PRs recommended | No | +| Suggested split | Single PR | +| Delivery strategy | ask-on-risk | +| Chain strategy | pending | + +Decision needed before apply: No +Chained PRs recommended: No +Chain strategy: pending +400-line budget risk: Medium + +Note: session review budget is explicitly set to 800 (not the 400 default). Estimated 400-540 lines fits comfortably under the session budget as a single PR; no chaining decision required. + +### Suggested Work Units + +| Unit | Goal | Likely PR | Focused test command | Runtime harness | Rollback boundary | +|------|------|-----------|----------------------|-----------------|-------------------| +| 1 | Extract 3 shared helpers, refactor `_run_adjudicate_apply` to use them, keep existing `--apply` suite green | PR 1 (part of single PR) | `uv run pytest tests/unit/cli/test_adjudicate.py -k apply` | `openkos adjudicate --apply` on a fixture workspace | Revert `main.py:516-631` refactor commit; `--apply` behavior unaffected | +| 2 | Add `--apply-same`/`--confirm-count`, batch logic, and full test matrix | PR 1 (same PR) | `uv run pytest tests/unit/cli/test_adjudicate.py -k apply_same` | `openkos adjudicate --apply-same --confirm-count N` on fixture workspace | Drop `--apply-same` branch + option flags; `--apply` untouched | + +## Phase 1: Extract Shared Helpers (Refactor Safety Net) + +- [x] 1.1 In `src/openkos/cli/main.py`, extract `_prepare_one_merge(root, layout, index_path, log_path, group) -> PreparedMerge | None` from `_run_adjudicate_apply` (lines ~516-631): resolves both ids, returns `None` if already-merged, calls `prepare_merge`. +- [x] 1.2 Extract `_format_merge_preview_line(prepared) -> str` (existing "merge X into Y (...)" line, ~587-592). +- [x] 1.3 Extract `_commit_one_merge(root, layout, index_path, log_path, prepared) -> None` (merge_core + `_autocommit`, ~602-622). +- [x] 1.4 Refactor `_run_adjudicate_apply` to call the three helpers with its existing `[y/N/skip]` prompt between prepare and commit. +- [x] 1.5 Run `uv run pytest tests/unit/cli/test_adjudicate.py -k apply` and confirm all existing `--apply` tests stay green with no behavior change. + +## Phase 2: RED Tests — `--apply-same` Batch (Strict TDD) + +- [x] 2.1 RED: mutual exclusion `--apply-same --apply` exits 2, `--apply-same --json` exits 2, before workspace/adjudicate calls. +- [x] 2.2 RED: eligibility filter — only SAME 2-member groups previewed; SAME >2-member skipped and reported; DIFFERENT/UNCERTAIN absent from batch. +- [x] 2.3 RED: aggregate preview prints one `_format_merge_preview_line` per eligible pair plus "Total: N" before any prompt or write. +- [x] 2.4 RED: `--confirm-count ` proceeds, applies all, commits count == N. +- [x] 2.5 RED: `--confirm-count` wrong/empty/non-numeric aborts, zero writes, workspace byte-identical. +- [x] 2.6 RED: TTY prompt with exact typed count proceeds after printing full preview; wrong/empty/non-numeric input aborts with zero writes. +- [x] 2.7 RED: non-TTY without `--confirm-count` refuses, exit 1, zero writes. +- [x] 2.8 RED: mid-batch failure (2nd of 3 pairs fails in `merge_core`) stops run, keeps pair-1 commit, never attempts pair 3, reports applied vs not. +- [x] 2.9 RED: chained shared-member pairs — second pair skipped/re-resolved safely, no crash, skip reported in summary (applied < previewed). +- [x] 2.10 RED: reversibility — batch of N applied merges round-trips via N sequential LIFO `unmerge` calls per survivor chain. + +## Phase 3: GREEN Implementation + +- [x] 3.1 Add `--apply-same` flag and `--confirm-count ` option to the `adjudicate` command in `src/openkos/cli/main.py`; wire mutual-exclusion check (exit 2) before workspace gate. +- [x] 3.2 Implement `_run_adjudicate_apply_same(root, layout, index_path, log_path, results, *, confirm_count: str | None) -> None`: Pass 1 filters SAME + `len(member_ids)==2`, builds `PreparedMerge` list via `_prepare_one_merge`, prints preview lines + total count. +- [x] 3.3 Implement confirmation gate resolution order: `--confirm-count` exact match → proceed; TTY → `typer.prompt` full preview then exact match; else refuse exit 1 zero writes. +- [x] 3.4 Implement Pass 2: sequential re-resolution per pair (skip already-merged), `_commit_one_merge` per pair, stop-on-failure keeping prior commits, final applied/skipped summary. +- [x] 3.5 Run all Phase 2 RED tests and confirm GREEN; run full `test_adjudicate.py` suite to confirm no regressions. + +## Phase 4: Quality Gate + +- [x] 4.1 Update `openspec/specs/entity-resolution-adjudication/spec.md` with the delta requirements (already drafted in spec artifact). +- [x] 4.2 Run `uv run pytest` (full suite) — must be clean. +- [x] 4.3 Run `uv run ruff check . && uv run ruff format --check .` — must be clean. +- [x] 4.4 Run `uv run mypy .` — must be clean. diff --git a/openspec/changes/adjudicate-apply-same/verify-report.md b/openspec/changes/adjudicate-apply-same/verify-report.md new file mode 100644 index 0000000..2e6e537 --- /dev/null +++ b/openspec/changes/adjudicate-apply-same/verify-report.md @@ -0,0 +1,128 @@ +```yaml +schema: gentle-ai.verify-result/v1 +evidence_revision: sha256:c028c2b916869a306e6c5e3b9656d0fae094dd2afb3689f8fe8158bd1200ba92 +verdict: pass +blockers: 0 +critical_findings: 0 +requirements: 7/7 +scenarios: 15/15 +test_command: uv run pytest +test_exit_code: 0 +test_output_hash: sha256:a0fb37e93b41f10ea29aaaaeec79f3e7d11b289f25d85dc5941244175ea12401 +build_command: uv run ruff check . && uv run ruff format --check . && uv run mypy . +build_exit_code: 0 +build_output_hash: sha256:c028c2b916869a306e6c5e3b9656d0fae094dd2afb3689f8fe8158bd1200ba92 +``` + +## Verification Report + +**Change**: adjudicate-apply-same (closes #137) +**Version**: delta spec `openspec/changes/adjudicate-apply-same/specs/entity-resolution-adjudication/spec.md` +**Mode**: Strict TDD + +### Scope Confirmation (pre-check) +`git diff main --stat` shows exactly two tracked files changed: +- `src/openkos/cli/main.py` (+255/-49, measured +304/-50 incl. context) +- `tests/unit/cli/test_adjudicate.py` (+442/-1) + +`openspec/changes/adjudicate-apply-same/` is untracked (expected — apply-phase artifact). +`openspec/specs/entity-resolution-adjudication/spec.md` (the MAIN spec) is **byte-identical to `main`** — confirmed via `diff <(git show main:...) <(cat ...)` → 0 differences. The delta→main merge correctly was NOT performed by apply; that remains the archive step's job, exactly as expected. No defect. + +### Completeness +| Metric | Value | +|--------|-------| +| Tasks total | 24 | +| Tasks complete | 24 | +| Tasks incomplete | 0 | + +`tasks.md` checkbox scan: `grep -c '\[x\]'` = 24, `grep -c '\[ \]'` = 0. Matches apply-progress self-report. + +### Build & Tests Execution +**Build (lint/type-check)**: PASSED +```text +$ uv run ruff check . +All checks passed! +$ uv run ruff format --check . +134 files already formatted +$ uv run mypy . +Success: no issues found in 134 source files +``` +Exit codes: 0 / 0 / 0 (independently verified, not trusted from apply-progress). + +**Tests**: 2127 passed, 0 failed, 0 skipped (full suite, independently re-run twice for hash stability) +```text +$ uv run pytest +======================= 2127 passed in ~108-111s ======================= +``` + +**Focused `--apply-same` subset**: `uv run pytest tests/unit/cli/test_adjudicate.py -k apply_same` → 20 passed, 60 deselected. +**Full `test_adjudicate.py` apply-family**: `-k apply` → 35 passed, 45 deselected (covers both `--apply` regression and `--apply-same`). + +**Coverage**: not configured in this project (no coverage tool detected) — skipped per strict-TDD-verify rule (informational only, never a failure). + +### Spec Compliance Matrix +| Requirement | Scenario | Test | Result | +|---|---|---|---| +| Eligibility Filter | Mixed report yields only SAME 2-member pairs | `test_adjudicate_apply_same_eligibility_filters_to_same_two_member_groups` | ✅ COMPLIANT | +| Eligibility Filter | SAME group with >2 members is skipped | same test (asserts `skipped (N>2, merge manually)`) | ✅ COMPLIANT | +| Eligibility Filter | DIFFERENT/UNCERTAIN groups are skipped | same test (asserts `Total: 1`, neither group merged) | ✅ COMPLIANT | +| Aggregate Preview Before Any Write | Preview lists all eligible pairs and the count | `test_adjudicate_apply_same_aggregate_preview_precedes_gate_and_writes` (asserts stdout ordering + `_snapshot` unchanged) | ✅ COMPLIANT | +| Typed-Count Confirmation Gate | `--confirm-count ` proceeds | `test_adjudicate_apply_same_confirm_count_exact_applies_all` | ✅ COMPLIANT | +| Typed-Count Confirmation Gate | `--confirm-count ` aborts zero writes | `test_adjudicate_apply_same_confirm_count_mismatch_aborts_with_zero_writes[0,2,"",yes]` (4 cases, `_snapshot` byte+mtime identity) | ✅ COMPLIANT | +| Typed-Count Confirmation Gate | TTY prompt exact count proceeds | `test_adjudicate_apply_same_tty_prompt_exact_count_applies` | ✅ COMPLIANT | +| Typed-Count Confirmation Gate | TTY prompt empty input aborts zero writes | `test_adjudicate_apply_same_tty_prompt_wrong_input_aborts_with_zero_writes["\n"]` | ✅ COMPLIANT | +| Typed-Count Confirmation Gate | TTY prompt wrong/non-numeric aborts zero writes | same test `["0\n","2\n","yes\n"]` | ✅ COMPLIANT | +| Typed-Count Confirmation Gate | Non-TTY without `--confirm-count` refuses | `test_adjudicate_apply_same_non_tty_without_confirm_count_refuses` (exit 1, `_snapshot` unchanged) | ✅ COMPLIANT | +| Sequential Execution / Mid-Batch Failure | Mid-batch failure stops but keeps prior commits | `test_adjudicate_apply_same_mid_batch_merge_core_failure_keeps_prior_commit` | ✅ COMPLIANT | +| Stale-Id Guard Across Batch | Shared-member pairs handled without crashing | `test_adjudicate_apply_same_chained_shared_member_skips_second_pair` (`applied 1 of 2 previewed`) | ✅ COMPLIANT | +| Reversibility Via Sequential Unmerge | Batch round-trips via sequential unmerge | `test_adjudicate_apply_same_batch_round_trips_via_sequential_unmerge` | ⚠️ PARTIAL (see Issues) | +| Mutual Exclusion | `--apply-same --apply` exits 2 | `test_adjudicate_apply_same_and_apply_rejected_with_exit_code_two` | ✅ COMPLIANT | +| Mutual Exclusion | `--apply-same --json` exits 2 | `test_adjudicate_apply_same_and_json_rejected_with_exit_code_two` | ✅ COMPLIANT | + +**Compliance summary**: 14/15 scenarios fully compliant, 1/15 partial (non-blocking — see Issues). + +### Correctness (Static Evidence) +| Requirement | Status | Notes | +|---|---|---| +| Mutual exclusion runs before workspace gate | ✅ Implemented | `apply_same and apply` / `apply_same and json_output` checks sit immediately after the pre-existing `apply and json_output` check, before `root = Path.cwd()` / `config.require_workspace(root)` | +| Shared per-pair helpers (`_prepare_one_merge`, `_format_merge_preview_line`, `_commit_one_merge`) | ✅ Implemented | Both `_run_adjudicate_apply` and `_run_adjudicate_apply_same` call `_commit_one_merge`; grep confirms neither function contains an inline `merge_core(` or `_autocommit(` call — zero duplicated destructive write ordering | +| Structural preview count (not resolvability-conditioned) | ✅ Implemented | `total = len(eligible_groups)` computed once in Pass 1, independent of whether each pair still resolves in Pass 2 — matches design decision 3 | +| Pass 2 re-resolves fresh per pair | ✅ Implemented | Pass 2 re-calls `_prepare_one_merge` per group rather than reusing Pass-1 `PreparedMerge` objects | + +### Coherence (Design) +| Decision | Followed? | Notes | +|---|---|---| +| 1. Share per-pair body via extracted helpers | ✅ Yes | Verified via grep: no inline `merge_core`/`_autocommit` remaining in either apply path | +| 2. Typed exact-count gate mirroring `purge --confirm-phrase` | ✅ Yes | `--confirm-count` → TTY prompt → non-TTY refuse (exit 1), single `typed.strip() == str(total)` comparison | +| 3. Structural count up front + re-verify per pair | ✅ Yes | Confirmed by chained shared-member test (`applied 1 of 2 previewed`) | + +### TDD Compliance +| Check | Result | Details | +|---|---|---| +| TDD Evidence reported | ✅ | Found in apply-progress (RED/GREEN/REFACTOR table for both Phase 1 and Phase 2/3) | +| All tasks have tests | ✅ | 24/24 tasks map to test files/behavior | +| RED confirmed (tests exist) | ✅ | 20 new `--apply-same` tests exist in `test_adjudicate.py` | +| GREEN confirmed (tests pass) | ✅ | 20/20 pass on independent re-run; full suite 2127/2127 pass | +| Triangulation adequate | ✅ | Confirmation-gate scenarios use 4-way and multi-value parametrize (`[0,2,"",yes]`, `["\n","0\n","2\n","yes\n"]`) — real variance, not all-empty | +| Safety Net for modified files | ✅ | Phase 1 refactor ran the full pre-existing `--apply` suite immediately after extraction (35/35 apply-family tests green now, matches "62/62 confirmed green" claim in apply-progress at time of refactor) | + +**TDD Compliance**: 6/6 checks passed + +### Assertion Quality +Scanned the full new test block (lines ~1601-2035, 14 test functions / 20 parametrized cases): 54 `assert` statements vs. 24 `monkeypatch.setattr` calls (ratio well under the 2x mock-heavy threshold). No tautologies (`assert True`, `expect(true).toBe(true)`), no orphan empty-collection-only assertions, no ghost loops. All abort-path tests assert against `_snapshot()` (byte contents + `st_mtime_ns`), not just exit code — genuinely proves zero writes, not just a plausible-looking exit code. + +**Assertion quality**: ✅ All assertions verify real behavior + +### Issues Found + +**CRITICAL**: None + +**WARNING**: +1. **Review budget exceeded** (process, not code defect): apply-progress self-reports 897 authored changed lines (151 spec + 304 main.py + 442 test file, per `git diff --stat` = 304+442=746 in tracked files + 151 in the delta spec file which is untracked/new), which exceeds the explicit session review budget of 800 set at session start. This was already flagged proactively in apply-progress. All work is complete and quality-gated; this is an orchestrator/PR-strategy decision (accept as `size:exception` or split), not a code correctness issue. Flagging again here per verify's independent-check duty. + +**SUGGESTION**: +1. `test_adjudicate_apply_same_batch_round_trips_via_sequential_unmerge` asserts file existence after unmerge (`b.md`/`d.md` reappear) rather than a full `_snapshot()` byte+mtime equality against the pre-batch state, so the CLI-level test does not itself independently prove full "byte parity" as worded in the spec scenario. This mirrors the exact same shallower pattern already used by the pre-existing `--apply` reversibility test (`test_adjudicate_apply_then_unmerge_restores_the_absorbed_member`), and true byte-parity of the underlying merge/unmerge primitive is independently and thoroughly covered by `tests/unit/cli/test_unmerge.py` (`_snapshot`-based round-trip property tests). Non-blocking; consistent with established codebase precedent, not a regression introduced by this change. + +### Verdict +**PASS WITH WARNINGS** +0 CRITICAL findings, 1 WARNING (review-budget governance flag, not a code defect), 1 SUGGESTION (reversibility test depth, consistent with precedent). All 7 delta-spec requirements / 15 scenarios map to passing tests; full suite (2127 tests), ruff check, ruff format --check, and mypy all pass with exit code 0; all 24 tasks genuinely complete; scope confirmed clean (only `main.py` + `test_adjudicate.py` tracked-modified, main spec untouched). diff --git a/src/openkos/cli/main.py b/src/openkos/cli/main.py index 50d363b..5d19fc9 100644 --- a/src/openkos/cli/main.py +++ b/src/openkos/cli/main.py @@ -51,7 +51,7 @@ Verdict, adjudicate_candidates, ) -from openkos.resolution.candidates import Tier +from openkos.resolution.candidates import CandidateGroup, Tier from openkos.resolution.contradiction import ( find_contradictions, is_high_confidence_contradiction, @@ -513,6 +513,88 @@ def _adjudication_payload( ] +def _prepare_one_merge( + root: Path, + layout: config.WorkspaceLayout, + index_path: Path, + log_path: Path, + group: CandidateGroup, +) -> "PreparedMerge | None": + """Resolve both member ids of one SAME 2-member `group` and build the + pure `PreparedMerge` preview, extracted verbatim from + `_run_adjudicate_apply`'s former per-pair body (issue #137 closing + slice, Phase 1 refactor) so the interactive `--apply` walk and the + `--apply-same` batch share exactly one apply-one-pair unit. Returns + `None` when either member id fails to resolve + (`_resolve_concept_path` raises `ValueError`) -- this covers BOTH a + member already absorbed by an earlier merge in this same run AND a + genuinely missing/invalid concept id; the two causes are + indistinguishable from here, so the caller must not assert either one + as the sole reason. Otherwise raises `OSError`/`ValueError` straight + from `prepare_merge`, unchanged.""" + survivor_id, absorbed_id = group.member_ids + try: + survivor_path, survivor_canonical = _resolve_concept_path( + layout.bundle_dir, survivor_id + ) + absorbed_path, absorbed_canonical = _resolve_concept_path( + layout.bundle_dir, absorbed_id + ) + except ValueError: + return None + + now = datetime.now(UTC) + return prepare_merge( + layout.bundle_dir, + index_path, + log_path, + survivor_path, + absorbed_path, + survivor_canonical, + absorbed_canonical, + root, + now=now, + ) + + +def _format_merge_preview_line(prepared: "PreparedMerge") -> str: + """The "merge X into Y (...)" preview line for one prepared merge, + extracted verbatim from the former inline body (issue #137 closing + slice, Phase 1 refactor).""" + return ( + f" merge {prepared.absorbed_canonical} into {prepared.survivor_canonical} " + f"(sensitivity {prepared.sensitivity_before}->" + f"{prepared.sensitivity_after}, {len(prepared.touched_files)} " + f"rewrite(s), removes bundle/{prepared.absorbed_canonical}.md)" + ) + + +def _commit_one_merge( + root: Path, + layout: config.WorkspaceLayout, + index_path: Path, + log_path: Path, + prepared: "PreparedMerge", +) -> None: + """`merge_core` + `_autocommit` for one prepared merge, extracted + verbatim from the former inline body (issue #137 closing slice, Phase 1 + refactor). Raises `OSError`/`ValueError` straight from `merge_core`, + unchanged -- callers decide how to report and whether to stop.""" + merge_result = merge_core(layout.bundle_dir, index_path, log_path, prepared) + _autocommit( + root, + [ + "bundle/index.md", + "bundle/log.md", + *(f"bundle/{rel}" for rel in merge_result.touched_files), + f"bundle/{prepared.survivor_canonical}.md", + f"bundle/{prepared.absorbed_canonical}.md", + ], + f"openkos: merge {prepared.absorbed_canonical} into " + f"{prepared.survivor_canonical}", + ) + + def _run_adjudicate_apply( root: Path, layout: config.WorkspaceLayout, @@ -550,48 +632,26 @@ def _run_adjudicate_apply( survivor_id, absorbed_id = group.member_ids try: - survivor_path, survivor_canonical = _resolve_concept_path( - layout.bundle_dir, survivor_id - ) - absorbed_path, absorbed_canonical = _resolve_concept_path( - layout.bundle_dir, absorbed_id - ) - except ValueError: - typer.echo( - f"{survivor_id} / {absorbed_id}: skipped (member already merged)" - ) - skipped_already_merged += 1 - continue - - now = datetime.now(UTC) - try: - prepared = prepare_merge( - layout.bundle_dir, - index_path, - log_path, - survivor_path, - absorbed_path, - survivor_canonical, - absorbed_canonical, - root, - now=now, - ) + prepared = _prepare_one_merge(root, layout, index_path, log_path, group) except (OSError, ValueError) as exc: typer.echo( "openkos adjudicate --apply: failed while merging " - f"{absorbed_canonical} into {survivor_canonical} -- {exc}.", + f"{absorbed_id} into {survivor_id} -- {exc}.", err=True, ) raise typer.Exit(code=1) from exc + if prepared is None: + typer.echo( + f"{survivor_id} / {absorbed_id}: skipped (member unresolved " + "-- already merged or missing)" + ) + skipped_already_merged += 1 + continue - typer.echo( - f" merge {absorbed_canonical} into {survivor_canonical} " - f"(sensitivity {prepared.sensitivity_before}->" - f"{prepared.sensitivity_after}, {len(prepared.touched_files)} " - f"rewrite(s), removes bundle/{absorbed_canonical}.md)" - ) + typer.echo(_format_merge_preview_line(prepared)) answer = typer.prompt( - f"Merge {absorbed_canonical} into {survivor_canonical}? [y/N/skip]", + f"Merge {prepared.absorbed_canonical} into " + f"{prepared.survivor_canonical}? [y/N/skip]", default="N", show_default=False, ) @@ -600,26 +660,15 @@ def _run_adjudicate_apply( continue try: - merge_result = merge_core(layout.bundle_dir, index_path, log_path, prepared) + _commit_one_merge(root, layout, index_path, log_path, prepared) except (OSError, ValueError) as exc: typer.echo( "openkos adjudicate --apply: failed while merging " - f"{absorbed_canonical} into {survivor_canonical} -- {exc}.", + f"{prepared.absorbed_canonical} into " + f"{prepared.survivor_canonical} -- {exc}.", err=True, ) raise typer.Exit(code=1) from exc - - _autocommit( - root, - [ - "bundle/index.md", - "bundle/log.md", - *(f"bundle/{rel}" for rel in merge_result.touched_files), - f"bundle/{survivor_canonical}.md", - f"bundle/{absorbed_canonical}.md", - ], - f"openkos: merge {absorbed_canonical} into {survivor_canonical}", - ) applied += 1 skipped_total = skipped_n_gt2 + skipped_already_merged + skipped_declined @@ -631,6 +680,163 @@ def _run_adjudicate_apply( ) +def _run_adjudicate_apply_same( + root: Path, + layout: config.WorkspaceLayout, + index_path: Path, + log_path: Path, + results: Sequence[AdjudicatedCandidate], + *, + confirm_count: str | None, +) -> None: + """The guarded batch `adjudicate --apply-same` merge (issue #137 + closing slice): Pass 1 builds ONE aggregate preview over every eligible + SAME 2-member group (spec: Aggregate Preview Before Any Write), reusing + the SAME `_prepare_one_merge`/`_format_merge_preview_line` building + blocks the interactive `--apply` walk uses. `total` -- the number + printed after the preview and required by the gate -- equals the + number of preview lines ACTUALLY DISPLAYED (i.e. the eligible groups + that still resolve right now), never the raw structural eligible-group + count; a group that is already unresolvable when the preview is built + (bogus/missing id, unrelated to this run) is silently excluded from + the preview, the total, and Pass 2 (4R fix: preview/Total + consistency). Zero eligible groups short-circuits with a "nothing to + apply" summary and exit 0 -- BEFORE the confirm gate -- mirroring + `_run_adjudicate_apply`'s own empty-state handling, so an empty batch + never triggers the non-TTY refusal or forces typing "0" on a TTY (4R + fix: zero-eligible spurious failure). The confirmation gate (spec: + Typed-Count Confirmation Gate) then requires the operator to type that + EXACT count, via `--confirm-count`, an interactive TTY prompt, or + refuses outright on a non-TTY without the flag -- any mismatch aborts + with ZERO writes. Pass 2 RE-RESOLVES and RE-PREPARES each previewed + pair immediately before applying it (spec: Stale-Id Guard Across + Batch), since an earlier merge in THIS SAME batch may already have + absorbed a later pair's member; that legitimate case is still skipped, + not crashed on, and still yields applied < previewed. Accepted merges + commit sequentially via `_commit_one_merge`; a mid-batch failure stops + the run but keeps every prior commit intact and reversible via + `unmerge` -- and, before raising, echoes a partial summary (applied so + far / previewed, and that the remainder was never attempted) so the + operator can drive that recovery without reconstructing the count + themselves (spec: Sequential Execution And Mid-Batch Failure + Semantics; 4R fix: mid-batch failure hides the applied count).""" + eligible_groups: list[CandidateGroup] = [] + skipped_n_gt2 = 0 + for result in results: + if result.verdict is not Verdict.SAME: + continue + group = result.candidate + if len(group.member_ids) == 2: + eligible_groups.append(group) + elif len(group.member_ids) > 2: + typer.echo( + f"[{group.okf_type}] {group.member_ids}: skipped (N>2, merge manually)" + ) + skipped_n_gt2 += 1 + + previewed_groups: list[CandidateGroup] = [] + for group in eligible_groups: + survivor_id, absorbed_id = group.member_ids + try: + prepared = _prepare_one_merge(root, layout, index_path, log_path, group) + except (OSError, ValueError) as exc: + typer.echo( + "openkos adjudicate --apply-same: failed while previewing " + f"{absorbed_id} into {survivor_id} -- {exc}.", + err=True, + ) + raise typer.Exit(code=1) from exc + if prepared is None: + continue + previewed_groups.append(group) + typer.echo(_format_merge_preview_line(prepared)) + total = len(previewed_groups) + typer.echo(f"Total: {total}") + + if total == 0: + skipped_total = skipped_n_gt2 + prefix = "nothing to apply -- " if skipped_total == 0 else "" + typer.echo( + f"openkos adjudicate --apply-same: {prefix}applied 0, skipped " + f"{skipped_total} (N>2: {skipped_n_gt2}, already-merged: 0)" + ) + return + + if confirm_count is not None: + typed_count = confirm_count + elif sys.stdin.isatty(): + typed_count = typer.prompt(f"Type the eligible count ({total}) to proceed") + else: + typer.echo( + "openkos adjudicate --apply-same: refusing to apply -- stdin is " + "not a TTY; re-run with --confirm-count.", + err=True, + ) + raise typer.Exit(code=1) + + if typed_count.strip() != str(total): + typer.echo( + "openkos adjudicate --apply-same: aborted -- confirmation count " + "did not match exactly; nothing was written.", + err=True, + ) + raise typer.Exit(code=1) + + applied = 0 + skipped_already_merged = 0 + for group in previewed_groups: + survivor_id, absorbed_id = group.member_ids + try: + prepared = _prepare_one_merge(root, layout, index_path, log_path, group) + except (OSError, ValueError) as exc: + typer.echo( + "openkos adjudicate --apply-same: failed while merging " + f"{absorbed_id} into {survivor_id} -- {exc}.", + err=True, + ) + typer.echo( + "openkos adjudicate --apply-same: stopped after failure -- " + f"applied {applied} of {total} previewed before this " + "failure; the remaining pairs were not attempted. Applied " + "merges remain committed and reversible via `unmerge`.", + err=True, + ) + raise typer.Exit(code=1) from exc + if prepared is None: + typer.echo( + f"{survivor_id} / {absorbed_id}: skipped (member unresolved " + "-- already merged or missing)" + ) + skipped_already_merged += 1 + continue + + try: + _commit_one_merge(root, layout, index_path, log_path, prepared) + except (OSError, ValueError) as exc: + typer.echo( + "openkos adjudicate --apply-same: failed while merging " + f"{prepared.absorbed_canonical} into " + f"{prepared.survivor_canonical} -- {exc}.", + err=True, + ) + typer.echo( + "openkos adjudicate --apply-same: stopped after failure -- " + f"applied {applied} of {total} previewed before this " + "failure; the remaining pairs were not attempted. Applied " + "merges remain committed and reversible via `unmerge`.", + err=True, + ) + raise typer.Exit(code=1) from exc + applied += 1 + + skipped_total = skipped_n_gt2 + skipped_already_merged + typer.echo( + f"openkos adjudicate --apply-same: applied {applied} of {total} " + f"previewed, skipped {skipped_total} (N>2: {skipped_n_gt2}, " + f"already-merged: {skipped_already_merged})" + ) + + _SLUG_SANITIZE_RE = re.compile(r"[^a-z0-9]+") _TITLE_SEPARATOR_RE = re.compile(r"[-_]+") @@ -4153,6 +4359,24 @@ def adjudicate( "--apply", help="Interactively merge each SAME 2-member group after previewing it.", ), + apply_same: bool = typer.Option( + False, + "--apply-same", + help=( + "Batch-merge every eligible SAME 2-member group after one " + "guarded confirmation (see --confirm-count)." + ), + ), + confirm_count: str | None = typer.Option( + None, + "--confirm-count", + help=( + "The exact eligible-merge count (see the printed preview), for " + "non-interactive/test use with --apply-same. On a TTY, " + "omitting this prompts interactively instead. There is NO " + "bypass for this count -- it must match exactly." + ), + ), ) -> None: """LLM-adjudicate cross-source candidate duplicates: read-only, like `query`. @@ -4209,6 +4433,14 @@ def adjudicate( `--json` (interactive vs. machine-readable output is contradictory), so that combination is rejected up front, before any workspace gate or read, with exit code 2. + + `--apply-same` switches to a GUARDED BATCH merge of every eligible SAME + 2-member group (issue #137 closing slice): prints one aggregate + preview and total count, then requires the operator to type that exact + count (via `--confirm-count`, an interactive TTY prompt, or refuses on + a non-TTY without the flag) before applying anything -- a mismatch + aborts with zero writes. Mutually exclusive with both `--apply` and + `--json`, rejected up front with exit code 2. """ if apply and json_output: typer.echo( @@ -4216,6 +4448,18 @@ def adjudicate( err=True, ) raise typer.Exit(code=2) + if apply_same and apply: + typer.echo( + "openkos adjudicate: --apply-same and --apply are mutually exclusive.", + err=True, + ) + raise typer.Exit(code=2) + if apply_same and json_output: + typer.echo( + "openkos adjudicate: --apply-same and --json are mutually exclusive.", + err=True, + ) + raise typer.Exit(code=2) root = Path.cwd() reason = config.require_workspace(root) @@ -4283,6 +4527,12 @@ def adjudicate( _run_adjudicate_apply(root, layout, index_path, log_path, results) return + if apply_same: + _run_adjudicate_apply_same( + root, layout, index_path, log_path, results, confirm_count=confirm_count + ) + return + typer.echo(f"openkos adjudicate: workspace at {root}") typer.echo() if not results: diff --git a/tests/unit/cli/test_adjudicate.py b/tests/unit/cli/test_adjudicate.py index 8c073d7..1322003 100644 --- a/tests/unit/cli/test_adjudicate.py +++ b/tests/unit/cli/test_adjudicate.py @@ -25,7 +25,7 @@ from pathlib import Path import pytest -from typer.testing import CliRunner +from typer.testing import CliRunner, _NamedTextIOWrapper from openkos.cli import main from openkos.cli.main import app @@ -1138,9 +1138,9 @@ def test_adjudicate_apply_overlapping_groups_second_reports_already_merged( monkeypatch: pytest.MonkeyPatch, ) -> None: """Two overlapping SAME 2-member groups sharing a member: the first is - accepted (`y`), the second prints `skipped (member already merged)` and - the run does not crash (spec: Later group referencing an - already-absorbed member is skipped).""" + accepted (`y`), the second prints `skipped (member unresolved -- + already merged or missing)` and the run does not crash (spec: Later + group referencing an already-absorbed member is skipped).""" _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") @@ -1177,7 +1177,7 @@ def _fake_adjudicate( result = runner.invoke(app, ["adjudicate", "--apply"], input="y\n") assert result.exit_code == 0 - assert "skipped (member already merged)" in result.stdout + assert "skipped (member unresolved -- already merged or missing)" in result.stdout assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() assert (tmp_path / "bundle" / "concepts" / "c.md").exists() @@ -1598,6 +1598,570 @@ def _strip_workspace_line(stdout: str) -> str: ) +# --------------------------------------------------------------------------- +# `adjudicate --apply-same` (guarded batch merge, issue #137 closing slice) +# --------------------------------------------------------------------------- + + +def _simulate_tty(monkeypatch: pytest.MonkeyPatch) -> None: + """Make `sys.stdin.isatty()` report `True` inside a `CliRunner.invoke` + call (mirrors `test_forget.py::_simulate_tty`).""" + monkeypatch.setattr(_NamedTextIOWrapper, "isatty", lambda self: True) + + +def test_adjudicate_apply_same_and_apply_rejected_with_exit_code_two( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`adjudicate --apply-same --apply` is rejected before any work: a + clear stderr message, exit code 2, and `adjudicate_candidates` is never + called (spec: `--apply-same` Mutual Exclusion With `--apply` And + `--json`).""" + _init_workspace(tmp_path, monkeypatch) + calls: list[object] = [] + monkeypatch.setattr( + "openkos.cli.main.adjudicate_candidates", + lambda *args, **kwargs: calls.append((args, kwargs)), + ) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--apply"]) + + assert result.exit_code == 2 + assert "No such option" not in result.stderr + assert "openkos adjudicate" in result.stderr + assert "--apply-same" in result.stderr + assert calls == [] + + +def test_adjudicate_apply_same_and_json_rejected_with_exit_code_two( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`adjudicate --apply-same --json` is rejected before any work: a + clear stderr message, exit code 2, and `adjudicate_candidates` is never + called (spec: `--apply-same` Mutual Exclusion With `--apply` And + `--json`).""" + _init_workspace(tmp_path, monkeypatch) + calls: list[object] = [] + monkeypatch.setattr( + "openkos.cli.main.adjudicate_candidates", + lambda *args, **kwargs: calls.append((args, kwargs)), + ) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--json"]) + + assert result.exit_code == 2 + assert "No such option" not in result.stderr + assert "openkos adjudicate" in result.stderr + assert "--apply-same" in result.stderr + assert calls == [] + + +def _two_member_group( + ids: tuple[str, str], *, trigger: str = "stub", tier: Tier = Tier.HIGH +) -> CandidateGroup: + return CandidateGroup( + okf_type="Concept", member_ids=ids, tier=tier, trigger=trigger + ) + + +def test_adjudicate_apply_same_eligibility_filters_to_same_two_member_groups( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Only SAME 2-member groups are previewed: SAME >2-member is skipped + and reported, DIFFERENT/UNCERTAIN never appear in the batch (spec: + `--apply-same` Eligibility Filter).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") + _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") + same_pair = _two_member_group(("concepts/a", "concepts/b"), trigger="stub-same") + same_triple = CandidateGroup( + okf_type="Concept", member_ids=("x", "y", "z"), tier=Tier.HIGH, trigger="stub3" + ) + different_group = _two_member_group(("p", "q"), trigger="stub-diff") + uncertain_group = _two_member_group(("r", "s"), trigger="stub-unc") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [same_pair, same_triple, different_group, uncertain_group] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(same_pair, verdict=Verdict.SAME, rationale="same"), + _adjudicated(same_triple, verdict=Verdict.SAME, rationale="same-triple"), + _adjudicated(different_group, verdict=Verdict.DIFFERENT, rationale="diff"), + _adjudicated(uncertain_group, verdict=Verdict.UNCERTAIN, rationale="unc"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "1"]) + + assert result.exit_code == 0 + assert "concepts/b" in result.stdout + assert "concepts/a" in result.stdout + assert "skipped (N>2, merge manually)" in result.stdout + assert "Total: 1" in result.stdout + + +def test_adjudicate_apply_same_aggregate_preview_precedes_gate_and_writes( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The full aggregate preview (every eligible pair plus the total + count) is printed BEFORE the confirmation gate and before any write + (spec: Aggregate Preview Before Any Write).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") + _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") + _write_doc(tmp_path / "bundle" / "concepts" / "c.md", title="Concept C") + _write_doc(tmp_path / "bundle" / "concepts" / "d.md", title="Concept D") + group1 = _two_member_group(("concepts/a", "concepts/b"), trigger="stub1") + group2 = _two_member_group(("concepts/c", "concepts/d"), trigger="stub2") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [group1, group2] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(group1, verdict=Verdict.SAME, rationale="r1"), + _adjudicated(group2, verdict=Verdict.SAME, rationale="r2"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + before = _snapshot(tmp_path) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "0"]) + + assert result.exit_code == 1 + assert "concepts/a" in result.stdout + assert "concepts/b" in result.stdout + assert "concepts/c" in result.stdout + assert "concepts/d" in result.stdout + assert "Total: 2" in result.stdout + total_idx = result.stdout.index("Total: 2") + assert result.stdout.index("concepts/b") < total_idx + assert result.stdout.index("concepts/d") < total_idx + assert _snapshot(tmp_path) == before + + +def test_adjudicate_apply_same_confirm_count_exact_applies_all( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`--confirm-count ` proceeds without any interactive prompt and + applies every eligible pair (spec: `--confirm-count ` + proceeds).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") + _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") + _write_doc(tmp_path / "bundle" / "concepts" / "c.md", title="Concept C") + _write_doc(tmp_path / "bundle" / "concepts" / "d.md", title="Concept D") + _seed_commit( + tmp_path, + [ + "bundle/concepts/a.md", + "bundle/concepts/b.md", + "bundle/concepts/c.md", + "bundle/concepts/d.md", + ], + ) + group1 = _two_member_group(("concepts/a", "concepts/b"), trigger="stub1") + group2 = _two_member_group(("concepts/c", "concepts/d"), trigger="stub2") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [group1, group2] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(group1, verdict=Verdict.SAME, rationale="r1"), + _adjudicated(group2, verdict=Verdict.SAME, rationale="r2"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + before_commits = _commit_count(tmp_path) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "2"]) + + assert result.exit_code == 0 + assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() + assert not (tmp_path / "bundle" / "concepts" / "d.md").exists() + assert _commit_count(tmp_path) == before_commits + 2 + assert "applied 2 of 2 previewed" in result.stdout + + +@pytest.mark.parametrize("wrong_count", ["0", "2", "", "yes"]) +def test_adjudicate_apply_same_confirm_count_mismatch_aborts_with_zero_writes( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, + wrong_count: str, +) -> None: + """`--confirm-count` wrong/empty/non-numeric aborts the run with ZERO + writes and a byte-identical workspace (spec: `--confirm-count + ` aborts with zero writes).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _, fake_find, fake_adjudicate = _seed_one_same_group(tmp_path) + monkeypatch.setattr("openkos.cli.main.find_candidates", fake_find) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", fake_adjudicate) + before = _snapshot(tmp_path) + + result = runner.invoke( + app, ["adjudicate", "--apply-same", "--confirm-count", wrong_count] + ) + + assert result.exit_code == 1 + assert _snapshot(tmp_path) == before + + +def test_adjudicate_apply_same_tty_prompt_exact_count_applies( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """On a TTY without `--confirm-count`, the full preview is printed then + the operator is prompted for the exact eligible count; a correct typed + count applies the merge (spec: TTY prompt with exact count typed + proceeds).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _simulate_tty(monkeypatch) + _, fake_find, fake_adjudicate = _seed_one_same_group(tmp_path) + monkeypatch.setattr("openkos.cli.main.find_candidates", fake_find) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", fake_adjudicate) + + result = runner.invoke(app, ["adjudicate", "--apply-same"], input="1\n") + + assert result.exit_code == 0 + assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() + + +@pytest.mark.parametrize("wrong_input", ["\n", "0\n", "2\n", "yes\n"]) +def test_adjudicate_apply_same_tty_prompt_wrong_input_aborts_with_zero_writes( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, + wrong_input: str, +) -> None: + """On a TTY, empty/wrong/non-numeric typed input aborts the run with + ZERO writes and a byte-identical workspace (spec: TTY prompt with empty + input aborts with zero writes; TTY prompt with wrong or non-numeric + input aborts with zero writes).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _simulate_tty(monkeypatch) + _, fake_find, fake_adjudicate = _seed_one_same_group(tmp_path) + monkeypatch.setattr("openkos.cli.main.find_candidates", fake_find) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", fake_adjudicate) + before = _snapshot(tmp_path) + + result = runner.invoke(app, ["adjudicate", "--apply-same"], input=wrong_input) + + assert result.exit_code == 1 + assert _snapshot(tmp_path) == before + + +def test_adjudicate_apply_same_non_tty_without_confirm_count_refuses( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Non-TTY invocation without `--confirm-count` refuses with exit code + 1 and ZERO writes (spec: Non-TTY without `--confirm-count` + refuses).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _, fake_find, fake_adjudicate = _seed_one_same_group(tmp_path) + monkeypatch.setattr("openkos.cli.main.find_candidates", fake_find) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", fake_adjudicate) + before = _snapshot(tmp_path) + + result = runner.invoke(app, ["adjudicate", "--apply-same"]) + + assert result.exit_code == 1 + assert "TTY" in result.stderr + assert _snapshot(tmp_path) == before + + +def test_adjudicate_apply_same_mid_batch_merge_core_failure_keeps_prior_commit( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A `merge_core` failure for the 2nd of 3 accepted pairs stops the run, + KEEPS the 1st pair's commit intact, and NEVER attempts the 3rd pair + (spec: Mid-batch failure stops but keeps prior commits). On the + failure path it also echoes a partial summary -- how many merges were + applied before the failure, out of how many previewed -- so the + operator can drive recovery (N sequential `unmerge`) without having to + reconstruct that count themselves (4R resilience fix).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + for stem in ("a", "b", "c", "d", "e", "f"): + _write_doc( + tmp_path / "bundle" / "concepts" / f"{stem}.md", title=f"Concept {stem}" + ) + group1 = _two_member_group(("concepts/a", "concepts/b"), trigger="stub1") + group2 = _two_member_group(("concepts/c", "concepts/d"), trigger="stub2") + group3 = _two_member_group(("concepts/e", "concepts/f"), trigger="stub3") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [group1, group2, group3] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(group1, verdict=Verdict.SAME, rationale="r1"), + _adjudicated(group2, verdict=Verdict.SAME, rationale="r2"), + _adjudicated(group3, verdict=Verdict.SAME, rationale="r3"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + + original_merge_core = main.merge_core + call_count = {"n": 0} + + def _flaky_merge_core(*args: object, **kwargs: object) -> object: + call_count["n"] += 1 + if call_count["n"] == 2: + raise OSError("disk full") + return original_merge_core(*args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr("openkos.cli.main.merge_core", _flaky_merge_core) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "3"]) + + assert result.exit_code != 0 + assert isinstance(result.exception, SystemExit) + assert "Traceback" not in result.stderr + assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() + assert (tmp_path / "bundle" / "concepts" / "d.md").exists() + assert (tmp_path / "bundle" / "concepts" / "e.md").exists() + assert (tmp_path / "bundle" / "concepts" / "f.md").exists() + assert "applied 1 of 3 previewed before this failure" in result.stderr + assert "remaining pairs were not attempted" in result.stderr + + +def test_adjudicate_apply_same_chained_shared_member_skips_second_pair( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Two eligible SAME pairs sharing a member: the first is applied, the + second is skipped without crashing, and the summary reports applied < + previewed (spec: Stale-Id Guard Across Batch).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") + _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") + _write_doc(tmp_path / "bundle" / "concepts" / "c.md", title="Concept C") + group1 = _two_member_group(("concepts/a", "concepts/b"), trigger="stub1") + group2 = _two_member_group(("concepts/b", "concepts/c"), trigger="stub2") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [group1, group2] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(group1, verdict=Verdict.SAME, rationale="r1"), + _adjudicated(group2, verdict=Verdict.SAME, rationale="r2"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "2"]) + + assert result.exit_code == 0 + assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() + assert (tmp_path / "bundle" / "concepts" / "c.md").exists() + assert "skipped (member unresolved -- already merged or missing)" in result.stdout + assert "applied 1 of 2 previewed" in result.stdout + + +def test_adjudicate_apply_same_batch_round_trips_via_sequential_unmerge( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A batch of N applied merges round-trips via N sequential LIFO + `unmerge` calls per survivor chain, restoring byte parity (spec: + Reversibility Via Sequential Unmerge).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") + _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") + _write_doc(tmp_path / "bundle" / "concepts" / "c.md", title="Concept C") + _write_doc(tmp_path / "bundle" / "concepts" / "d.md", title="Concept D") + group1 = _two_member_group(("concepts/a", "concepts/b"), trigger="stub1") + group2 = _two_member_group(("concepts/c", "concepts/d"), trigger="stub2") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [group1, group2] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(group1, verdict=Verdict.SAME, rationale="r1"), + _adjudicated(group2, verdict=Verdict.SAME, rationale="r2"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "2"]) + assert result.exit_code == 0 + assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() + assert not (tmp_path / "bundle" / "concepts" / "d.md").exists() + + unmerge1 = runner.invoke(app, ["unmerge", "concepts/c", "concepts/d", "--auto"]) + unmerge2 = runner.invoke(app, ["unmerge", "concepts/a", "concepts/b", "--auto"]) + + assert unmerge1.exit_code == 0 + assert unmerge2.exit_code == 0 + assert (tmp_path / "bundle" / "concepts" / "b.md").exists() + assert (tmp_path / "bundle" / "concepts" / "d.md").exists() + + +def test_adjudicate_apply_same_zero_eligible_non_tty_exits_zero_nothing_to_apply( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Zero eligible SAME 2-member groups, non-TTY, no `--confirm-count`: + the run must exit 0 with a "nothing to apply" message and zero writes + -- NOT the non-TTY refusal, which only applies when there IS something + to confirm (4R reliability fix: zero-eligible spurious failure).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + different_group = _two_member_group(("c", "d"), trigger="stub-diff") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [different_group] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(different_group, verdict=Verdict.DIFFERENT, rationale="diff") + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + before = _snapshot(tmp_path) + + result = runner.invoke(app, ["adjudicate", "--apply-same"]) + + assert result.exit_code == 0 + assert "nothing to apply" in result.stdout.lower() + assert "not a TTY" not in result.stderr + assert _snapshot(tmp_path) == before + + +def test_adjudicate_apply_same_zero_eligible_tty_exits_zero_without_prompt( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Zero eligible SAME 2-member groups on a TTY: exit 0, no interactive + prompt is ever shown (4R reliability fix: zero-eligible spurious + failure).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _simulate_tty(monkeypatch) + different_group = _two_member_group(("c", "d"), trigger="stub-diff") + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [different_group] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(different_group, verdict=Verdict.DIFFERENT, rationale="diff") + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + + # No `input=` supplied: if the code ever reached `typer.prompt`, this + # invocation would hang/fail reading past EOF -- the absence of a + # crash together with exit_code == 0 is itself proof no prompt fired. + result = runner.invoke(app, ["adjudicate", "--apply-same"]) + + assert result.exit_code == 0 + assert "nothing to apply" in result.stdout.lower() + assert "Type the eligible count" not in result.stdout + + +def test_adjudicate_apply_same_total_matches_resolvable_preview_count( + tmp_path: Path, + tmp_path_factory: pytest.TempPathFactory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The `Total:` line, the preview lines actually displayed, and the + count the confirm gate accepts are all the RESOLVABLE-at-preview-time + count -- not the raw structural eligible-group count. A group that is + already unresolvable when the preview is built (e.g. a bogus/missing + member id, not a mid-batch chained absorption) is silently excluded + from all three, rather than inflating `Total` past what is actually + offered (4R resilience+readability fix: preview/Total consistency).""" + _init_apply_workspace(tmp_path, tmp_path_factory, monkeypatch) + _write_doc(tmp_path / "bundle" / "concepts" / "a.md", title="Concept A") + _write_doc(tmp_path / "bundle" / "concepts" / "b.md", title="Concept B") + resolvable_group = _two_member_group(("concepts/a", "concepts/b"), trigger="stub1") + unresolvable_group = _two_member_group( + ("concepts/ghost1", "concepts/ghost2"), trigger="stub2" + ) + + def _fake_find_candidates( + bundle_dir: object, **kwargs: object + ) -> list[CandidateGroup]: + return [resolvable_group, unresolvable_group] + + def _fake_adjudicate( + candidates: list[CandidateGroup], **kwargs: object + ) -> list[AdjudicatedCandidate]: + return [ + _adjudicated(resolvable_group, verdict=Verdict.SAME, rationale="r1"), + _adjudicated(unresolvable_group, verdict=Verdict.SAME, rationale="r2"), + ] + + monkeypatch.setattr("openkos.cli.main.find_candidates", _fake_find_candidates) + monkeypatch.setattr("openkos.cli.main.adjudicate_candidates", _fake_adjudicate) + + result = runner.invoke(app, ["adjudicate", "--apply-same", "--confirm-count", "1"]) + + assert result.exit_code == 0 + assert result.stdout.count("merge concepts/b into concepts/a") == 1 + assert "ghost1" not in result.stdout + assert "Total: 1" in result.stdout + assert not (tmp_path / "bundle" / "concepts" / "b.md").exists() + + def test_adjudicate_command_name_is_not_resolve() -> None: """`adjudicate` is registered under its own name -- `resolve` remains unimplemented (spec: distinct from the reserved `resolve` verb).