From 034823abd81abddc2f7842ee7f47eeea8f130fca Mon Sep 17 00:00:00 2001 From: gethin Date: Sat, 9 May 2026 21:47:25 +0100 Subject: [PATCH 1/7] fix(install_hooks): use python.exe for SessionStart, keep pythonw.exe for async hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows pythonw.exe silently nulls sys.stdout when no console is attached, so hooks installed under it can print without erroring but their output never reaches the parent. Claude Code reads SessionStart hook stdout for additionalContext — bootstrap silently produced nothing this whole time, with no entry in hook_errors to flag the failure. Adds HookSpec.needs_stdout (True only for session_bootstrap). _hook_entry picks venv_py for stdout-required hooks and venv_pyw for the async observer and session_close hooks (which keep pythonw.exe to avoid console flashes on every tool call). merge_settings_json gains an optional venv_py kwarg that defaults to venv_pyw for back-compat with existing tests; setup.sh already passes both. Two new regression tests guard the split. Co-Authored-By: Claude Opus 4.7 (1M context) --- better_memory/cli/install_hooks.py | 51 ++++++++++++++++++++++-------- tests/cli/test_install_hooks.py | 25 +++++++++++++++ 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/better_memory/cli/install_hooks.py b/better_memory/cli/install_hooks.py index a48d067..df8745f 100644 --- a/better_memory/cli/install_hooks.py +++ b/better_memory/cli/install_hooks.py @@ -27,16 +27,21 @@ @dataclass(frozen=True) class HookSpec: - module: str # e.g. "better_memory.hooks.session_start" - event: str # "SessionStart" | "PostToolUse" | "Stop" - matcher: str | None # None for SessionStart/Stop; "Write|Edit|Bash" for observer - is_async: bool # True for PostToolUse + Stop + module: str # e.g. "better_memory.hooks.session_start" + event: str # "SessionStart" | "PostToolUse" | "Stop" + matcher: str | None # None for SessionStart/Stop; "Write|Edit|Bash" for observer + is_async: bool # True for PostToolUse + Stop + needs_stdout: bool # True for SessionStart bootstrap — Claude Code reads + # the hook's stdout for additionalContext, so the + # interpreter MUST keep stdout attached. On Windows + # pythonw.exe silently nulls sys.stdout; the bootstrap + # would print nothing and Claude would get no context. _OUR_HOOKS: tuple[HookSpec, ...] = ( - HookSpec("better_memory.hooks.session_bootstrap", "SessionStart", None, False), - HookSpec("better_memory.hooks.observer", "PostToolUse", "Write|Edit|Bash", True), - HookSpec("better_memory.hooks.session_close", "Stop", None, True), + HookSpec("better_memory.hooks.session_bootstrap", "SessionStart", None, False, True), + HookSpec("better_memory.hooks.observer", "PostToolUse", "Write|Edit|Bash", True, False), + HookSpec("better_memory.hooks.session_close", "Stop", None, True, False), ) # Module paths that are no longer registered but may be present in users' @@ -83,18 +88,28 @@ def merge_claude_json(existing: dict, *, command: str, home: str) -> dict: # ------------------------------------------------------- pure merge: settings -def _hook_entry(spec: HookSpec, venv_pyw: str) -> dict: - """Build the JSON object for a single hook entry.""" +def _hook_entry(spec: HookSpec, venv_py: str, venv_pyw: str) -> dict: + """Build the JSON object for a single hook entry. + + Interpreter selection: hooks marked ``needs_stdout`` use ``venv_py`` + (python.exe on Windows) so Claude Code can read the hook's stdout + for ``additionalContext``. Other hooks use ``venv_pyw`` (pythonw.exe + on Windows) to avoid the brief console flash on each tool call. + On non-Windows systems setup.sh passes the same path for both. + """ + interpreter = venv_py if spec.needs_stdout else venv_pyw entry: dict = { "type": "command", - "command": f'"{venv_pyw}" -m {spec.module}', + "command": f'"{interpreter}" -m {spec.module}', } if spec.is_async: entry["async"] = True return entry -def merge_settings_json(existing: dict, *, venv_pyw: str) -> dict: +def merge_settings_json( + existing: dict, *, venv_pyw: str, venv_py: str | None = None, +) -> dict: """Smart-merge our hook entries into ~/.claude/settings.json content. Two-pass strategy: @@ -107,7 +122,15 @@ def merge_settings_json(existing: dict, *, venv_pyw: str) -> dict: each get their own group. User's other (non-better-memory) hooks and matcher-groups are untouched. + + ``venv_py`` defaults to ``venv_pyw`` for back-compat with callers that + only know about one interpreter. On Windows the two are different: + ``venv_py`` is python.exe (foreground, keeps stdout), ``venv_pyw`` is + pythonw.exe (background, no console). See ``_hook_entry`` for the + per-spec selection rule. """ + if venv_py is None: + venv_py = venv_pyw config = dict(existing) hooks = dict(config.get("hooks", {})) our_module_paths = {spec.module for spec in _OUR_HOOKS} @@ -132,18 +155,18 @@ def merge_settings_json(existing: dict, *, venv_pyw: str) -> dict: session_start_specs = [s for s in _OUR_HOOKS if s.event == "SessionStart"] if session_start_specs: hooks.setdefault("SessionStart", []).append({ - "hooks": [_hook_entry(s, venv_pyw) for s in session_start_specs], + "hooks": [_hook_entry(s, venv_py, venv_pyw) for s in session_start_specs], }) for spec in (s for s in _OUR_HOOKS if s.event == "PostToolUse"): - group: dict = {"hooks": [_hook_entry(spec, venv_pyw)]} + group: dict = {"hooks": [_hook_entry(spec, venv_py, venv_pyw)]} if spec.matcher is not None: group["matcher"] = spec.matcher hooks.setdefault("PostToolUse", []).append(group) for spec in (s for s in _OUR_HOOKS if s.event == "Stop"): hooks.setdefault("Stop", []).append({ - "hooks": [_hook_entry(spec, venv_pyw)], + "hooks": [_hook_entry(spec, venv_py, venv_pyw)], }) config["hooks"] = hooks diff --git a/tests/cli/test_install_hooks.py b/tests/cli/test_install_hooks.py index b23dff3..0d9b872 100644 --- a/tests/cli/test_install_hooks.py +++ b/tests/cli/test_install_hooks.py @@ -286,6 +286,31 @@ def test_idempotent_second_run_is_noop(self) -> None: second = merge_settings_json(first, venv_pyw="/p/pyw") assert first == second + def test_session_bootstrap_uses_venv_py_async_hooks_use_venv_pyw(self) -> None: + """Foreground bootstrap needs python.exe (stdout reaches Claude + Code); the two async hooks keep pythonw.exe so they don't flash + a console window on every tool call. setup.sh passes the same + path on non-Windows; the split only matters on Windows.""" + out = merge_settings_json( + {}, venv_py="/venv/python.exe", venv_pyw="/venv/pythonw.exe", + ) + ss_cmd = out["hooks"]["SessionStart"][0]["hooks"][0]["command"] + assert "python.exe" in ss_cmd and "pythonw.exe" not in ss_cmd + assert "session_bootstrap" in ss_cmd + + ptu_cmd = out["hooks"]["PostToolUse"][0]["hooks"][0]["command"] + assert "pythonw.exe" in ptu_cmd + + stop_cmd = out["hooks"]["Stop"][0]["hooks"][0]["command"] + assert "pythonw.exe" in stop_cmd + + def test_default_venv_py_falls_back_to_venv_pyw(self) -> None: + """Back-compat: callers that only pass venv_pyw get the old + behavior (every hook uses the same interpreter).""" + out = merge_settings_json({}, venv_pyw="/legacy/path/pyw") + ss_cmd = out["hooks"]["SessionStart"][0]["hooks"][0]["command"] + assert "/legacy/path/pyw" in ss_cmd + def test_merge_settings_strips_legacy_session_start_and_session_retrieve(): """Re-running install_hooks after upgrade scrubs the two old hook entries.""" From 234de6359fc37f9a7992ca03816b56e591b5f06f Mon Sep 17 00:00:00 2001 From: gethin Date: Sat, 9 May 2026 21:47:36 +0100 Subject: [PATCH 2/7] docs: spec + plan for promote-reflection-to-general User-driven flip of a reflection's scope from project to general from the Reflections drawer, alongside Confirm / Retire / Edit. Active-status-only, promote-only (no demote), no schema migration. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...026-05-09-promote-reflection-to-general.md | 721 ++++++++++++++++++ ...09-promote-reflection-to-general-design.md | 264 +++++++ 2 files changed, 985 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-09-promote-reflection-to-general.md create mode 100644 docs/superpowers/specs/2026-05-09-promote-reflection-to-general-design.md diff --git a/docs/superpowers/plans/2026-05-09-promote-reflection-to-general.md b/docs/superpowers/plans/2026-05-09-promote-reflection-to-general.md new file mode 100644 index 0000000..42cd686 --- /dev/null +++ b/docs/superpowers/plans/2026-05-09-promote-reflection-to-general.md @@ -0,0 +1,721 @@ +# Promote Reflection to General Scope — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a "Promote to general" button to the Reflections drawer so the user can flip a project-scoped reflection's `scope` column to `general`, making it visible from `memory.retrieve` in every project. + +**Architecture:** Tiny vertical slice — new `ReflectionService.promote_to_general()` method, new `POST /reflections//promote` route, drawer template gains a `Scope` meta row and a gated promote button, `reflection_detail` query / `ReflectionFull` dataclass gain a `scope` field. No schema migration. Status guard (`pending_review` / `confirmed` only) enforced both server-side and in the template. + +**Tech Stack:** Python 3.12, sqlite3, Flask, htmx, Jinja2, pytest. + +**Spec:** `docs/superpowers/specs/2026-05-09-promote-reflection-to-general-design.md` + +--- + +## File Structure + +| File | Role | +| --- | --- | +| `better_memory/services/reflection.py` | Add `ReflectionService.promote_to_general()` method, sibling of `confirm` / `retire` / `update_text`. | +| `better_memory/ui/queries.py` | Add `scope: str` to `ReflectionFull` dataclass; include `scope` in `reflection_detail` SELECT and constructor. | +| `better_memory/ui/app.py` | Add `POST /reflections//promote` route, modelled on `reflection_confirm`. | +| `better_memory/ui/templates/fragments/reflection_drawer.html` | Add `Scope` meta row; add gated `Promote to general` button inside the existing actions block. | +| `tests/services/test_reflection_writes.py` | Add `TestPromoteToGeneral` class (6 tests). | +| `tests/ui/test_queries_reflections.py` | Extend `_seed` helper with `scope` param; add scope-population assertion for `reflection_detail`. | +| `tests/ui/test_reflections.py` | Extend `_seed_reflection` helper with `scope` param; add `TestReflectionPromote` class (3 tests) + 2 drawer template tests. | + +--- + +## Task 1: Read-model — add `scope` to `ReflectionFull` and `reflection_detail` + +**Files:** +- Modify: `better_memory/ui/queries.py:298-314` (`ReflectionFull` dataclass), `better_memory/ui/queries.py:357-363` (SELECT), `better_memory/ui/queries.py:404-418` (constructor). +- Modify: `tests/ui/test_queries_reflections.py:31-59` (`_seed` helper); `tests/ui/test_queries_reflections.py` (existing `TestReflectionDetail` block — add new test). + +The drawer template needs `detail.reflection.scope` to render the meta row and gate the button. Without this change, the template would crash on `AttributeError`. Do this task first so subsequent tasks can rely on the field. + +- [ ] **Step 1: Extend the test seed helper to support `scope`** + +In `tests/ui/test_queries_reflections.py`, modify `_seed`: + +```python +def _seed( + conn, + *, + rid: str, + project: str = "proj-a", + tech: str | None = None, + phase: str = "general", + polarity: str = "do", + confidence: float = 0.7, + status: str = "confirmed", + use_cases: str = "uc", + hints: str = "h", + title: str | None = None, + created_at: str = "2026-04-25T10:00:00+00:00", + updated_at: str = "2026-04-25T10:00:00+00:00", + evidence_count: int = 0, + scope: str = "project", +) -> None: + conn.execute( + "INSERT INTO reflections " + "(id, title, project, tech, phase, polarity, use_cases, hints, " + "confidence, status, evidence_count, created_at, updated_at, scope) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ( + rid, title or f"title-{rid}", project, tech, phase, polarity, + use_cases, hints, confidence, status, evidence_count, + created_at, updated_at, scope, + ), + ) + conn.commit() +``` + +- [ ] **Step 2: Write failing tests for the new `scope` field** + +In `tests/ui/test_queries_reflections.py`, append to the `TestReflectionDetail` class: + +```python + def test_returns_default_project_scope_when_unspecified(self, conn): + _seed(conn, rid="r-1") # default scope='project' + detail = reflection_detail(conn, reflection_id="r-1") + assert detail is not None + assert detail.reflection.scope == "project" + + def test_returns_general_scope_when_seeded_general(self, conn): + _seed(conn, rid="r-1", scope="general") + detail = reflection_detail(conn, reflection_id="r-1") + assert detail is not None + assert detail.reflection.scope == "general" +``` + +- [ ] **Step 3: Run the new tests; confirm they fail** + +Run: `uv run pytest tests/ui/test_queries_reflections.py::TestReflectionDetail -v` + +Expected: the two new tests fail with `AttributeError: 'ReflectionFull' object has no attribute 'scope'` (or similar). + +- [ ] **Step 4: Add `scope` to `ReflectionFull`** + +In `better_memory/ui/queries.py`, modify the `ReflectionFull` dataclass (currently lines 298-314). Add `scope: str` after `evidence_count`: + +```python +@dataclass(frozen=True) +class ReflectionFull: + """Full reflection row for the drawer.""" + + id: str + title: str + project: str + tech: str | None + phase: str + polarity: str + confidence: float + status: str + use_cases: str + hints: str + evidence_count: int + scope: str + created_at: str + updated_at: str +``` + +(Keep `created_at` / `updated_at` last — they were last before; `scope` slots in just before them.) + +- [ ] **Step 5: Add `scope` to the `reflection_detail` SELECT and constructor** + +In `better_memory/ui/queries.py`, modify the `r_row` SELECT inside `reflection_detail` (currently lines 357-363): + +```python + r_row = conn.execute( + "SELECT id, title, project, tech, phase, polarity, " + "confidence, status, use_cases, hints, evidence_count, scope, " + "created_at, updated_at " + "FROM reflections WHERE id = ?", + (reflection_id,), + ).fetchone() +``` + +And the `ReflectionFull(...)` constructor call (currently lines 404-418): + +```python + return ReflectionDetail( + reflection=ReflectionFull( + id=r_row["id"], + title=r_row["title"], + project=r_row["project"], + tech=r_row["tech"], + phase=r_row["phase"], + polarity=r_row["polarity"], + confidence=r_row["confidence"], + status=r_row["status"], + use_cases=r_row["use_cases"], + hints=r_row["hints"], + evidence_count=r_row["evidence_count"], + scope=r_row["scope"], + created_at=r_row["created_at"], + updated_at=r_row["updated_at"], + ), + sources=sources, + ) +``` + +- [ ] **Step 6: Run the queries tests; confirm they pass** + +Run: `uv run pytest tests/ui/test_queries_reflections.py -v` + +Expected: all tests pass, including the two new scope tests. + +- [ ] **Step 7: Run the broader test suite to catch any consumer that builds `ReflectionFull` positionally** + +Run: `uv run pytest tests -x -q` + +Expected: PASS. If any test fails because it constructed `ReflectionFull` with positional args, fix it to use keyword args (or update positional order). The dataclass is `frozen=True` and called by name in the existing code, so this should not be an issue — but verify. + +- [ ] **Step 8: Commit** + +```bash +git add better_memory/ui/queries.py tests/ui/test_queries_reflections.py +git commit -m "feat(reflections): expose scope field on reflection_detail read model" +``` + +--- + +## Task 2: Service method — `ReflectionService.promote_to_general` + +**Files:** +- Modify: `better_memory/services/reflection.py` (the `ReflectionService` class — append the new method). +- Modify: `tests/services/test_reflection_writes.py:31-40` (`_seed_reflection` helper); append `TestPromoteToGeneral` class. + +- [ ] **Step 1: Extend the test seed helper to support `scope`** + +In `tests/services/test_reflection_writes.py`, modify `_seed_reflection`: + +```python +def _seed_reflection( + conn, reflection_id: str, status: str = "pending_review", + *, scope: str = "project", +) -> None: + conn.execute( + "INSERT INTO reflections " + "(id, title, project, phase, polarity, use_cases, hints, " + "confidence, status, scope, created_at, updated_at) " + "VALUES (?, ?, 'proj-a', 'general', 'do', 'old uc', 'old h', " + "0.7, ?, ?, '2026-04-25T00:00:00+00:00', '2026-04-25T00:00:00+00:00')", + (reflection_id, f"title-{reflection_id}", status, scope), + ) + conn.commit() +``` + +- [ ] **Step 2: Write the failing service tests** + +Append to `tests/services/test_reflection_writes.py`: + +```python +class TestPromoteToGeneral: + def test_promotes_pending_review_project_to_general(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="pending_review", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + + svc.promote_to_general(reflection_id="r1") + + row = conn.execute( + "SELECT scope, updated_at FROM reflections WHERE id = ?", ("r1",) + ).fetchone() + assert row["scope"] == "general" + assert row["updated_at"] == "2026-04-26T12:00:00+00:00" + + def test_promotes_confirmed_project_to_general(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="confirmed", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + + svc.promote_to_general(reflection_id="r1") + + row = conn.execute( + "SELECT scope FROM reflections WHERE id = ?", ("r1",) + ).fetchone() + assert row["scope"] == "general" + + def test_promote_is_idempotent_on_already_general(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="pending_review", scope="general") + svc = ReflectionService(conn, clock=fixed_clock) + + svc.promote_to_general(reflection_id="r1") + + row = conn.execute( + "SELECT scope, updated_at FROM reflections WHERE id = ?", ("r1",) + ).fetchone() + assert row["scope"] == "general" + # No-op: updated_at NOT bumped (matches confirm/retire idempotency). + assert row["updated_at"] == "2026-04-25T00:00:00+00:00" + + def test_raises_when_reflection_does_not_exist(self, conn, fixed_clock): + svc = ReflectionService(conn, clock=fixed_clock) + with pytest.raises(ValueError, match="Reflection not found"): + svc.promote_to_general(reflection_id="nope") + + def test_raises_when_retired(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="retired", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + with pytest.raises(ValueError, match="Cannot promote reflection in status 'retired'"): + svc.promote_to_general(reflection_id="r1") + + def test_raises_when_superseded(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="superseded", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + with pytest.raises(ValueError, match="Cannot promote reflection in status 'superseded'"): + svc.promote_to_general(reflection_id="r1") +``` + +- [ ] **Step 3: Run the new tests; confirm they fail** + +Run: `uv run pytest tests/services/test_reflection_writes.py::TestPromoteToGeneral -v` + +Expected: all six tests fail with `AttributeError: 'ReflectionService' object has no attribute 'promote_to_general'`. + +- [ ] **Step 4: Implement `promote_to_general` on `ReflectionService`** + +In `better_memory/services/reflection.py`, append a new method to the `ReflectionService` class (after `update_text`): + +```python + def promote_to_general(self, *, reflection_id: str) -> None: + """project → general; idempotent on already-general; raise on retired/superseded. + + Mirrors the no-op-on-already-target semantics of ``confirm`` and + ``retire``: when the reflection is already general we return + without bumping ``updated_at`` so audit trails stay honest. + + Status guard matches the UI gate in the drawer template — the + button is hidden on retired/superseded, but we enforce server + side too in case of direct API calls. + """ + row = self._conn.execute( + "SELECT scope, status FROM reflections WHERE id = ?", + (reflection_id,), + ).fetchone() + if row is None: + raise ValueError(f"Reflection not found: {reflection_id}") + status = row["status"] + if status not in ("pending_review", "confirmed"): + raise ValueError( + f"Cannot promote reflection in status {status!r}" + ) + if row["scope"] == "general": + return + now = self._clock().isoformat() + self._conn.execute( + "UPDATE reflections SET scope = 'general', updated_at = ? " + "WHERE id = ?", + (now, reflection_id), + ) + self._conn.commit() +``` + +- [ ] **Step 5: Run the service tests; confirm they pass** + +Run: `uv run pytest tests/services/test_reflection_writes.py -v` + +Expected: all tests pass, including the six new `TestPromoteToGeneral` cases. + +- [ ] **Step 6: Commit** + +```bash +git add better_memory/services/reflection.py tests/services/test_reflection_writes.py +git commit -m "feat(reflections): add ReflectionService.promote_to_general" +``` + +--- + +## Task 3: Route — `POST /reflections//promote` + +**Files:** +- Modify: `better_memory/ui/app.py` (the `/reflections` route block — append after `reflection_edit_save`). +- Modify: `tests/ui/test_reflections.py:13-43` (`_seed_reflection` helper); append `TestReflectionPromote` class. + +- [ ] **Step 1: Extend the test seed helper to support `scope`** + +In `tests/ui/test_reflections.py`, modify `_seed_reflection`: + +```python +def _seed_reflection( + db_path: Path, + *, + rid: str, + project: str = "proj-a", + tech: str | None = None, + phase: str = "general", + polarity: str = "do", + confidence: float = 0.7, + status: str = "confirmed", + use_cases: str = "uc", + hints: str = "h", + title: str | None = None, + evidence_count: int = 0, + scope: str = "project", +) -> None: + conn = connect(db_path) + try: + conn.execute( + "INSERT INTO reflections " + "(id, title, project, tech, phase, polarity, use_cases, hints, " + "confidence, status, evidence_count, scope, created_at, updated_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, " + "'2026-04-26T10:00:00+00:00', '2026-04-26T10:00:00+00:00')", + ( + rid, title or f"title-{rid}", project, tech, phase, polarity, + use_cases, hints, confidence, status, evidence_count, scope, + ), + ) + conn.commit() + finally: + conn.close() +``` + +- [ ] **Step 2: Write the failing route tests** + +Append to `tests/ui/test_reflections.py`: + +```python +class TestReflectionPromote: + def test_promotes_project_pending( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="pending_review", scope="project") + + response = client.post( + "/reflections/r-1/promote", + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 200 + assert response.headers.get("HX-Trigger") == "reflection-changed" + + conn = connect(tmp_db) + try: + row = conn.execute( + "SELECT scope FROM reflections WHERE id = ?", ("r-1",) + ).fetchone() + finally: + conn.close() + assert row["scope"] == "general" + + def test_404_for_unknown(self, client: FlaskClient): + response = client.post( + "/reflections/does-not-exist/promote", + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 404 + + def test_409_for_retired( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="retired", scope="project") + + response = client.post( + "/reflections/r-1/promote", + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 409 + body = response.get_data(as_text=True) + assert "card-error" in body +``` + +- [ ] **Step 3: Run the new tests; confirm they fail** + +Run: `uv run pytest tests/ui/test_reflections.py::TestReflectionPromote -v` + +Expected: all three tests fail with 404 (route doesn't exist) — Flask returns 404 for unmapped paths. + +- [ ] **Step 4: Add the `reflection_promote` route** + +In `better_memory/ui/app.py`, immediately after the `reflection_edit_save` function (currently ends around line 367), insert: + +```python + @app.post("/reflections//promote") + def reflection_promote(id: str) -> tuple[str, int, dict[str, str]]: + conn = app.extensions["db_connection"] + if queries.reflection_detail(conn, reflection_id=id) is None: + abort(404) + try: + app.extensions["reflection_service"].promote_to_general( + reflection_id=id, + ) + except ValueError as exc: + return ( + f'
' + f"

