From 03dfce20b437cd480468ed80ec88e8f3fd942131 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 16 Jul 2026 19:45:21 -0700 Subject: [PATCH] =?UTF-8?q?feat(devpulse):=20compass=20curation=20v2=20Tra?= =?UTF-8?q?ck=201=20=E2=80=94=20supersedes=20links=20+=20atomic=20archive,?= =?UTF-8?q?=20write-time=20FTS=20conflict=20advisory,=20note=20command,=20?= =?UTF-8?q?--include-archived,=20score=20code-removal,=20review-per-prep?= =?UTF-8?q?=20(DPLAN-0246/FPLAN-0331).=20435=20green,=20seedgo=2031/31?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/commands/prep.md | 8 +- CHANGELOG.md | 17 ++ .../apps/handlers/compass/__init__.py | 4 + .../devpulse/apps/handlers/compass/store.py | 209 ++++++++++++++++-- src/aipass/devpulse/apps/modules/compass.py | 145 +++++++++++- .../devpulse/tests/test_compass_command.py | 173 +++++++++++++++ .../devpulse/tests/test_compass_store.py | 158 +++++++++++++ 7 files changed, 688 insertions(+), 26 deletions(-) diff --git a/.claude/commands/prep.md b/.claude/commands/prep.md index 4f576fcb..26ed82fe 100644 --- a/.claude/commands/prep.md +++ b/.claude/commands/prep.md @@ -55,7 +55,12 @@ Quick checks beat assumptions: `ls`/`find` for files, `git ls-files`/`grep` for - Run `drone @ai_mail inbox 2>/dev/null` — report any unread emails - Close any that were already processed but not formally closed -## 5. Loose Ends +## 5. Compass Review (Devpulse only) + +- Run ONE `drone @devpulse compass review` — it serves the oldest-unreviewed entry. Judge it: still true → confirm; superseded → archive it and note what replaced it; wrong → fix or archive. +- One entry per prep, every prep. This is the curation cadence — review only works if it actually runs (DPLAN-0246: all 127 entries sat unreviewed because nothing invoked it). + +## 6. Loose Ends - Flag anything in-flight: running background agents, dispatched branches waiting for replies, pending decisions - If anything can't survive compaction (e.g., agent IDs needed for resume), write it to local.json todos[] @@ -71,5 +76,6 @@ Prep complete: - Plans: [which ones updated] - Git: [branch, uncommitted count, suggestion] - Inbox: [count, action taken] +- Compass: [entry #N reviewed — verdict] - Loose ends: [any flagged] ``` diff --git a/CHANGELOG.md b/CHANGELOG.md index 199e9a7d..ffff82f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,23 @@ PyPI version — not the changelog header. ### Added +- **Compass curation v2 Track 1 (DPLAN-0246/FPLAN-0331): supersedes links + + write-time conflict check.** A correcting compass entry now archives and + links what it replaces in one transaction (`compass add --supersedes N`); + query renders both directions ("supersedes #N" / "ARCHIVED — superseded by + #M") so a retracted decision can never masquerade as current truth. Every + `compass add` FTS-checks the new text against active entries and prints a + non-blocking "possible conflict with #X" advisory — flag-and-ask, no LLM, no + auto-resolve (boardroom ruling). New `compass note ` command (FTS + re-index proven by test), `--include-archived` query flag (the avoid-list is + finally searchable), dead `score` column removed from all code surfaces + (kept inert on disk — zero migration risk). Idempotent PRAGMA-checked + migration ran clean on the production store (128 rows, no loss); the four + fresh-eyes-audit archive pairs got their links backfilled. /prep now runs + one `compass review` per session — curation living in a path that already + runs, the lesson of all three compass eras. 435 devpulse tests green, + seedgo 31/31 on both touched modules. + - **Close pipeline completes itself (DPLAN-0245): auto-vectorization + crash-safe registry writes + drone timeout policy.** Closing a plan now produces all side effects from one command — `post_close_runner` invokes diff --git a/src/aipass/devpulse/apps/handlers/compass/__init__.py b/src/aipass/devpulse/apps/handlers/compass/__init__.py index 7ec0cbc3..cf3242b6 100644 --- a/src/aipass/devpulse/apps/handlers/compass/__init__.py +++ b/src/aipass/devpulse/apps/handlers/compass/__init__.py @@ -23,9 +23,11 @@ from aipass.devpulse.apps.handlers.compass.store import ( VALID_STATUSES, add_decision, archive, + find_conflicts, query_decisions, rate, review, + set_note, stats, ) @@ -36,8 +38,10 @@ __all__ = [ "VALID_STATUSES", "add_decision", "archive", + "find_conflicts", "query_decisions", "rate", "review", + "set_note", "stats", ] diff --git a/src/aipass/devpulse/apps/handlers/compass/store.py b/src/aipass/devpulse/apps/handlers/compass/store.py index f700fdc7..52665ae6 100644 --- a/src/aipass/devpulse/apps/handlers/compass/store.py +++ b/src/aipass/devpulse/apps/handlers/compass/store.py @@ -29,6 +29,7 @@ command, and maintenance UX are later phases. """ import logging +import re import sqlite3 from datetime import date from pathlib import Path @@ -57,6 +58,9 @@ VALID_SOURCES = ("devpulse", "user") VALID_STATUSES = ("active", "archived") # Columns we return / surface from the decisions table (everything useful). +# NOTE: ``score`` is deliberately absent — it is code-invisible (DPLAN-0246 +# seedgo ruling). The column stays physically on disk as inert NULL, but no +# Python surface (SELECTs, returned dicts) touches it. _DECISION_COLUMNS = ( "id", "created", @@ -66,10 +70,10 @@ _DECISION_COLUMNS = ( "note", "tags", "source", - "score", "status", "last_reviewed", "times_surfaced", + "supersedes", ) _SCHEMA = """ @@ -85,7 +89,8 @@ CREATE TABLE IF NOT EXISTS decisions ( score INTEGER, status TEXT NOT NULL DEFAULT 'active' CHECK(status IN ('active','archived')), last_reviewed TEXT, - times_surfaced INTEGER NOT NULL DEFAULT 0 + times_surfaced INTEGER NOT NULL DEFAULT 0, + supersedes INTEGER ); CREATE VIRTUAL TABLE IF NOT EXISTS decisions_fts USING fts5( @@ -133,6 +138,42 @@ def _verify_fts5(conn: sqlite3.Connection) -> None: ) from exc +def _migrate(conn: sqlite3.Connection) -> None: + """Apply idempotent schema migrations to an already-open DB. + + Adds the ``supersedes`` column to DBs created before it existed. Guarded by + a ``PRAGMA table_info`` pre-check so it is safe to run on every connect — + never a blind ``ALTER`` (DPLAN-0246 seedgo ruling: idempotent migration). + Fresh DBs already carry the column from ``_SCHEMA``; the pre-check makes + this a no-op for them. + """ + cols = {row["name"] for row in conn.execute("PRAGMA table_info(decisions)")} + if "supersedes" not in cols: + conn.execute("ALTER TABLE decisions ADD COLUMN supersedes INTEGER") + conn.commit() + logger.info("[compass] migration: added supersedes column") + + +# FTS5 MATCH treats characters like " * ( ) : - ^ and the words AND/OR/NOT as +# syntax. Untrusted text (a decision's own words) can therefore crash MATCH. +# We defuse it by extracting bare word tokens and OR-ing them as quoted string +# literals — no operator can survive, and quoting a bareword is exact. +_FTS_WORD = re.compile(r"\w+", re.UNICODE) + + +def _sanitize_fts_query(text: Optional[str]) -> Optional[str]: + """Turn arbitrary text into a safe FTS5 MATCH expression, or None. + + Returns an ``OR`` of the text's word tokens, each quoted as a string + literal so FTS5 syntax characters can never reach the parser. Returns None + when there are no usable tokens (caller should skip the search). + """ + tokens = _FTS_WORD.findall(text or "") + if not tokens: + return None + return " OR ".join(f'"{t}"' for t in tokens) + + def _resolve_db_path(db_path: Optional[Path | str]) -> Path: """Resolve the effective DB path, defaulting to the branch-root location.""" return Path(db_path) if db_path is not None else DEFAULT_DB_PATH @@ -141,8 +182,9 @@ def _resolve_db_path(db_path: Optional[Path | str]) -> Path: def _connect(db_path: Optional[Path | str]) -> sqlite3.Connection: """Open (and lazily initialise) the compass DB. - Creates parent directories on first use, verifies FTS5, ensures schema. - Rows come back as ``sqlite3.Row`` so we can build clean dicts. + Creates parent directories on first use, verifies FTS5, ensures schema, and + runs idempotent migrations. Rows come back as ``sqlite3.Row`` so we can + build clean dicts. """ path = _resolve_db_path(db_path) path.parent.mkdir(parents=True, exist_ok=True) @@ -151,6 +193,7 @@ def _connect(db_path: Optional[Path | str]) -> sqlite3.Connection: conn.execute("PRAGMA foreign_keys = ON") _verify_fts5(conn) conn.executescript(_SCHEMA) + _migrate(conn) return conn @@ -168,6 +211,7 @@ def add_decision( source: str = "devpulse", db_path: Optional[Path | str] = None, created: Optional[str] = None, + supersedes: Optional[int] = None, ) -> int: """Add a rated decision and return its new id. @@ -181,12 +225,16 @@ def add_decision( db_path: Optional DB path override (tests pass a temp path). created: Optional ISO date override; defaults to today. This is the ONLY place a "today" date is stamped. + supersedes: Optional id of the decision this entry corrects. When set, + the new entry links to it AND that entry is archived — atomically, + in one transaction. Errors cleanly (no write) if the id is unknown. Returns: The new row's integer id. Raises: - ValueError: On empty context/decision or invalid rating/source. + ValueError: On empty context/decision, invalid rating/source, or a + ``supersedes`` target that does not exist. """ if not context or not context.strip(): raise ValueError("context must be a non-empty string") @@ -201,22 +249,46 @@ def add_decision( conn = _connect(db_path) try: + # Validate the supersede target BEFORE any write so a bad id never + # leaves a partial insert behind. + if supersedes is not None: + target = conn.execute("SELECT 1 FROM decisions WHERE id = ?", (supersedes,)).fetchone() + if target is None: + raise ValueError(f"cannot supersede #{supersedes}: no decision with that id") + + # Insert + archive-the-target in ONE transaction (commit once at the + # end); any failure rolls the whole thing back — never half-applied. cur = conn.execute( """ - INSERT INTO decisions (created, context, decision, rating, note, tags, source) - VALUES (?, ?, ?, ?, ?, ?, ?) + INSERT INTO decisions (created, context, decision, rating, note, tags, source, supersedes) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) """, - (stamp, context.strip(), decision.strip(), rating, note, tags, source), + (stamp, context.strip(), decision.strip(), rating, note, tags, source, supersedes), ) - conn.commit() if cur.lastrowid is None: # pragma: no cover - sqlite always sets this on INSERT raise RuntimeError("compass: INSERT did not return a rowid") new_id = int(cur.lastrowid) + + if supersedes is not None: + conn.execute("UPDATE decisions SET status = 'archived' WHERE id = ?", (supersedes,)) + + conn.commit() + except Exception: + conn.rollback() + raise finally: conn.close() - logger.info("[compass] added decision id=%s rating=%s source=%s", new_id, rating, source) - json_handler.log_operation("compass_add", {"id": new_id, "rating": rating, "source": source}) + logger.info( + "[compass] added decision id=%s rating=%s source=%s supersedes=%s", + new_id, + rating, + source, + supersedes, + ) + json_handler.log_operation( + "compass_add", {"id": new_id, "rating": rating, "source": source, "supersedes": supersedes} + ) return new_id @@ -224,21 +296,28 @@ def query_decisions( query: str, rating: Optional[str] = None, limit: int = 5, + include_archived: bool = False, db_path: Optional[Path | str] = None, ) -> list[dict]: - """Search active decisions, ranked by FTS5 BM25 relevance. + """Search decisions, ranked by FTS5 BM25 relevance. - Increments ``times_surfaced`` for every returned row. + Increments ``times_surfaced`` for every returned row. Each result dict + carries a computed ``superseded_by`` field: the id of the row that + supersedes this one, or None. Combined with the ``supersedes`` column this + lets callers render both pointer directions. Args: query: FTS5 match query (keywords). rating: Optional exact rating filter (one of VALID_RATINGS). limit: Max rows to return (default 5). + include_archived: When True, lift the ``status = 'active'`` filter so + archived rows (the avoid-list) are searchable too. Default False — + unchanged active-only behaviour. db_path: Optional DB path override. Returns: A list of decision dicts (most relevant first). Each dict includes the - rating and all useful fields. + rating, all useful fields, and the computed ``superseded_by`` pointer. Raises: ValueError: On empty query, bad rating filter, or non-positive limit. @@ -256,9 +335,10 @@ def query_decisions( FROM decisions_fts f JOIN decisions d ON d.id = f.rowid WHERE decisions_fts MATCH ? - AND d.status = 'active' """ params: list = [query.strip()] + if not include_archived: + sql += " AND d.status = 'active'" if rating is not None: sql += " AND d.rating = ?" params.append(rating) @@ -269,24 +349,119 @@ def query_decisions( try: rows = conn.execute(sql, params).fetchall() results = [_row_to_dict(r) for r in rows] + for r in results: + r["superseded_by"] = None ids = [r["id"] for r in results] if ids: placeholders = ",".join("?" for _ in ids) + # Reverse-lookup: which returned rows are pointed AT by a superseder? + successors: dict = {} + for row in conn.execute( + f"SELECT id, supersedes FROM decisions WHERE supersedes IN ({placeholders})", + ids, + ): + successors[row["supersedes"]] = row["id"] conn.execute( f"UPDATE decisions SET times_surfaced = times_surfaced + 1 WHERE id IN ({placeholders})", ids, ) conn.commit() - # Reflect the increment in the returned dicts without a re-query. + # Reflect the increment + attach the reverse pointer without re-query. for r in results: r["times_surfaced"] = (r["times_surfaced"] or 0) + 1 + r["superseded_by"] = successors.get(r["id"]) finally: conn.close() - logger.info("[compass] query %r rating=%s -> %d hit(s)", query, rating, len(results)) + logger.info( + "[compass] query %r rating=%s include_archived=%s -> %d hit(s)", + query, + rating, + include_archived, + len(results), + ) return results +def find_conflicts( + context: str, + decision: str, + limit: int = 3, + db_path: Optional[Path | str] = None, +) -> list[dict]: + """Return ACTIVE decisions whose text overlaps a would-be new entry. + + A write-time, side-effect-free advisory helper: it does NOT increment + ``times_surfaced`` and never writes. The combined ``context + decision`` + text is sanitised (:func:`_sanitize_fts_query`) so no FTS5 syntax character + can crash the MATCH. Returns up to ``limit`` active hits by BM25 relevance, + or an empty list when the text has no usable tokens / nothing overlaps. + + Args: + context: The would-be new entry's context. + decision: The would-be new entry's decision. + limit: Max advisory hits to return (default 3). + db_path: Optional DB path override. + + Returns: + A list of active decision dicts (most relevant first), possibly empty. + """ + match = _sanitize_fts_query(f"{context or ''} {decision or ''}") + if match is None: + return [] + + select_cols = ", ".join(f"d.{c}" for c in _DECISION_COLUMNS) + sql = f""" + SELECT {select_cols} + FROM decisions_fts f + JOIN decisions d ON d.id = f.rowid + WHERE decisions_fts MATCH ? + AND d.status = 'active' + ORDER BY bm25(decisions_fts) ASC + LIMIT ? + """ + conn = _connect(db_path) + try: + rows = conn.execute(sql, (match, limit)).fetchall() + results = [_row_to_dict(r) for r in rows] + finally: + conn.close() + + logger.info("[compass] conflict-check -> %d active hit(s)", len(results)) + return results + + +def set_note( + decision_id: int, + note: Optional[str], + db_path: Optional[Path | str] = None, +) -> bool: + """Set (replace) the note on an existing decision. + + The FTS5 external-content ``decisions_au`` trigger re-indexes the row on + UPDATE, so the new note is immediately searchable. + + Args: + decision_id: Target decision id. + note: The note text to store (may be empty to clear). + db_path: Optional DB path override. + + Returns: + True if a row was updated, False if no such id. + """ + conn = _connect(db_path) + try: + cur = conn.execute("UPDATE decisions SET note = ? WHERE id = ?", (note, decision_id)) + conn.commit() + changed = cur.rowcount > 0 + finally: + conn.close() + + logger.info("[compass] note id=%s (changed=%s)", decision_id, changed) + json_handler.log_operation("compass_note", {"id": decision_id, "changed": changed}) + return changed + + def stats(db_path: Optional[Path | str] = None) -> dict: """Return decision counts by rating, by status, and the total. diff --git a/src/aipass/devpulse/apps/modules/compass.py b/src/aipass/devpulse/apps/modules/compass.py index 11e02881..b9fa6f64 100644 --- a/src/aipass/devpulse/apps/modules/compass.py +++ b/src/aipass/devpulse/apps/modules/compass.py @@ -18,11 +18,12 @@ This module is the thin command layer (FPLAN P2). It parses args, calls the No business logic lives here — that's the handler's job. Subcommands: - add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..] - query "question" [--rating R] [--limit N] + add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..] [--supersedes N] + query "question" [--rating R] [--limit N] [--include-archived] stats rate archive + note "text" review Every subcommand accepts ``--db PATH`` (passed through as ``db_path=``) for @@ -40,7 +41,7 @@ from aipass.devpulse.apps.handlers.json import json_handler console = err_console -_VALID_SUBCOMMANDS = ("add", "query", "stats", "rate", "archive", "review") +_VALID_SUBCOMMANDS = ("add", "query", "stats", "rate", "archive", "note", "review") # Console colour per rating — the rating is the signal, so make it pop. _RATING_STYLE = { @@ -59,6 +60,7 @@ HELP_TEXT = """\ compass stats Counts by rating/status compass rate Re-rate a decision compass archive Archive a decision + compass note "text" Set a decision's note compass review Surface one to review compass --help Show this help @@ -70,19 +72,43 @@ HELP_TEXT = """\ --note "..." Optional human observation. --tags a,b,c Optional comma-separated tags. --source S Optional. devpulse (default) or user. + --supersedes N Optional. Archive decision #N and link this entry as its + correction (atomic). At add time, overlapping active + entries are shown as a non-blocking advisory. + +[bold]Options (query):[/bold] + --rating R Optional exact-rating filter. + --limit N Optional max results (default 5). + --include-archived Also search archived (avoid-list) entries; archived hits + show their status + supersession pointer. [bold]Options (all subcommands):[/bold] --db PATH Use an alternate SQLite store (testing / power use). [bold]Examples:[/bold] drone @devpulse compass add "auth fork" "chose JWT over sessions" --rating good + drone @devpulse compass add "auth fork" "switch to sessions" --rating good --supersedes 4 drone @devpulse compass query "auth" --rating good --limit 3 + drone @devpulse compass query "auth" --include-archived drone @devpulse compass stats drone @devpulse compass rate 4 bad drone @devpulse compass archive 4 + drone @devpulse compass note 4 "revisited — this held up" drone @devpulse compass review -See DPLAN-0212 (design) and the compass handler (apps/handlers/compass/). +See DPLAN-0212 / DPLAN-0246 (design) and the compass handler (apps/handlers/compass/). +""" + + +_NOTE_HELP_TEXT = """\ +[bold]compass note[/bold] — set (replace) a decision's note + +Usage: + compass note "text" Set the note on decision # + compass note --help Show this help + +The note is re-indexed for search immediately — the FTS5 mirror stays in sync, +so the new note text is findable by 'compass query' right away. """ @@ -93,7 +119,7 @@ def print_introspection() -> None: console.print("[dim]Devpulse rated decision store. The truth-store of choices —[/dim]") console.print("[dim]each decision rated; the rating is the signal at a fork.[/dim]") console.print() - console.print("[yellow]Subcommands:[/yellow] [cyan]add, query, stats, rate, archive, review[/cyan]") + console.print("[yellow]Subcommands:[/yellow] [cyan]add, query, stats, rate, archive, note, review[/cyan]") console.print("[dim]Run 'compass --help' for full usage.[/dim]") console.print() @@ -141,6 +167,8 @@ def handle_command(command: str, args: List[str]) -> bool: return _handle_rate(sub_args) if subcommand == "archive": return _handle_archive(sub_args) + if subcommand == "note": + return _handle_note(sub_args) if subcommand == "review": return _handle_review(sub_args) @@ -179,6 +207,18 @@ def _extract_db_path(args: List[str]) -> tuple[List[str], Optional[str]]: return _extract_flag(args, "--db") +def _extract_bool_flag(args: List[str], flag: str) -> tuple[List[str], bool]: + """Pull a valueless boolean ``--flag`` out of args. + + Returns the remaining args (every occurrence of the flag removed) and True + if the flag was present, else False. Unlike ``_extract_flag`` this consumes + no following value. + """ + if flag in args: + return [a for a in args if a != flag], True + return args, False + + def _rating_tag(rating: str) -> str: """Render a coloured ``[RATING]`` tag for query/review output.""" style = _RATING_STYLE.get(rating, "bold white") @@ -198,13 +238,16 @@ def _handle_add(sub_args: List[str]) -> bool: rest, note = _extract_flag(rest, "--note") rest, tags = _extract_flag(rest, "--tags") rest, source = _extract_flag(rest, "--source") + rest, supersedes_raw = _extract_flag(rest, "--supersedes") except ValueError as exc: logger.warning("[compass] add arg-parse error: %s", exc) error(str(exc), suggestion="Use 'compass --help' for usage") return True if len(rest) < 2: - error('Usage: compass add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..]') + error( + 'Usage: compass add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..] [--supersedes N]' + ) return True if rating is None: error("compass add requires --rating", suggestion="One of: good | bad | impressive | interesting") @@ -213,6 +256,34 @@ def _handle_add(sub_args: List[str]) -> bool: context = rest[0] decision = rest[1] + supersedes: Optional[int] = None + if supersedes_raw is not None: + try: + supersedes = int(supersedes_raw) + except ValueError as exc: + logger.warning("[compass] add bad --supersedes %r: %s", supersedes_raw, exc) + error(f"--supersedes must be an integer, got {supersedes_raw!r}") + return True + + # Write-time conflict check — a NON-BLOCKING advisory (DPLAN-0246). Skipped + # when the writer already chose to supersede, and never allowed to block or + # crash the add. Only shown when NOT already superseding. + if supersedes is None: + try: + conflicts = compass.find_conflicts(context, decision, db_path=db_path) + except Exception as exc: # advisory must never break a write + logger.warning("[compass] conflict-check failed (non-blocking): %s", exc) + conflicts = [] + for c in conflicts: + cid = c.get("id") + excerpt = (c.get("context") or "").strip() + if len(excerpt) > 80: + excerpt = excerpt[:77] + "..." + console.print( + f"[yellow]possible conflict with #{cid}[/yellow]: {excerpt} " + f"[dim]— supersede? (--supersedes {cid})[/dim]" + ) + try: new_id = compass.add_decision( context, @@ -222,6 +293,7 @@ def _handle_add(sub_args: List[str]) -> bool: tags=tags, source=source if source is not None else "devpulse", db_path=db_path, + supersedes=supersedes, ) except ValueError as exc: logger.warning("[compass] add rejected: %s", exc) @@ -235,6 +307,8 @@ def _handle_add(sub_args: List[str]) -> bool: console.print(f" [cyan]note:[/cyan] {note}") if tags: console.print(f" [cyan]tags:[/cyan] {tags}") + if supersedes is not None: + console.print(f" [magenta]supersedes #{supersedes}[/magenta] [dim](archived)[/dim]") return True @@ -242,6 +316,7 @@ def _handle_query(sub_args: List[str]) -> bool: """Parse and dispatch ``compass query "question" [--rating R] [--limit N]``.""" try: rest, db_path = _extract_db_path(sub_args) + rest, include_archived = _extract_bool_flag(rest, "--include-archived") rest, rating = _extract_flag(rest, "--rating") rest, limit_raw = _extract_flag(rest, "--limit") except ValueError as exc: @@ -250,7 +325,7 @@ def _handle_query(sub_args: List[str]) -> bool: return True if not rest: - error('Usage: compass query "question" [--rating R] [--limit N]') + error('Usage: compass query "question" [--rating R] [--limit N] [--include-archived]') return True query_text = rest[0] @@ -265,7 +340,13 @@ def _handle_query(sub_args: List[str]) -> bool: return True try: - results = compass.query_decisions(query_text, rating=rating, limit=limit, db_path=db_path) + results = compass.query_decisions( + query_text, + rating=rating, + limit=limit, + include_archived=include_archived, + db_path=db_path, + ) except ValueError as exc: logger.warning("[compass] query rejected: %s", exc) error(str(exc)) @@ -294,6 +375,16 @@ def _render_query_results(query_text: str, rating: Optional[str], results: List[ console.print(f" [cyan]note:[/cyan] {r['note']}") if r.get("tags"): console.print(f" [cyan]tags:[/cyan] {r['tags']}") + # Supersession pointers — an archived hit must never masquerade as + # current truth, so flag its status + who replaced it (DPLAN-0246). + if r.get("status") == "archived": + superseded_by = r.get("superseded_by") + if superseded_by: + console.print(f" [bold yellow]ARCHIVED[/bold yellow] — superseded by #{superseded_by}") + else: + console.print(" [bold yellow]ARCHIVED[/bold yellow] (avoid-list)") + if r.get("supersedes"): + console.print(f" [magenta]supersedes #{r['supersedes']}[/magenta]") meta = f"source={r.get('source', '?')} status={r.get('status', '?')} surfaced={r.get('times_surfaced', 0)}" console.print(f" [dim]{meta}[/dim]") console.print() @@ -389,6 +480,44 @@ def _handle_archive(sub_args: List[str]) -> bool: return True +def _handle_note(sub_args: List[str]) -> bool: + """Dispatch ``compass note "text"`` — set a decision's note. + + Follows the subcommand-help convention: ``compass note --help`` prints the + per-subcommand help block; malformed input shows the Usage line. + """ + try: + rest, db_path = _extract_db_path(sub_args) + except ValueError as exc: + logger.warning("[compass] note arg-parse error: %s", exc) + error(str(exc)) + return True + + if rest and rest[0] in ("--help", "-h", "help"): + console.print(_NOTE_HELP_TEXT) + return True + + if len(rest) < 2: + error('Usage: compass note "text"') + return True + + try: + decision_id = int(rest[0]) + except ValueError as exc: + logger.warning("[compass] note bad id %r: %s", rest[0], exc) + error(f" must be an integer, got {rest[0]!r}") + return True + + note_text = rest[1] + changed = compass.set_note(decision_id, note_text, db_path=db_path) + if changed: + console.print(f"[green]Note set[/green] on [bold]#{decision_id}[/bold]") + console.print(f" [cyan]note:[/cyan] {note_text}") + else: + warning(f"No decision with id {decision_id} — nothing changed.") + return True + + def _handle_review(sub_args: List[str]) -> bool: """Dispatch ``compass review`` — surface one active decision to review.""" try: diff --git a/src/aipass/devpulse/tests/test_compass_command.py b/src/aipass/devpulse/tests/test_compass_command.py index f11f1696..f7c92a80 100644 --- a/src/aipass/devpulse/tests/test_compass_command.py +++ b/src/aipass/devpulse/tests/test_compass_command.py @@ -275,3 +275,176 @@ def test_flag_without_value_errors(capsys, db): assert compass_cmd.handle_command("compass", ["query", "x", "--rating"]) is True out = _output(capsys).lower() assert "rating" in out and "value" in out + + +# --------------------------------------------------------------------------- +# supersedes — atomic archive + link, both pointer directions +# --------------------------------------------------------------------------- + + +def test_add_supersedes_archives_and_links(capsys, db): + """add --supersedes N archives #N, links the new row, shows 'supersedes #N'.""" + old = _add(capsys, db, "old ctx sessions", "use sessions", "good") + capsys.readouterr() + assert ( + compass_cmd.handle_command( + "compass", + [ + "add", + "new ctx jwt", + "switch to jwt", + "--rating", + "good", + "--supersedes", + str(old), + "--db", + db, + ], + ) + is True + ) + out = _output(capsys) + assert f"supersedes #{old}" in out + + # The archived row is gone from the default (active-only) query... + q = _query_out(capsys, db, "sessions") + assert "0 result(s)" in q + + # ...but --include-archived surfaces it WITH its status + forward pointer. + q2 = _query_out(capsys, db, "sessions", "--include-archived") + assert "ARCHIVED" in q2.upper() + assert "superseded by #" in q2.lower() + + +def test_add_supersedes_bad_id_errors_no_write(capsys, db): + """add --supersedes to a missing id errors and writes nothing.""" + capsys.readouterr() + compass_cmd.handle_command( + "compass", + ["add", "ctx", "dec", "--rating", "good", "--supersedes", "9999", "--db", db], + ) + out = _output(capsys).lower() + assert "9999" in out + assert "total decisions: 0" in _stats_out(capsys, db).lower() + + +def test_add_supersedes_non_integer_errors(capsys, db): + """A non-integer --supersedes fails loud.""" + assert ( + compass_cmd.handle_command( + "compass", + ["add", "ctx", "dec", "--rating", "good", "--supersedes", "abc", "--db", db], + ) + is True + ) + out = _output(capsys).lower() + assert "supersedes" in out and "integer" in out + + +# --------------------------------------------------------------------------- +# write-time conflict advisory — non-blocking +# --------------------------------------------------------------------------- + + +def test_add_conflict_advisory_prints_but_does_not_block(capsys, db): + """An overlapping active row triggers an advisory; the add still succeeds.""" + _add(capsys, db, "caching layer strategy", "add redis caching", "good") + capsys.readouterr() + compass_cmd.handle_command( + "compass", + ["add", "caching approach again", "another caching layer", "--rating", "good", "--db", db], + ) + out = _output(capsys) + assert "possible conflict" in out.lower() + assert "--supersedes" in out # advisory hints the fix + assert "Added decision" in out # NON-BLOCKING: still added + + +def test_add_no_conflict_on_empty_store(capsys, db): + """First add on an empty store prints no advisory.""" + capsys.readouterr() + compass_cmd.handle_command("compass", ["add", "unique ctx", "unique dec", "--rating", "good", "--db", db]) + out = _output(capsys).lower() + assert "possible conflict" not in out + + +# --------------------------------------------------------------------------- +# note — set a note, prove it is immediately searchable +# --------------------------------------------------------------------------- + + +def test_note_command_sets_and_is_searchable(capsys, db): + """note "text" sets the note; a later query finds the new note text.""" + did = _add(capsys, db, "note cmd ctx", "note cmd dec", "good") + capsys.readouterr() + assert compass_cmd.handle_command("compass", ["note", str(did), "findme pterodactyl", "--db", db]) is True + out = _output(capsys).lower() + assert "note set" in out + + q = _query_out(capsys, db, "pterodactyl") + assert "1 result(s)" in q + + +def test_note_help(capsys): + """compass note --help prints per-subcommand usage.""" + assert compass_cmd.handle_command("compass", ["note", "--help"]) is True + out = _output(capsys).lower() + assert "note" in out and "usage" in out + + +def test_note_missing_id_warns(capsys, db): + """note on a non-existent id reports nothing changed, does not crash.""" + assert compass_cmd.handle_command("compass", ["note", "999", "text", "--db", db]) is True + out = _output(capsys).lower() + assert "999" in out and ("nothing changed" in out or "no decision" in out) + + +def test_note_missing_args_shows_usage(capsys, db): + """note with too few args shows usage.""" + assert compass_cmd.handle_command("compass", ["note", "5", "--db", db]) is True + out = _output(capsys).lower() + assert "usage" in out + + +def test_help_and_introspection_list_note(capsys): + """Both --help and bare introspection advertise the note subcommand.""" + compass_cmd.handle_command("compass", ["--help"]) + assert "note" in _output(capsys).lower() + compass_cmd.handle_command("compass", []) + assert "note" in _output(capsys).lower() + + +# --------------------------------------------------------------------------- +# --include-archived — archived hits must show status + supersession pointer +# --------------------------------------------------------------------------- + + +def test_include_archived_shows_archived_pointer(capsys, db): + """--include-archived surfaces an archived row flagged with its successor.""" + old = _add(capsys, db, "archived-visible ctx", "the old choice", "bad") + capsys.readouterr() + compass_cmd.handle_command( + "compass", + [ + "add", + "replacement ctx", + "the new choice", + "--rating", + "good", + "--supersedes", + str(old), + "--db", + db, + ], + ) + capsys.readouterr() + + # Default query hides the archived row. + q = _query_out(capsys, db, "old choice") + assert "0 result(s)" in q + + # With the flag it appears, unmistakably marked archived + superseded. + q2 = _query_out(capsys, db, "old choice", "--include-archived") + assert "1 result(s)" in q2 + assert "archived" in q2.lower() + assert "superseded by #" in q2.lower() diff --git a/src/aipass/devpulse/tests/test_compass_store.py b/src/aipass/devpulse/tests/test_compass_store.py index 4abfd957..7d22abd9 100644 --- a/src/aipass/devpulse/tests/test_compass_store.py +++ b/src/aipass/devpulse/tests/test_compass_store.py @@ -283,3 +283,161 @@ class TestInputValidation: compass.add_decision("ctx", "dec", "good", db_path=db) with pytest.raises(ValueError): compass.query_decisions("ctx", limit=0, db_path=db) + + +class TestSupersedes: + """supersedes column, atomic archive+link, idempotent migration (DPLAN-0246).""" + + def test_add_with_supersedes_archives_and_links(self, db): + """--supersedes links the corrector AND archives the target, atomically.""" + old = compass.add_decision("old auth ctx", "use sessions", "good", db_path=db) + new = compass.add_decision("new auth ctx", "switch to JWT", "good", db_path=db, supersedes=old) + # The corrector row links back to what it replaced. + hit = compass.query_decisions("JWT", db_path=db)[0] + assert hit["supersedes"] == old + # The old entry is archived → gone from the active query. + assert compass.query_decisions("sessions", db_path=db) == [] + # With include_archived it reappears, pointing FORWARD to its successor. + arch = compass.query_decisions("sessions", include_archived=True, db_path=db)[0] + assert arch["status"] == "archived" + assert arch["superseded_by"] == new + + def test_supersedes_nonexistent_raises_no_partial_write(self, db): + """A bad --supersedes id errors cleanly and leaves NO partial write.""" + with pytest.raises(ValueError): + compass.add_decision("ctx", "dec", "good", db_path=db, supersedes=9999) + assert compass.stats(db_path=db)["total"] == 0 + + def test_supersedes_defaults_none(self, db): + """A plain add has supersedes=None and superseded_by=None.""" + compass.add_decision("plain ctx", "plain dec", "good", db_path=db) + hit = compass.query_decisions("plain", db_path=db)[0] + assert hit["supersedes"] is None + assert hit["superseded_by"] is None + + def test_active_hit_shows_its_supersedes_pointer(self, db): + """An ACTIVE corrector still exposes its supersedes pointer on query.""" + old = compass.add_decision("legacy topic zzz", "old way", "bad", db_path=db) + compass.add_decision("current topic zzz", "new way", "good", db_path=db, supersedes=old) + hit = compass.query_decisions("current", db_path=db)[0] + assert hit["supersedes"] == old + assert hit["superseded_by"] is None # nothing supersedes the corrector + + def test_migration_idempotent_repeated_connects(self, db): + """Re-opening the DB re-runs migration harmlessly; column stays present.""" + compass.add_decision("ctx one", "dec one", "good", db_path=db) + for _ in range(3): + conn = store._connect(db) + try: + cols = {r["name"] for r in conn.execute("PRAGMA table_info(decisions)")} + finally: + conn.close() + assert "supersedes" in cols + + def test_migration_adds_column_to_legacy_db(self, db): + """A pre-supersedes DB gets the column added in via _connect migration.""" + # Build a legacy `decisions` table WITHOUT supersedes, as older builds had. + conn = sqlite3.connect(str(db)) + conn.execute( + "CREATE TABLE decisions (" + "id INTEGER PRIMARY KEY, created TEXT, context TEXT NOT NULL, " + "decision TEXT NOT NULL, rating TEXT NOT NULL, note TEXT, tags TEXT, " + "source TEXT, score INTEGER, status TEXT DEFAULT 'active', " + "last_reviewed TEXT, times_surfaced INTEGER DEFAULT 0)" + ) + conn.commit() + conn.close() + # Legacy table lacks the column... + conn = sqlite3.connect(str(db)) + pre = {r[1] for r in conn.execute("PRAGMA table_info(decisions)")} + conn.close() + assert "supersedes" not in pre + # ...a store connect migrates it in (CREATE IF NOT EXISTS is a no-op here). + conn = store._connect(db) + try: + post = {r["name"] for r in conn.execute("PRAGMA table_info(decisions)")} + finally: + conn.close() + assert "supersedes" in post + + +class TestFindConflicts: + """Write-time conflict check: FTS over ACTIVE rows, sanitized, side-effect-free.""" + + def test_finds_overlapping_active_row(self, db): + """A would-be entry surfaces an existing active row it overlaps.""" + compass.add_decision("caching strategy for the API", "add a redis caching layer", "good", db_path=db) + hits = compass.find_conflicts("caching approach", "use a caching layer", db_path=db) + assert len(hits) >= 1 + assert any("caching" in h["context"] for h in hits) + + def test_ignores_archived_rows(self, db): + """Conflict check searches active rows only — archived never surfaces.""" + did = compass.add_decision("archived topic xyzzy", "some decision", "good", db_path=db) + compass.archive(did, db_path=db) + assert compass.find_conflicts("xyzzy topic", "another decision", db_path=db) == [] + + def test_no_side_effects_on_times_surfaced(self, db): + """The advisory must NOT bump times_surfaced — it is not a real surface.""" + compass.add_decision("surfacing guard ctx", "a decision here", "good", db_path=db) + compass.find_conflicts("surfacing guard", "a decision", db_path=db) + hit = compass.query_decisions("surfacing", db_path=db)[0] + assert hit["times_surfaced"] == 1 # only the query above counted + + def test_sanitizes_fts_special_chars(self, db): + """Raw FTS5 syntax characters must not crash MATCH — sanitized to literals.""" + compass.add_decision("special ctx", "a normal decision", "good", db_path=db) + weird = 'broken " ( ) * : query -term AND OR NOT' + result = compass.find_conflicts(weird, "more * (text) ^caret", db_path=db) + assert isinstance(result, list) # no exception raised + + def test_empty_text_returns_empty(self, db): + """Text with no usable tokens yields no conflicts (and no crash).""" + assert compass.find_conflicts(" ", " ", db_path=db) == [] + + +class TestSetNote: + """note edits persist AND re-index immediately via the FTS5 UPDATE trigger.""" + + def test_set_note_updates_and_returns_true(self, db): + """set_note stores the note and reports the row was changed.""" + did = compass.add_decision("note ctx", "note dec", "good", db_path=db) + assert compass.set_note(did, "a fresh observation", db_path=db) is True + hit = compass.query_decisions("note ctx", db_path=db)[0] + assert hit["note"] == "a fresh observation" + + def test_note_edit_is_immediately_fts_searchable(self, db): + """PROOF: the decisions_au trigger re-indexes a note UPDATE for FTS.""" + did = compass.add_decision("indexing ctx", "indexing dec", "good", db_path=db) + # 'zebra' appears nowhere yet. + assert compass.query_decisions("zebra", db_path=db) == [] + compass.set_note(did, "mentions zebra now", db_path=db) + # Immediately findable through the freshly re-indexed note column. + found = compass.query_decisions("zebra", db_path=db) + assert len(found) == 1 + assert found[0]["id"] == did + + def test_set_note_missing_id_returns_false(self, db): + """set_note on a non-existent id returns False (no silent create).""" + assert compass.set_note(9999, "nope", db_path=db) is False + + +class TestScoreRemoved: + """score is gone from every Python surface (DPLAN-0246 seedgo ruling).""" + + def test_score_not_in_decision_columns(self): + """The code-level column list no longer names score.""" + assert "score" not in store._DECISION_COLUMNS + + def test_score_absent_from_query_dict(self, db): + """Query result dicts carry no score key.""" + compass.add_decision("score ctx", "score dec", "good", db_path=db) + hit = compass.query_decisions("score", db_path=db)[0] + assert "score" not in hit + + def test_score_absent_from_review_dict(self, db): + """Review result dicts carry no score key.""" + compass.add_decision("review score ctx", "dec", "good", db_path=db) + result = compass.review(db_path=db) + assert result is not None + assert "score" not in result