diff --git a/docs/reference/review.en.md b/docs/reference/review.en.md index eccc636e..ecf4aed6 100644 --- a/docs/reference/review.en.md +++ b/docs/reference/review.en.md @@ -15,6 +15,31 @@ factlog reject Acme uses Datadog # pending → superseded (retired, kept for factlog accept Acme uses FastAPI --dry-run ``` +### Selecting reviewed facts by number + +`factlog review` assigns stable numbers to the pending triples and prints a +full `sha256:` snapshot digest. After a person has reviewed that exact output, +they can select one or more items without retyping a triple: + +```bash +factlog review +# [1] Acme / uses / FastAPI +# [2] Acme / uses / PostgreSQL +# snapshot: sha256:... +factlog accept --number 1 --number 2 --from sha256:... +factlog reject --number 2 --from sha256:... --dry-run +``` + +`--number` is repeatable and requires the digest printed by `review`. The +digest covers the complete normalized pending queue; if it is missing, +malformed, or stale, the command changes nothing and asks you to review again. +Only the default all-pending `factlog review` prints numbers and a digest; +`review --status ...` is a display filter and is not numeric approval evidence. +Numbers are only available with `--from`, so the existing positional triple +and `-` wildcard syntax remains unchanged and cannot be mixed with numbered +selection. A fresh snapshot proves that the human saw this queue; it is not an +authorization for a model to promote facts without a human decision. + `accept`/`reject` change **only pending rows**; a `confirmed`/`accepted`/ `superseded` match is reported and left untouched (use `factlog eject` to retire a non-pending fact). Both recompile `accepted.dl`. diff --git a/docs/reference/review.md b/docs/reference/review.md index d4cc53be..ea5e56f2 100644 --- a/docs/reference/review.md +++ b/docs/reference/review.md @@ -17,6 +17,30 @@ factlog reject Acme uses Datadog # pending → superseded (retired, kept for factlog accept Acme uses FastAPI --dry-run ``` +### 검토한 사실을 번호로 선택하기 + +`factlog review` 는 대기 트리플에 안정적인 번호를 붙이고 전체 큐의 `sha256:` +스냅샷 다이제스트를 출력합니다. 사람이 그 출력 자체를 검토한 뒤에는 트리플을 +다시 입력하지 않고 하나 이상을 선택할 수 있습니다. + +```bash +factlog review +# [1] Acme / uses / FastAPI +# [2] Acme / uses / PostgreSQL +# snapshot: sha256:... +factlog accept --number 1 --number 2 --from sha256:... +factlog reject --number 2 --from sha256:... --dry-run +``` + +`--number` 는 반복할 수 있으며 `review` 가 출력한 다이제스트가 반드시 필요합니다. +다이제스트는 정규화된 대기 큐 전체를 포함하므로, 없거나 형식이 잘못됐거나 큐가 +바뀌었으면 명령은 아무것도 변경하지 않고 다시 검토하라고 안내합니다. 번호 선택은 +기본 all-pending `factlog review` 에서만 번호와 다이제스트를 출력합니다. `review --status` +는 표시 필터이므로 번호 승인 근거로 사용할 수 없습니다. 번호 선택은 +`--from` 과 함께만 가능하므로 기존 위치 트리플과 `-` 와일드카드 문법은 변하지 않으며 +번호와 한 명령에서 섞을 수 없습니다. 새 스냅샷은 사람이 이 큐를 보았다는 근거이지, +모델이 사람의 결정 없이 사실을 승격할 권한은 아닙니다. + `accept`/`reject` 는 **대기(pending) 행만** 변경합니다. `confirmed`/`accepted`/ `superseded` 와 일치하는 항목은 보고만 되고 그대로 유지됩니다(대기 상태가 아닌 사실을 폐기하려면 `factlog eject` 를 사용). 둘 다 `accepted.dl` 을 재컴파일합니다. diff --git a/factlog/cli.py b/factlog/cli.py index 6e9c5da5..35d3cc83 100644 --- a/factlog/cli.py +++ b/factlog/cli.py @@ -794,6 +794,40 @@ def _triple_filter(terms: list[str]) -> dict[str, str] | None: return filt or None +def _review_queue(rows: list[dict[str, str]]) -> tuple[list[tuple[str, str, str]], str]: + """Return stable pending-fact numbers and a digest of their full snapshot. + + Numbers name unique NFC-normalized triples, sorted lexicographically. The + digest additionally covers every pending backing row (including source, + status, confidence and note), so accepting a number cannot silently act on + a queue that changed after it was reviewed. + """ + import hashlib + import json + import unicodedata + + from factlog.common import REVIEW_STATUSES + + def fld(row: dict[str, str], key: str) -> str: + return unicodedata.normalize("NFC", (row.get(key) or "").strip()) + + pending_rows = [row for row in rows if (row.get("status") or "").strip() in REVIEW_STATUSES] + triples = sorted({(fld(row, "subject"), fld(row, "relation"), fld(row, "object")) for row in pending_rows}) + snapshot_rows = sorted( + ( + fld(row, "subject"), fld(row, "relation"), fld(row, "object"), + fld(row, "source"), fld(row, "status"), fld(row, "confidence"), fld(row, "note"), + ) + for row in pending_rows + ) + payload = json.dumps( + {"domain": "factlog-review-snapshot-v1", "rows": snapshot_rows}, + ensure_ascii=False, + separators=(",", ":"), + ).encode("utf-8") + return triples, "sha256:" + hashlib.sha256(payload).hexdigest() + + def cmd_review(args: argparse.Namespace) -> int: """List facts awaiting a human decision (status candidate/needs_review). @@ -821,9 +855,12 @@ def nfc(s: str) -> str: if csv_path.is_file(): with csv_path.open(newline="", encoding="utf-8") as f: rows = list(csv.DictReader(f)) + queue, digest = _review_queue(rows) pending = [r for r in rows if (r.get("status") or "").strip() in want] if not pending: print(f"factlog review (KB: {target}): no pending facts ({'/'.join(sorted(want))})") + if args.status is None: + print(f" snapshot: {digest}") return 0 def fld(r: dict, k: str) -> str: @@ -833,9 +870,12 @@ def fld(r: dict, k: str) -> str: for r in pending: groups.setdefault((fld(r, "subject"), fld(r, "relation"), fld(r, "object")), []).append(r) + number = {triple: index for index, triple in enumerate(queue, start=1)} print(f"factlog review (KB: {target}): {len(groups)} pending fact(s), {len(pending)} row(s)") - for (s, rel, o), grp in groups.items(): - print(f" {s} / {rel} / {o}") + for (s, rel, o) in sorted(groups): + grp = groups[(s, rel, o)] + prefix = f"[{number[(s, rel, o)]}] " if args.status is None else "" + print(f" {prefix}{s} / {rel} / {o}") for r in sorted(grp, key=lambda r: fld(r, "source")): src = fld(r, "source") status = (r.get("status") or "").strip() @@ -845,6 +885,9 @@ def fld(r: dict, k: str) -> str: if note: print(f" note: {note}") print(" decide with: factlog accept (or: factlog reject ...)") + if args.status is None: + print(f" snapshot: {digest}") + print(f" or by reviewed number: factlog accept --number 1 --from {digest}") return 0 @@ -869,15 +912,42 @@ def nfc(s: str) -> str: target = Path(target_str) if not _require_kb(target, verb): return 1 - if len(args.terms) > 3: + numbers = list(args.numbers or []) + numbered = bool(numbers) + if numbered: + import re + + if args.terms: + print( + f"factlog {verb}: do not mix --number with a triple selector", + file=sys.stderr, + ) + return 2 + if len(set(numbers)) != len(numbers): + print(f"factlog {verb}: duplicate --number value; give each review number once", file=sys.stderr) + return 2 + if any(number < 1 for number in numbers): + print(f"factlog {verb}: --number must be a positive review number", file=sys.stderr) + return 2 + if args.from_digest is None or not re.fullmatch(r"sha256:[0-9a-f]{64}", args.from_digest): + print( + f"factlog {verb}: numeric selection needs the current review snapshot; no changes made. " + "Run factlog review again.", + file=sys.stderr, + ) + return 1 + elif args.from_digest is not None: + print(f"factlog {verb}: --from is only valid with one or more --number selectors", file=sys.stderr) + return 2 + elif len(args.terms) > 3: print( f"factlog {verb}: too many terms — give at most SUBJECT RELATION OBJECT " "(quote a value that contains spaces)", file=sys.stderr, ) return 2 - filt = _triple_filter(args.terms) - if filt is None: + filt = None if numbered else _triple_filter(args.terms) + if not numbered and filt is None: print( f"factlog {verb}: give at least one of SUBJECT RELATION OBJECT " "(use '-' to wildcard a position)", @@ -897,7 +967,29 @@ def nfc(s: str) -> str: def fld(r: dict, k: str) -> str: return nfc((r.get(k) or "").strip()) - matched = [r for r in rows if all(fld(r, k) == v for k, v in filt.items())] + selected_numbers: set[int] = set() + selected_triples: set[tuple[str, str, str]] = set() + if numbered: + queue, actual_digest = _review_queue(rows) + selected_numbers = set(numbers) + invalid = sorted(number for number in selected_numbers if number > len(queue)) + if invalid: + print( + f"factlog {verb}: review number(s) out of range: {', '.join(map(str, invalid))}; no changes made. " + "Run factlog review again.", + file=sys.stderr, + ) + return 2 + if args.from_digest != actual_digest: + print( + f"factlog {verb}: review snapshot is stale; no changes made. Run factlog review again.", + file=sys.stderr, + ) + return 1 + selected_triples = {queue[number - 1] for number in selected_numbers} + matched = [r for r in rows if (fld(r, "subject"), fld(r, "relation"), fld(r, "object")) in selected_triples] + else: + matched = [r for r in rows if all(fld(r, k) == v for k, v in filt.items())] if not matched: shown = ", ".join(f"{k}={v}" for k, v in filt.items()) print(f"factlog {verb}: no fact matches ({shown})", file=sys.stderr) @@ -929,7 +1021,11 @@ def fld(r: dict, k: str) -> str: out_fields = [*out_fields, "status"] changed = 0 for r in rows: - if all(fld(r, k) == v for k, v in filt.items()) and (r.get("status") or "").strip() in REVIEW_STATUSES: + is_selected = ( + (fld(r, "subject"), fld(r, "relation"), fld(r, "object")) in selected_triples + if numbered else all(fld(r, k) == v for k, v in filt.items()) + ) + if is_selected and (r.get("status") or "").strip() in REVIEW_STATUSES: r["status"] = new_status changed += 1 _atomic_write_csv(csv_path, rows, out_fields) @@ -2761,10 +2857,25 @@ def build_parser() -> argparse.ArgumentParser: ) _p.add_argument( "terms", - nargs="+", + nargs="*", metavar="TERM", help="SUBJECT [RELATION [OBJECT]] prefix; use '-' to wildcard a position", ) + _p.add_argument( + "--number", + dest="numbers", + action="append", + type=int, + metavar="N", + help="select reviewed pending fact number N (repeatable; requires --from)", + ) + _p.add_argument( + "--from", + dest="from_digest", + default=None, + metavar="SNAPSHOT", + help="select reviewed numeric item(s) only if this review snapshot digest still matches", + ) _p.add_argument("--dry-run", action="store_true", help="print the planned changes without modifying anything") _p.add_argument("--target", default=None, help="KB root (default: the active KB; see `factlog where`)") _p.set_defaults(func=_func) diff --git a/skills/factlog/SKILL.md b/skills/factlog/SKILL.md index 06a96f17..b3e71d4b 100644 --- a/skills/factlog/SKILL.md +++ b/skills/factlog/SKILL.md @@ -542,7 +542,14 @@ without hand-editing `candidates.csv`, use the review CLI: `factlog review` lists the pending queue, `factlog accept ` sets matching pending rows to `accepted`, and `factlog reject ...` sets them to `superseded` (both recompile `accepted.dl`; `-` wildcards a position). To -correct a fact's value, `factlog amend +select facts a human has just reviewed without retyping a triple, copy the +`sha256:` snapshot printed by `factlog review` into +`factlog accept --number N --from sha256:...` (repeat `--number` as needed). +The snapshot must still match and is evidence of the human's explicit choice; +it never authorizes the model to promote a fact on its own. Keep using the +triple form for any decision the human has not explicitly made. + +To correct a fact's value, `factlog amend --set-object ... [--set-subject/--set-relation/--set-note] [--accept]` rewrites it durably (updates both `candidates.csv` and the backing `runs/*.json`). These human decisions are preserved across re-merge. diff --git a/tests/test_review.sh b/tests/test_review.sh index 0abdadbb..b9bdbc25 100644 --- a/tests/test_review.sh +++ b/tests/test_review.sh @@ -7,6 +7,7 @@ # - accept promotes matching pending row(s) -> accepted (into accepted.dl) # - reject retires matching pending row(s) -> superseded (out of accepted.dl) # - a non-pending (confirmed/accepted/superseded) match is skipped -> rc 1 +# - numbered selection is snapshot-guarded, stable, atomic, and durable # - partial/wildcard terms; --dry-run no-op; no-match rc 1; no/extra term rc 2 # # Usage: bash tests/test_review.sh @@ -43,11 +44,64 @@ printf '%s' "$out" | grep -qF "2 pending fact(s)" && ok "review counts both pend printf '%s' "$out" | grep -qF "X / rel / Y" && printf '%s' "$out" | grep -qF "X / rel / Z" && ok "review lists candidate + needs_review" || bad "pending facts missing" printf '%s' "$out" | grep -qF "W / rel / V" && bad "review listed a confirmed fact" || ok "review omits the confirmed fact" printf '%s' "$out" | grep -qF "note: maybe" && ok "review shows the note" || bad "note missing" +printf '%s' "$out" | grep -qF "[1] X / rel / Y" && printf '%s' "$out" | grep -qF "[2] X / rel / Z" \ + && ok "review assigns stable sorted numbers" || bad "review numbers missing or unstable" +digest="$(printf '%s\n' "$out" | sed -n 's/^ snapshot: //p')" +[ "${#digest}" -eq 71 ] && ok "review emits a full sha256 snapshot digest" || bad "review snapshot digest missing" + +# Reordering equivalent CSV rows changes neither queue numbering nor snapshot. +printf '%s\n%s\n%s\n%s\n' "$H" \ + 'X,rel,Z,sources/a.md,needs_review,0.5,unsure' \ + 'X,rel,Y,sources/a.md,candidate,0.8,maybe' \ + 'W,rel,V,sources/a.md,confirmed,0.9,already engine input' > "$KB/facts/candidates.csv" +reordered="$($PYTHON -m factlog review --target "$KB" 2>&1)" +reordered_digest="$(printf '%s\n' "$reordered" | sed -n 's/^ snapshot: //p')" +[ "$digest" = "$reordered_digest" ] && printf '%s' "$reordered" | grep -qF "[1] X / rel / Y" \ + && ok "numbering and snapshot ignore equivalent row ordering" || bad "row ordering changed numbered snapshot" + +# Numeric selection is explicit, snapshot-guarded, and dry-run is a no-op. +before="$(cat "$KB/facts/candidates.csv")" +set +e; out="$($PYTHON -m factlog accept --number 1 --target "$KB" 2>&1)"; rc=$?; set -e +[ "$rc" -eq 1 ] && printf '%s' "$out" | grep -qF "Run factlog review again" && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] \ + && ok "numeric selection without snapshot rejects without writes" || bad "missing snapshot was not atomic" +set +e; out="$($PYTHON -m factlog accept --number 1 --from bad --target "$KB" 2>&1)"; rc=$?; set -e +[ "$rc" -eq 1 ] && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] && ok "malformed snapshot rejects without writes" || bad "malformed snapshot was not atomic" +"$PYTHON" -m factlog accept --number 1 --number 2 --from "$digest" --dry-run --target "$KB" >/dev/null 2>&1 +[ "$(cat "$KB/facts/candidates.csv")" = "$before" ] && ok "numeric --dry-run leaves candidates.csv unchanged" || bad "numeric --dry-run mutated state" +set +e; "$PYTHON" -m factlog accept --number 1 --number 1 --from "$digest" --target "$KB" >/dev/null 2>&1; rc=$?; set -e +[ "$rc" -eq 2 ] && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] && ok "duplicate review number rejects atomically" || bad "duplicate review number handling wrong" +set +e; "$PYTHON" -m factlog accept --number 3 --from "$digest" --target "$KB" >/dev/null 2>&1; rc=$?; set -e +[ "$rc" -eq 2 ] && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] && ok "out-of-range review number rejects atomically" || bad "out-of-range number handling wrong" +set +e; "$PYTHON" -m factlog accept X --number 1 --from "$digest" --target "$KB" >/dev/null 2>&1; rc=$?; set -e +[ "$rc" -eq 2 ] && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] && ok "number and triple selectors cannot be mixed" || bad "mixed selectors accepted" + +# Queue mutation makes a formerly valid number stale, with no partial write. +printf '%s\n' 'A,rel,B,sources/a.md,candidate,0.7,new row' >> "$KB/facts/candidates.csv" +before="$(cat "$KB/facts/candidates.csv")" +set +e; out="$($PYTHON -m factlog accept --number 1 --from "$digest" --target "$KB" 2>&1)"; rc=$?; set -e +[ "$rc" -eq 1 ] && printf '%s' "$out" | grep -qF "snapshot is stale" && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] \ + && ok "stale snapshot rejects atomically" || bad "stale snapshot was not atomic" + +# A fresh snapshot maps multi-number decisions to exactly the displayed facts. +out="$($PYTHON -m factlog review --target "$KB" 2>&1)" +digest="$(printf '%s\n' "$out" | sed -n 's/^ snapshot: //p')" +"$PYTHON" -m factlog accept --number 1 --number 3 --from "$digest" --target "$KB" >/dev/null 2>&1 +grep -q 'A,rel,B,sources/a.md,accepted,' "$KB/facts/candidates.csv" \ + && grep -q 'X,rel,Z,sources/a.md,accepted,' "$KB/facts/candidates.csv" \ + && grep -q 'X,rel,Y,sources/a.md,candidate,' "$KB/facts/candidates.csv" \ + && ok "multi-number selection changes exactly the numbered facts" || bad "multi-number selection changed wrong facts" + +# Keep the legacy review tests independent of the numbered-selection fixture. +KB="$(mktemp -d)/wiki"; seed "$KB" # --- review --status narrows ------------------------------------------------- out="$("$PYTHON" -m factlog review --status candidate --target "$KB" 2>&1)" printf '%s' "$out" | grep -qF "X / rel / Y" && ! printf '%s' "$out" | grep -qF "X / rel / Z" \ && ok "review --status candidate shows only candidate rows" || bad "--status filter wrong" +before="$(cat "$KB/facts/candidates.csv")" +! printf '%s' "$out" | grep -qF "snapshot:" && ! printf '%s' "$out" | grep -qF "[1] X / rel / Y" \ + && [ "$(cat "$KB/facts/candidates.csv")" = "$before" ] \ + && ok "filtered review cannot serve as numbered approval evidence" || bad "filtered review exposed numbered approval evidence" # --- accept --dry-run changes nothing ---------------------------------------- before="$(cat "$KB/facts/candidates.csv")" @@ -75,6 +129,14 @@ KB="$(mktemp -d)/wiki"; seed "$KB" "$PYTHON" -m factlog accept X --target "$KB" >/dev/null 2>&1 [ "$(grep -c ",accepted," "$KB/facts/candidates.csv")" -eq 2 ] && ok "subject-only accept promotes all pending for X" || bad "partial accept count wrong" +# A numeric-looking subject remains the legacy positional selector unless +# --number is explicitly present. +KB="$(mktemp -d)/wiki"; seed "$KB" +printf '%s\n%s\n' "$H" '1,rel,Y,sources/a.md,candidate,0.8,numeric subject' > "$KB/facts/candidates.csv" +"$PYTHON" -m factlog accept 1 --target "$KB" >/dev/null 2>&1 +grep -q '1,rel,Y,sources/a.md,accepted,' "$KB/facts/candidates.csv" \ + && ok "numeric subject keeps the legacy positional accept syntax" || bad "numeric subject was misread as review number" + # --- error paths ------------------------------------------------------------- set +e "$PYTHON" -m factlog accept nope nope nope --target "$KB" >/dev/null 2>&1; [ $? -eq 1 ] && ok "no-match accept rc 1" || bad "no-match rc wrong" @@ -100,11 +162,15 @@ printf 'a\n' > "$KB/sources/a.md" printf '[{"subject":"X","relation":"rel","object":"Y","source":"sources/a.md","status":"candidate","confidence":0.8,"note":""},{"subject":"Z","relation":"rel","object":"W","source":"sources/a.md","status":"candidate","confidence":0.8,"note":""}]\n' \ > "$KB/runs/r.json" "$PYTHON" "$PLUGIN_ROOT/tools/merge_candidates.py" --wiki "$KB" >/dev/null 2>&1 -"$PYTHON" -m factlog accept X rel Y --target "$KB" >/dev/null 2>&1 -"$PYTHON" -m factlog reject Z rel W --target "$KB" >/dev/null 2>&1 +out="$($PYTHON -m factlog review --target "$KB" 2>&1)" +digest="$(printf '%s\n' "$out" | sed -n 's/^ snapshot: //p')" +"$PYTHON" -m factlog accept --number 1 --from "$digest" --target "$KB" >/dev/null 2>&1 +out="$($PYTHON -m factlog review --target "$KB" 2>&1)" +digest="$(printf '%s\n' "$out" | sed -n 's/^ snapshot: //p')" +"$PYTHON" -m factlog reject --number 1 --from "$digest" --target "$KB" >/dev/null 2>&1 "$PYTHON" "$PLUGIN_ROOT/tools/merge_candidates.py" --wiki "$KB" >/dev/null 2>&1 # re-extract & merge -grep -q "X,rel,Y,sources/a.md,accepted," "$KB/facts/candidates.csv" && ok "accept survives a re-merge (preserved)" || bad "accept reverted after re-merge" -grep -q "Z,rel,W,sources/a.md,superseded," "$KB/facts/candidates.csv" && ok "reject still durable after re-merge" || bad "reject reverted after re-merge" +grep -q "X,rel,Y,sources/a.md,accepted," "$KB/facts/candidates.csv" && ok "numbered accept survives a re-merge (preserved)" || bad "numbered accept reverted after re-merge" +grep -q "Z,rel,W,sources/a.md,superseded," "$KB/facts/candidates.csv" && ok "numbered reject survives a re-merge" || bad "numbered reject reverted after re-merge" # a human-'confirmed' row is restored as confirmed, not coerced to accepted KB="$(mktemp -d)/wiki"