{escape(str(exc))}

" + "
" + ), 409, {} + detail = queries.reflection_detail(conn, reflection_id=id) + rendered = render_template( + "fragments/reflection_drawer.html", detail=detail + ) + return rendered, 200, {"HX-Trigger": "reflection-changed"} +``` + +- [ ] **Step 5: Run the route tests; confirm they pass** + +Run: `uv run pytest tests/ui/test_reflections.py::TestReflectionPromote -v` + +Expected: all three tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add better_memory/ui/app.py tests/ui/test_reflections.py +git commit -m "feat(reflections): add POST /reflections//promote route" +``` + +--- + +## Task 4: Template — `Scope` meta row + gated `Promote to general` button + +**Files:** +- Modify: `better_memory/ui/templates/fragments/reflection_drawer.html`. +- Modify: `tests/ui/test_reflections.py` (append two drawer-render tests in a new `TestReflectionDrawerScope` class). + +- [ ] **Step 1: Write the failing drawer-render tests** + +Append to `tests/ui/test_reflections.py`: + +```python +class TestReflectionDrawerScope: + def test_drawer_shows_scope_meta_and_promote_button_for_active_project( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection( + tmp_db, rid="r-1", status="pending_review", scope="project", + ) + response = client.get("/reflections/r-1/drawer") + assert response.status_code == 200 + body = response.get_data(as_text=True) + # Scope meta row visible. + assert "Scope" in body + assert ">project<" in body + # Promote button rendered. + assert "Promote to general" in body + assert "action-promote" in body + assert "/reflections/r-1/promote" in body + + def test_drawer_hides_promote_button_when_already_general( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection( + tmp_db, rid="r-1", status="pending_review", scope="general", + ) + response = client.get("/reflections/r-1/drawer") + assert response.status_code == 200 + body = response.get_data(as_text=True) + # Scope meta row still visible (now general). + assert "Scope" in body + assert ">general<" in body + # No promote button. + assert "Promote to general" not in body + assert "action-promote" not in body + + def test_drawer_hides_promote_button_when_retired( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection( + tmp_db, rid="r-1", status="retired", scope="project", + ) + response = client.get("/reflections/r-1/drawer") + assert response.status_code == 200 + body = response.get_data(as_text=True) + # Scope meta row still visible. + assert "Scope" in body + # No promote button (existing actions block already hidden on retired). + assert "Promote to general" not in body + assert "action-promote" not in body +``` + +- [ ] **Step 2: Run the new tests; confirm they fail** + +Run: `uv run pytest tests/ui/test_reflections.py::TestReflectionDrawerScope -v` + +Expected: all three tests fail — first one fails because `Scope` / `Promote to general` are not in the body; the others may pass accidentally (no button rendered today). Either way, expect at least one failure. + +- [ ] **Step 3: Add the `Scope` meta row to the drawer template** + +In `better_memory/ui/templates/fragments/reflection_drawer.html`, find the existing `
` block. Insert a new `
/
` pair right after the `Status` line so lifecycle metadata clusters together. The new block (lines 9-23 currently) should become: + +```html +
+
Project
{{ detail.reflection.project }}
+ {% if detail.reflection.tech %} +
Tech
{{ detail.reflection.tech }}
+ {% endif %} +
Phase
+
{{ detail.reflection.phase }}
+
Polarity
+
{{ detail.reflection.polarity }}
+
Confidence
+
{{ '%.2f' | format(detail.reflection.confidence) }}
+
Status
{{ detail.reflection.status }}
+
Scope
{{ detail.reflection.scope }}
+
Evidence
{{ detail.reflection.evidence_count }} observation{{ 's' if detail.reflection.evidence_count != 1 else '' }}
+
Updated
{{ detail.reflection.updated_at }}
+
+``` + +- [ ] **Step 4: Add the gated `Promote to general` button inside the actions block** + +Still in `reflection_drawer.html`, find the existing `{% if detail.reflection.status in ('pending_review', 'confirmed') %}` actions block. Inside it, after the `Edit` button (lines 60-66 currently) and before the closing `` of `drawer-actions`, insert: + +```html + {% if detail.reflection.scope == 'project' %} + + {% endif %} +``` + +The full updated actions block should now read: + +```html + {% if detail.reflection.status in ('pending_review', 'confirmed') %} +
+ {% if detail.reflection.status == 'pending_review' %} + + {% endif %} + + + {% if detail.reflection.scope == 'project' %} + + {% endif %} +
+ {% endif %} +``` + +- [ ] **Step 5: Run the drawer tests; confirm they pass** + +Run: `uv run pytest tests/ui/test_reflections.py::TestReflectionDrawerScope -v` + +Expected: all three tests pass. + +- [ ] **Step 6: Run the full reflections test surface** + +Run: `uv run pytest tests/ui/test_reflections.py tests/services/test_reflection_writes.py tests/ui/test_queries_reflections.py -v` + +Expected: PASS (all four feature tasks now landed). + +- [ ] **Step 7: Commit** + +```bash +git add better_memory/ui/templates/fragments/reflection_drawer.html tests/ui/test_reflections.py +git commit -m "feat(reflections): drawer shows scope and gated promote button" +``` + +--- + +## Task 5: End-to-end smoke + full suite + +**Files:** +- No new files. Verification only. + +- [ ] **Step 1: Run the entire test suite** + +Run: `uv run pytest -x -q` + +Expected: PASS, no regressions. If any unrelated test fails for environmental reasons (e.g. Ollama not running), note it and proceed; the new feature must not introduce new failures. + +- [ ] **Step 2: Manual UI smoke** + +Start (or re-use) the UI: + +```bash +cd /c/Users/gethi/source/better-memory && uv run python -m better_memory.ui & +url=$(cat ~/.better-memory/ui.url) && start "$url" +``` + +In the browser: +1. Navigate to the Reflections tab. +2. Open a reflection drawer for any project-scoped row. +3. Confirm the `Scope: project` meta row is visible. +4. Confirm a `Promote to general` button is visible in the actions row alongside `Confirm` / `Retire` / `Edit`. +5. Click `Promote to general`. The drawer should re-render with `Scope: general` and the promote button gone. +6. Open a `general`-scoped row (or the same one again post-promotion). Confirm no promote button is shown. +7. Open a `retired` row. Confirm no actions block is rendered (the existing status guard still applies). + +If any of these fail, fix the underlying issue (template gate, route, service) before continuing. + +- [ ] **Step 3: Run pyright on the touched files** + +Run: `uv run pyright better_memory/services/reflection.py better_memory/ui/queries.py better_memory/ui/app.py` + +Expected: 0 errors, 0 warnings. + +- [ ] **Step 4: Final commit (if any cleanup landed)** + +If steps 2 or 3 surfaced fixes, commit them with a clear message. Otherwise this step is a no-op. + +```bash +git status # confirm clean +``` + +--- + +## Self-Review + +**Spec coverage:** +- Service method (Spec → Service layer) → Task 2. +- Route (Spec → Route) → Task 3. +- Read-model `scope` field (Spec → Read model) → Task 1. +- Template `Scope` meta + gated promote button (Spec → Templates) → Task 4. +- Service tests (Spec → Service tests, 6 cases) → Task 2 step 2. +- Route tests (Spec → Route tests, 3 of 5 cases) → Task 3 step 2. +- Drawer template tests (Spec → Route tests, remaining 2 cases) → Task 4 step 1. +- Read-model test for scope population → Task 1 step 2. + +**Placeholder scan:** none. + +**Type / signature consistency:** +- `ReflectionFull.scope: str` — used the same field name in template, route handler, and tests. +- `ReflectionService.promote_to_general(*, reflection_id: str) -> None` — keyword-only arg matching `confirm` / `retire` / `update_text`. +- Route `reflection_promote(id: str)` — `id` matches the URL var name, matching sibling routes. +- HX-Trigger `"reflection-changed"` — matches the value used by `reflection_confirm` / `reflection_retire` / `reflection_edit_save`. +- Idempotency wording (`"already general"` no-op, no `updated_at` bump) matches `confirm` / `retire` semantics. + +No gaps detected. diff --git a/docs/superpowers/specs/2026-05-09-promote-reflection-to-general-design.md b/docs/superpowers/specs/2026-05-09-promote-reflection-to-general-design.md new file mode 100644 index 0000000..e51015f --- /dev/null +++ b/docs/superpowers/specs/2026-05-09-promote-reflection-to-general-design.md @@ -0,0 +1,264 @@ +# Promote a project-scoped reflection to general scope + +Date: 2026-05-09 +Status: design — pending implementation + +## Problem + +The Reflections tab (`/reflections`) lets the user `Confirm`, `Retire`, and +`Edit` a reflection from the drawer. The reflection schema already has a +`scope` column (`'project' | 'general'`, default `'project'`) — but no UI +affordance and no service method to flip it. + +Synthesis only emits a `general`-scope reflection when **every** source +observation is itself general (`_derive_new_reflection_scope`). When a +useful project-scoped lesson turns out to apply cross-project, the user +currently has no way to promote it short of writing SQL. + +The existing pattern on the semantic-memories page is a bidirectional +toggle (`Make general` / `Make project`). For reflections we want the +weaker, one-directional version: **promote project → general**, no +demote. + +## Decisions (settled with user 2026-05-09) + +- **Direction:** promote-only. No demote (general → project) action. + Treat promotion as a deliberate curation step. +- **UI placement:** drawer button only — alongside `Confirm`, `Retire`, + `Edit`. No row badge, no row toggle. The reflection row's whole-row + `hx-get` makes embedded buttons awkward, and the user already opens + the drawer to make any other lifecycle change. +- **Status guard:** active-only (`pending_review`, `confirmed`). The + button is hidden on `retired` and `superseded`, mirroring the gate + on the existing actions block. The service method enforces the same + guard so direct API calls can't bypass the UI. +- **Already-general:** the button is hidden when `scope == 'general'`. + Service is idempotent (no-op, no `updated_at` bump) on already-general + for safety against direct calls. +- **Provenance:** only `scope` flips. `reflections.project` keeps the + originating project as historical context. This matches + `_derive_new_reflection_scope` and `SemanticMemoryService.set_scope`. + +## Out of scope + +- Reverse direction (`general` → `project`) and any general-side toggle UX. +- Row-level scope badge or row-level toggle button. +- Bulk promote (multi-select on the panel). +- A dedicated audit-log entry. The `updated_at` bump and the `scope` + field itself are the only persisted trace. +- MCP-side `reflection.promote_to_general` tool. UI-only for now. + +## Architecture + +### Data layer + +No schema migration. `reflections.scope` already satisfies the +`CHECK(scope IN ('project','general'))` constraint introduced by +migration `0007_reflection_scope.sql`. + +### Service layer — `ReflectionService.promote_to_general` + +New method on the existing UI-facing `ReflectionService` class +(`better_memory/services/reflection.py`), sibling of `confirm`, `retire`, +`update_text`: + +```python +def promote_to_general(self, *, reflection_id: str) -> None: + """project → general; idempotent on already-general; raise on retired/superseded.""" +``` + +Behaviour: + +1. `SELECT scope, status FROM reflections WHERE id = ?`. Row is `None` + → `ValueError("Reflection not found: ")`. +2. `status in ('retired', 'superseded')` → + `ValueError("Cannot promote reflection in status ")`. Same + shape as existing `Cannot confirm` / `Cannot edit` messages. +3. `scope == 'general'` → return without writing. No `updated_at` bump, + matching the no-op semantics of `confirm` on already-confirmed and + `retire` on already-retired. +4. Otherwise: `UPDATE reflections SET scope='general', updated_at=? WHERE id=?` + with `self._clock().isoformat()`, then `self._conn.commit()`. + +The method is small enough that no helper extraction is justified. The +status- and scope-guards each get their own branch in the body. + +### Route — `POST /reflections//promote` + +New route in `better_memory/ui/app.py`, modelled on `reflection_confirm`: + +```python +@app.post("/reflections//promote") +def reflection_promote(id: str) -> tuple[str, int, dict[str, str]]: + conn = app.extensions["db_connection"] + if queries.reflection_detail(conn, reflection_id=id) is None: + abort(404) + try: + app.extensions["reflection_service"].promote_to_general(reflection_id=id) + except ValueError as exc: + return ( + f'

