Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions openspec/changes/adjudicate-apply-same/design.md
Original file line number Diff line number Diff line change
@@ -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 <str>` (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 <survivor> <absorbed>` (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).
76 changes: 76 additions & 0 deletions openspec/changes/adjudicate-apply-same/explore.md
Original file line number Diff line number Diff line change
@@ -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`.
103 changes: 103 additions & 0 deletions openspec/changes/adjudicate-apply-same/proposal.md
Original file line number Diff line number Diff line change
@@ -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 <survivor> <absorbed>` 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.
Loading