{escape(str(exc))}

', + 409, {}, + ) + detail = queries.reflection_detail(conn, reflection_id=id) + rendered = render_template("fragments/reflection_drawer.html", detail=detail) + return rendered, 200, {"HX-Trigger": "reflection-changed"} +``` + +Notes: + +- Status code shape matches existing reflection routes: + - 404 on missing id (existence check before service call). + - 409 on lifecycle violation (retired/superseded), with the standard + `card card-error` body fragment so htmx can render it. + - 200 with the re-rendered drawer fragment on success. +- The `HX-Trigger: reflection-changed` header reuses the existing + panel-refresh hook the other reflection actions already fire. +- No origin check is needed beyond the global `_origin_check` already + registered for non-GET requests. + +### Read model — `queries.reflection_detail` / `ReflectionFull` + +The drawer needs to render the current `scope` and the template needs +to gate the button on it. Currently `ReflectionFull` +(`better_memory/ui/queries.py:298-314`) does **not** include `scope`, +and the `reflection_detail` SELECT doesn't fetch it. + +Changes: + +- Add `scope: str` to the `ReflectionFull` dataclass. +- Add `scope` to the SELECT column list in `reflection_detail`. +- Pass `scope=r_row["scope"]` to the constructor. + +`reflection_list_for_ui` does not need changes (no row badge in this +spec). + +### Templates + +`better_memory/ui/templates/fragments/reflection_drawer.html`: + +1. Add a meta entry inside the `
` block, + placed after `Status` (so lifecycle metadata clusters together): + + ```html +
Scope
{{ detail.reflection.scope }}
+ ``` + +2. Inside the existing `{% if detail.reflection.status in ('pending_review','confirmed') %}` + actions block, after the `Edit` button, add the promote button — + gated by `scope`: + + ```html + {% if detail.reflection.scope == 'project' %} + + {% endif %} + ``` + + The status guard is provided by the surrounding `{% if %}`; the + scope guard hides the button when already general. Together they + implement: "show only on active project-scoped reflections". + +No CSS additions required — `.action-promote` reuses the default +button styling already applied to sibling action buttons. + +### URL helper / shutdown / origin / watchdog + +No changes. The new route inherits all of these from existing app +plumbing. + +## Tests + +Following the existing layout (one test module per service / route +file). + +### Service tests — `tests/services/test_reflection_writes.py` + +This is where the existing `confirm` / `retire` / `update_text` tests +live; add a new `class TestPromoteToGeneral` alongside them. + +- `test_promote_to_general_flips_scope_and_bumps_updated_at`: + insert a `pending_review` row with `scope='project'`, advance the + clock, call `promote_to_general`, assert `scope='general'` and + `updated_at` matches the new clock reading. +- `test_promote_to_general_works_on_confirmed`: + insert with `status='confirmed'`, promote, assert success. +- `test_promote_to_general_idempotent_on_already_general`: + insert with `scope='general'`, capture `updated_at`, advance clock, + call `promote_to_general`, assert no exception, `scope` unchanged, + `updated_at` **not** bumped. +- `test_promote_to_general_rejects_retired`: + insert with `status='retired'`, expect `ValueError` whose message + contains `'retired'`. +- `test_promote_to_general_rejects_superseded`: + same shape with `status='superseded'`. +- `test_promote_to_general_rejects_missing`: + call with a fabricated id, expect `ValueError` whose message + contains `'not found'`. + +### Route tests — `tests/ui/test_reflections.py` + +This holds the existing route + drawer-render tests for the reflections +tab; add the promote-route assertions here. + +- `test_promote_route_200_renders_drawer_with_general_scope`: + POST `/reflections//promote` for a project-scoped pending_review + row. Assert 200, response body contains `scope='general'` (or the + drawer's scope `
` text), and the response headers include + `HX-Trigger: reflection-changed`. +- `test_promote_route_404_on_missing`: + POST with a fabricated id, assert 404. +- `test_promote_route_409_on_retired`: + insert `status='retired'`, POST, assert 409 and the response body + contains the standard `card card-error` shape. +- `test_promote_button_hidden_in_drawer_when_already_general`: + GET `/reflections//drawer` for a `scope='general'` row, assert + the response body does **not** contain `action-promote` / + `Promote to general`. +- `test_promote_button_hidden_in_drawer_when_retired`: + same shape for `status='retired'`. + +### Read-model test — `tests/ui/test_queries_reflections.py` + +Add an assertion to the existing `reflection_detail` test that the +returned `ReflectionFull.scope` field is populated from the row. + +## File-by-file impact summary + +| File | Change | +| --- | --- | +| `better_memory/services/reflection.py` | Add `ReflectionService.promote_to_general` | +| `better_memory/ui/app.py` | Add `POST /reflections//promote` route | +| `better_memory/ui/queries.py` | Add `scope` to `ReflectionFull` + `reflection_detail` SELECT and constructor | +| `better_memory/ui/templates/fragments/reflection_drawer.html` | Add `Scope` meta row + gated `Promote to general` button | +| `tests/services/test_reflection_writes.py` | Add 6 service tests (`TestPromoteToGeneral`) | +| `tests/ui/test_reflections.py` | Add 5 route/template tests | +| `tests/ui/test_queries_reflections.py` | Add `scope` assertion to existing `reflection_detail` test | + +No migrations, no MCP server changes, no Skill changes, no CSS. + +## Risks and mitigations + +- **Flag drift between UI and service guard.** The drawer template + hides the button on retired/superseded but the service also raises + on those statuses. Both must agree. Mitigation: the route test + `test_promote_route_409_on_retired` exercises the service guard + directly, and `test_promote_button_hidden_in_drawer_when_retired` + exercises the template guard. If either drifts, one of the tests + fails. +- **`updated_at` semantics.** Two existing siblings (`confirm`, + `retire`) bump `updated_at` only when the row actually changes. + `promote_to_general` follows the same rule — explicitly tested by + `test_promote_to_general_idempotent_on_already_general`. Diverging + here would surprise audit-trail consumers. +- **Cross-project visibility for promoted reflections is immediate.** + Once `scope='general'`, the next `memory.retrieve` call from any + project sees the lesson (subject to the existing tech / phase / + polarity filters). There is no review queue for the promotion + itself. This is intentional — promote is a deliberate user action, + not a synthesis-emitted suggestion. From 91c8495a761802069b203d4cbee0c151eaafc2d9 Mon Sep 17 00:00:00 2001 From: gethin Date: Sat, 9 May 2026 21:55:21 +0100 Subject: [PATCH 3/7] feat(reflections): expose scope field on reflection_detail read model Add `scope: str` to `ReflectionFull` dataclass, include it in the `reflection_detail` SELECT and constructor, and extend the test seed helper + add two tests to drive and verify the field. Co-Authored-By: Claude Opus 4.7 (1M context) --- better_memory/ui/queries.py | 4 +++- tests/ui/test_queries_reflections.py | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/better_memory/ui/queries.py b/better_memory/ui/queries.py index 2e02b37..7865511 100644 --- a/better_memory/ui/queries.py +++ b/better_memory/ui/queries.py @@ -310,6 +310,7 @@ class ReflectionFull: use_cases: str hints: str evidence_count: int + scope: str created_at: str updated_at: str @@ -356,7 +357,7 @@ def reflection_detail( """ r_row = conn.execute( "SELECT id, title, project, tech, phase, polarity, " - "confidence, status, use_cases, hints, evidence_count, " + "confidence, status, use_cases, hints, evidence_count, scope, " "created_at, updated_at " "FROM reflections WHERE id = ?", (reflection_id,), @@ -413,6 +414,7 @@ def reflection_detail( use_cases=r_row["use_cases"], hints=r_row["hints"], evidence_count=r_row["evidence_count"], + scope=r_row["scope"], created_at=r_row["created_at"], updated_at=r_row["updated_at"], ), diff --git a/tests/ui/test_queries_reflections.py b/tests/ui/test_queries_reflections.py index 82d4c8d..893345a 100644 --- a/tests/ui/test_queries_reflections.py +++ b/tests/ui/test_queries_reflections.py @@ -44,16 +44,17 @@ def _seed( created_at: str = "2026-04-25T10:00:00+00:00", updated_at: str = "2026-04-25T10:00:00+00:00", evidence_count: int = 0, + scope: str = "project", ) -> None: conn.execute( "INSERT INTO reflections " "(id, title, project, tech, phase, polarity, use_cases, hints, " - "confidence, status, evidence_count, created_at, updated_at) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + "confidence, status, evidence_count, created_at, updated_at, scope) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", ( rid, title or f"title-{rid}", project, tech, phase, polarity, use_cases, hints, confidence, status, evidence_count, - created_at, updated_at, + created_at, updated_at, scope, ), ) conn.commit() @@ -271,3 +272,15 @@ def test_sources_ordered_by_observation_created_at_desc(self, conn): detail = reflection_detail(conn, reflection_id="r-1") assert detail is not None, "reflection_detail returns for ordered-sources seed" assert [s.observation_id for s in detail.sources] == ["obs-new", "obs-old"] + + def test_returns_default_project_scope_when_unspecified(self, conn): + _seed(conn, rid="r-1") # default scope='project' + detail = reflection_detail(conn, reflection_id="r-1") + assert detail is not None + assert detail.reflection.scope == "project" + + def test_returns_general_scope_when_seeded_general(self, conn): + _seed(conn, rid="r-1", scope="general") + detail = reflection_detail(conn, reflection_id="r-1") + assert detail is not None + assert detail.reflection.scope == "general" From 93058334b58e8601263ca80dd080ff4b1424e9a2 Mon Sep 17 00:00:00 2001 From: gethin Date: Sat, 9 May 2026 22:04:14 +0100 Subject: [PATCH 4/7] feat(reflections): add ReflectionService.promote_to_general Co-Authored-By: Claude Opus 4.7 (1M context) --- better_memory/services/reflection.py | 32 ++++++++++++ tests/services/test_reflection_writes.py | 66 ++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index bfcbf80..4b1b225 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -1292,3 +1292,35 @@ def update_text( (use_cases, json.dumps(hint_list), now, reflection_id), ) self._conn.commit() + + def promote_to_general(self, *, reflection_id: str) -> None: + """project → general; idempotent on already-general; raise on retired/superseded. + + Mirrors the no-op-on-already-target semantics of ``confirm`` and + ``retire``: when the reflection is already general we return + without bumping ``updated_at`` so audit trails stay honest. + + Status guard matches the UI gate in the drawer template — the + button is hidden on retired/superseded, but we enforce server + side too in case of direct API calls. + """ + row = self._conn.execute( + "SELECT scope, status FROM reflections WHERE id = ?", + (reflection_id,), + ).fetchone() + if row is None: + raise ValueError(f"Reflection not found: {reflection_id}") + status = row["status"] + if status not in ("pending_review", "confirmed"): + raise ValueError( + f"Cannot promote reflection in status {status!r}" + ) + if row["scope"] == "general": + return + now = self._clock().isoformat() + self._conn.execute( + "UPDATE reflections SET scope = 'general', updated_at = ? " + "WHERE id = ?", + (now, reflection_id), + ) + self._conn.commit() diff --git a/tests/services/test_reflection_writes.py b/tests/services/test_reflection_writes.py index c6de8bf..c4d9c0e 100644 --- a/tests/services/test_reflection_writes.py +++ b/tests/services/test_reflection_writes.py @@ -28,14 +28,17 @@ def fixed_clock(): return lambda: fixed -def _seed_reflection(conn, reflection_id: str, status: str = "pending_review") -> None: +def _seed_reflection( + conn, reflection_id: str, status: str = "pending_review", + *, scope: str = "project", +) -> None: conn.execute( "INSERT INTO reflections " "(id, title, project, phase, polarity, use_cases, hints, " - "confidence, status, created_at, updated_at) " + "confidence, status, scope, created_at, updated_at) " "VALUES (?, ?, 'proj-a', 'general', 'do', 'old uc', 'old h', " - "0.7, ?, '2026-04-25T00:00:00+00:00', '2026-04-25T00:00:00+00:00')", - (reflection_id, f"title-{reflection_id}", status), + "0.7, ?, ?, '2026-04-25T00:00:00+00:00', '2026-04-25T00:00:00+00:00')", + (reflection_id, f"title-{reflection_id}", status, scope), ) conn.commit() @@ -228,3 +231,58 @@ def test_hints_round_trip_through_synthesis_read_path( # this through json.loads at retrieve_reflections / _apply_augment. decoded = json.loads(row["hints"]) assert decoded == ["hint a", "hint b"] + + +class TestPromoteToGeneral: + def test_promotes_pending_review_project_to_general(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="pending_review", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + + svc.promote_to_general(reflection_id="r1") + + row = conn.execute( + "SELECT scope, updated_at FROM reflections WHERE id = ?", ("r1",) + ).fetchone() + assert row["scope"] == "general" + assert row["updated_at"] == "2026-04-26T12:00:00+00:00" + + def test_promotes_confirmed_project_to_general(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="confirmed", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + + svc.promote_to_general(reflection_id="r1") + + row = conn.execute( + "SELECT scope FROM reflections WHERE id = ?", ("r1",) + ).fetchone() + assert row["scope"] == "general" + + def test_promote_is_idempotent_on_already_general(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="pending_review", scope="general") + svc = ReflectionService(conn, clock=fixed_clock) + + svc.promote_to_general(reflection_id="r1") + + row = conn.execute( + "SELECT scope, updated_at FROM reflections WHERE id = ?", ("r1",) + ).fetchone() + assert row["scope"] == "general" + # No-op: updated_at NOT bumped (matches confirm/retire idempotency). + assert row["updated_at"] == "2026-04-25T00:00:00+00:00" + + def test_raises_when_reflection_does_not_exist(self, conn, fixed_clock): + svc = ReflectionService(conn, clock=fixed_clock) + with pytest.raises(ValueError, match="Reflection not found"): + svc.promote_to_general(reflection_id="nope") + + def test_raises_when_retired(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="retired", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + with pytest.raises(ValueError, match="Cannot promote reflection in status 'retired'"): + svc.promote_to_general(reflection_id="r1") + + def test_raises_when_superseded(self, conn, fixed_clock): + _seed_reflection(conn, "r1", status="superseded", scope="project") + svc = ReflectionService(conn, clock=fixed_clock) + with pytest.raises(ValueError, match="Cannot promote reflection in status 'superseded'"): + svc.promote_to_general(reflection_id="r1") From fcdc244fe7e31c8977c60fbb5f48d7e32f9cba00 Mon Sep 17 00:00:00 2001 From: gethin Date: Sat, 9 May 2026 22:56:41 +0100 Subject: [PATCH 5/7] feat(reflections): add POST /reflections//promote route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires ReflectionService.promote_to_general into a Flask route matching the reflection_confirm/retire pattern: 404 on missing, 409+card-error on ValueError, 200+HX-Trigger on success. Extends _seed_reflection with scope kwarg (default "project") for backward compat. Adds TestReflectionPromote with red→green TDD cycle. Co-Authored-By: Claude Opus 4.7 (1M context) --- better_memory/ui/app.py | 21 ++++++++++++++ tests/ui/test_reflections.py | 56 ++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/better_memory/ui/app.py b/better_memory/ui/app.py index f14d36c..8f6bed1 100644 --- a/better_memory/ui/app.py +++ b/better_memory/ui/app.py @@ -366,6 +366,27 @@ def reflection_edit_save(id: str) -> tuple[str, int, dict[str, str]]: ) return rendered, 200, {"HX-Trigger": "reflection-changed"} + @app.post("/reflections//promote") + def reflection_promote(id: str) -> tuple[str, int, dict[str, str]]: + conn = app.extensions["db_connection"] + if queries.reflection_detail(conn, reflection_id=id) is None: + abort(404) + try: + app.extensions["reflection_service"].promote_to_general( + reflection_id=id, + ) + except ValueError as exc: + return ( + f'
' + f"

{escape(str(exc))}

" + "
" + ), 409, {} + detail = queries.reflection_detail(conn, reflection_id=id) + rendered = render_template( + "fragments/reflection_drawer.html", detail=detail + ) + return rendered, 200, {"HX-Trigger": "reflection-changed"} + @app.get("/semantic") def semantic() -> str: return render_template( diff --git a/tests/ui/test_reflections.py b/tests/ui/test_reflections.py index a3f5ac4..1ba0806 100644 --- a/tests/ui/test_reflections.py +++ b/tests/ui/test_reflections.py @@ -24,18 +24,19 @@ def _seed_reflection( hints: str = "h", title: str | None = None, evidence_count: int = 0, + scope: str = "project", ) -> None: conn = connect(db_path) try: conn.execute( "INSERT INTO reflections " "(id, title, project, tech, phase, polarity, use_cases, hints, " - "confidence, status, evidence_count, created_at, updated_at) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, " + "confidence, status, evidence_count, scope, created_at, updated_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, " "'2026-04-26T10:00:00+00:00', '2026-04-26T10:00:00+00:00')", ( rid, title or f"title-{rid}", project, tech, phase, polarity, - use_cases, hints, confidence, status, evidence_count, + use_cases, hints, confidence, status, evidence_count, scope, ), ) conn.commit() @@ -372,3 +373,52 @@ def test_post_409_for_retired( headers={"Origin": "http://localhost"}, ) assert response.status_code == 409 + + +class TestReflectionPromote: + def test_promotes_project_pending( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="pending_review", scope="project") + + response = client.post( + "/reflections/r-1/promote", + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 200 + assert response.headers.get("HX-Trigger") == "reflection-changed" + + conn = connect(tmp_db) + try: + row = conn.execute( + "SELECT scope FROM reflections WHERE id = ?", ("r-1",) + ).fetchone() + finally: + conn.close() + assert row["scope"] == "general" + + def test_404_for_unknown(self, client: FlaskClient): + response = client.post( + "/reflections/does-not-exist/promote", + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 404 + + def test_409_for_retired( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="retired", scope="project") + + response = client.post( + "/reflections/r-1/promote", + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 409 + body = response.get_data(as_text=True) + assert "card-error" in body From 6d0a07e33e6949f9a1acda9e25d2cbcb1e1c3c1c Mon Sep 17 00:00:00 2001 From: gethin Date: Sat, 9 May 2026 23:06:11 +0100 Subject: [PATCH 6/7] feat(reflections): drawer shows scope and gated promote button Co-Authored-By: Claude Opus 4.7 (1M context) --- .../fragments/reflection_drawer.html | 10 ++++ tests/ui/test_reflections.py | 59 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/better_memory/ui/templates/fragments/reflection_drawer.html b/better_memory/ui/templates/fragments/reflection_drawer.html index 5859d14..7a6249f 100644 --- a/better_memory/ui/templates/fragments/reflection_drawer.html +++ b/better_memory/ui/templates/fragments/reflection_drawer.html @@ -18,6 +18,7 @@

{{ detail.reflection.title }}

Confidence
{{ '%.2f' | format(detail.reflection.confidence) }}
Status
{{ detail.reflection.status }}
+
Scope
{{ detail.reflection.scope }}
Evidence
{{ detail.reflection.evidence_count }} observation{{ 's' if detail.reflection.evidence_count != 1 else '' }}
Updated
{{ detail.reflection.updated_at }}
@@ -64,6 +65,15 @@

Hints

hx-swap="innerHTML"> Edit + {% if detail.reflection.scope == 'project' %} + + {% endif %} {% endif %} diff --git a/tests/ui/test_reflections.py b/tests/ui/test_reflections.py index 1ba0806..9f69940 100644 --- a/tests/ui/test_reflections.py +++ b/tests/ui/test_reflections.py @@ -422,3 +422,62 @@ def test_409_for_retired( assert response.status_code == 409 body = response.get_data(as_text=True) assert "card-error" in body + + +class TestReflectionDrawerScope: + def test_drawer_shows_scope_meta_and_promote_button_for_active_project( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection( + tmp_db, rid="r-1", status="pending_review", scope="project", + ) + response = client.get("/reflections/r-1/drawer") + assert response.status_code == 200 + body = response.get_data(as_text=True) + # Scope meta row visible. + assert "Scope" in body + assert ">project<" in body + # Promote button rendered. + assert "Promote to general" in body + assert "action-promote" in body + assert "/reflections/r-1/promote" in body + + def test_drawer_hides_promote_button_when_already_general( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection( + tmp_db, rid="r-1", status="pending_review", scope="general", + ) + response = client.get("/reflections/r-1/drawer") + assert response.status_code == 200 + body = response.get_data(as_text=True) + # Scope meta row still visible (now general). + assert "Scope" in body + assert ">general<" in body + # No promote button. + assert "Promote to general" not in body + assert "action-promote" not in body + + def test_drawer_hides_promote_button_when_retired( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection( + tmp_db, rid="r-1", status="retired", scope="project", + ) + response = client.get("/reflections/r-1/drawer") + assert response.status_code == 200 + body = response.get_data(as_text=True) + # Scope meta row still visible. + assert "Scope" in body + # No promote button (existing actions block already hidden on retired). + assert "Promote to general" not in body + assert "action-promote" not in body From 7d9d05d94dc158c1b5741e9bfdf608bf230f1519 Mon Sep 17 00:00:00 2001 From: gethin Date: Sun, 10 May 2026 09:08:33 +0100 Subject: [PATCH 7/7] fix: address final-review findings on promote-reflection branch - install_hooks.main() now passes --venv-py to merge_settings_json so the SessionStart bootstrap actually gets python.exe on Windows (the prior fix in 034823a only updated the merge function; main() silently used the back-compat fallback). Regression test added. - ReflectionService docstring updated to list promote_to_general as the fourth lifecycle action. - test_reflection_writes module docstring updated to mention promote_to_general. Co-Authored-By: Claude Opus 4.7 (1M context) --- better_memory/cli/install_hooks.py | 4 ++- better_memory/services/reflection.py | 7 +++-- tests/cli/test_install_hooks.py | 39 ++++++++++++++++++++++++ tests/services/test_reflection_writes.py | 2 +- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/better_memory/cli/install_hooks.py b/better_memory/cli/install_hooks.py index df8745f..f1fdb00 100644 --- a/better_memory/cli/install_hooks.py +++ b/better_memory/cli/install_hooks.py @@ -259,7 +259,9 @@ def main(argv: list[str] | None = None) -> None: ( "hooks", Path.home() / ".claude" / "settings.json", - lambda d: merge_settings_json(d, venv_pyw=args.venv_pyw), + lambda d: merge_settings_json( + d, venv_pyw=args.venv_pyw, venv_py=args.venv_py, + ), ), ] diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index 4b1b225..118dbf9 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -1180,7 +1180,7 @@ class ReflectionService: """UI-facing writes for reflections. Sibling of ``ReflectionSynthesisService``: this class does NOT - synthesise — it handles the three lifecycle actions the user + synthesise — it handles the four lifecycle actions the user drives from the Reflections tab drawer: - ``confirm``: pending_review → confirmed (idempotent on confirmed). @@ -1188,8 +1188,11 @@ class ReflectionService: - ``update_text``: edit use_cases / hints in place; blocked on retired and superseded so we don't surprise the synthesis pipeline by mutating retired text. + - ``promote_to_general``: project → general scope; idempotent on + already-general; blocked on retired and superseded so promoted-but- + invisible state can't slip into the cross-project pile. - All three bump ``updated_at`` only when the row actually changes + All four bump ``updated_at`` only when the row actually changes (no-op cases leave the timestamp untouched so reinforcement / audit trails stay honest). """ diff --git a/tests/cli/test_install_hooks.py b/tests/cli/test_install_hooks.py index 0d9b872..7c14d61 100644 --- a/tests/cli/test_install_hooks.py +++ b/tests/cli/test_install_hooks.py @@ -557,3 +557,42 @@ def test_summary_lines_printed_on_success(self, mock_home: Path) -> None: assert "MCP server" in result.stdout assert "hooks" in result.stdout assert "Restart Claude Code" in result.stdout + + +def test_main_passes_venv_py_separately_to_settings_merge( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """main() must forward --venv-py to merge_settings_json so the SessionStart + hook gets python.exe (not pythonw.exe) on Windows. Regression test for + the bug where main() only passed venv_pyw and silently relied on the + back-compat fallback.""" + from better_memory.cli import install_hooks + + fake_home = tmp_path / "home" + (fake_home / ".claude").mkdir(parents=True) + monkeypatch.setenv("HOME", str(fake_home)) + monkeypatch.setenv("USERPROFILE", str(fake_home)) # Path.home() on Windows + + bm_home = tmp_path / "bm-home" + (bm_home / "install-backups").mkdir(parents=True) + + install_hooks.main([ + "--venv-py", "C:/venv/python.exe", + "--venv-pyw", "C:/venv/pythonw.exe", + "--home", str(bm_home), + ]) + + settings = _json.loads( + (fake_home / ".claude" / "settings.json").read_text(encoding="utf-8") + ) + ss_cmd = settings["hooks"]["SessionStart"][0]["hooks"][0]["command"] + assert "python.exe" in ss_cmd and "pythonw.exe" not in ss_cmd, ( + f"SessionStart command must use python.exe, got: {ss_cmd!r}" + ) + + # Sanity: async PostToolUse + Stop hooks should still use pythonw.exe. + ptu_cmd = settings["hooks"]["PostToolUse"][0]["hooks"][0]["command"] + assert "pythonw.exe" in ptu_cmd + + stop_cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] + assert "pythonw.exe" in stop_cmd diff --git a/tests/services/test_reflection_writes.py b/tests/services/test_reflection_writes.py index c4d9c0e..370cda6 100644 --- a/tests/services/test_reflection_writes.py +++ b/tests/services/test_reflection_writes.py @@ -1,4 +1,4 @@ -"""Tests for ReflectionService (UI write actions: confirm / retire / update_text).""" +"""Tests for ReflectionService (UI write actions: confirm / retire / update_text / promote_to_general).""" from __future__ import annotations