From 378a4b8f99e8022f2670819d87266e602880c878 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 14:52:06 -0400 Subject: [PATCH 01/10] feat(ci): check the app's own components against the tokens, not just snippets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap the theme.css repoint exposed (#2319, part of #2277). `check_code_against_tokens` could always answer "does this code use the sheet correctly?" — it was only ever fed recorded SNIPPETS. The app's own components, where sixteen unresolvable references were living quietly, were checked by nothing at all. That was structural rather than an oversight: the drift panel runs in the browser and cannot read source files, and the server has no repo access. CI is the only place holding both the sources and the ability to run the check — and it only became cheap once theme.css became a generated artifact, so the source of truth is a committed file with no network and no credentials. **The sheet now carries its own SUPERSEDES block.** That is what keeps the checker instance-agnostic (rule #115): it knows nothing about any palette, and reads both the declarations and the discouraged literals out of whatever stylesheet it is pointed at. Hardcoding "#fff means use the text token" would have baked one install's kit into the tool. Two severities, split on whether the count is already zero: FAIL an unresolvable var() reference — zero today, so this is a ratchet holding a line already reached. It cannot false-positive either: the name is declared or it is not. REPORT superseded literals (32 files) and raw colour literals (246). Gating those means a permanently-red job, and a check nobody reads is worse than no check. **Comments are stripped before scanning, and that fired on the first real run.** A comment explaining why a literal is avoided necessarily contains that literal — DesignSystemsView's stylesheet documents exactly that about `#fff`, and the checker reported the explanation as a violation. A checker that flags the documentation of a rule teaches people to stop documenting rules. Also narrowed `--fs-weight-medium`'s supersedes to the keywords. `600` and `700` are real violations of the two-weight rule, but a bare number matches too much to find by literal scan — `z-index: 600` is not a font weight. That needs a property-aware check, which is a different tool. Verified end to end: the generator's output parses back through the checker's reader, so the two halves cannot drift into disagreeing about the format. --- .forgejo/workflows/ci.yml | 13 +- frontend/src/assets/theme.css | 8 ++ scripts/check_design_tokens.py | 174 ++++++++++++++++++++++ src/scribe/services/design_stylesheet.py | 21 +++ tests/test_check_design_tokens.py | 175 +++++++++++++++++++++++ 5 files changed, 390 insertions(+), 1 deletion(-) create mode 100644 scripts/check_design_tokens.py create mode 100644 tests/test_check_design_tokens.py diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index fc4986c..ad6a9d6 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -165,7 +165,18 @@ jobs: # ruff is pre-installed in the ci-python image — no install # step needed, lint runs in ~2s. - name: Lint - run: ruff check src/ + run: ruff check src/ scripts/ + + # Design tokens: does the frontend's CSS agree with the stylesheet the + # design system generates? Fails only on an unresolvable var() reference — + # that count is at zero, so this is a ratchet rather than a backlog. The + # literal findings are printed, not gated; hundreds exist and a + # permanently-red job is one nobody reads. + # + # Stdlib only, no install, no network: the source of truth is theme.css, + # which is generated from the design system and committed. + - name: Design token check + run: python3 scripts/check_design_tokens.py --report-literals test: name: Python tests diff --git a/frontend/src/assets/theme.css b/frontend/src/assets/theme.css index 1b26bfb..0484367 100644 --- a/frontend/src/assets/theme.css +++ b/frontend/src/assets/theme.css @@ -178,6 +178,14 @@ --fs-text-tertiary: #9A9890; } +/* SUPERSEDES — write the token, not the literal. + * #fff -> --fs-text-primary + * #ffffff -> --fs-text-primary + * white -> --fs-text-primary + * bold -> --fs-weight-medium + * bolder -> --fs-weight-medium + */ + /* ========================================================================== COMPATIBILITY ALIASES — the app's historical names, pointing at the system. diff --git a/scripts/check_design_tokens.py b/scripts/check_design_tokens.py new file mode 100644 index 0000000..558f2b1 --- /dev/null +++ b/scripts/check_design_tokens.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Check the frontend's CSS against the tokens its stylesheet declares. + +The gap this closes: `services/design_stylesheet.check_code_against_tokens` has +always been able to answer "does this code use the sheet correctly?", but the +only thing ever fed to it was recorded SNIPPETS. The app's own components — where +sixteen unresolvable references were found living quietly (#2319) — were checked +by nothing at all. + +That was structural rather than an oversight. The drift panel runs in the browser +and cannot read source files, and the server has no repo access. CI is the only +place holding both the component sources and the ability to run the check, and it +only became cheap once `theme.css` became a generated artifact — so the source of +truth is a local file, with no network and no credentials. + +INSTANCE-AGNOSTIC ON PURPOSE (rule #115). Nothing here knows what a token should +be called or which literals are discouraged. Both come from the stylesheet: the +declarations, and the `SUPERSEDES` block the generator emits. Point it at a +different install's sheet and it checks that install's rules. + +Two severities, and the split is deliberate: + + FAIL an unresolvable `var()` reference. Currently zero, so this is a ratchet + that holds a line already reached rather than a backlog that keeps CI + red. It also cannot false-positive: either the name is declared or it + is not. + REPORT superseded literals and raw colour literals. Hundreds today, so gating + on them would mean a permanently failing job that everyone learns to + ignore — which is worse than no check. +""" +from __future__ import annotations + +import argparse +import pathlib +import re +import sys + +# A declaration is `--name:`; a reference is `var(--name)` or `var(--name, …)`. +DECLARATION = re.compile(r"(?\s*(--[A-Za-z0-9_-]+)\s*$") +HEX_LITERAL = re.compile(r"#[0-9a-fA-F]{3,8}\b") +STYLE_BLOCK = re.compile(r"]*>(.*?)", re.S) +CSS_COMMENT = re.compile(r"/\*.*?\*/", re.S) + + +def declared_tokens(sheet: str) -> set[str]: + """Every custom property the stylesheet declares. + + Anchored on the colon alone. Anchoring on `{` or `;` instead silently drops + every declaration that follows a comment — a mistake made once already, which + lost `--color-bg` and 2 others without erroring. + """ + return set(DECLARATION.findall(sheet)) + + +def superseded_literals(sheet: str) -> dict[str, str]: + """`{literal: token}` from the generator's SUPERSEDES block, lowercased.""" + out: dict[str, str] = {} + for line in sheet.splitlines(): + match = SUPERSEDES_LINE.match(line) + if match: + out[match.group(1).lower()] = match.group(2) + return out + + +def _literal_pattern(literal: str) -> re.Pattern: + """Match a literal without matching a longer one containing it. + + `#fff` must not fire inside `#ffffff`: different colours, and a finding on + the wrong one sends someone to change correct code. + """ + return re.compile( + r"(? str: + """The CSS in a file — `\n' + ) + css = check.style_source(sfc) + assert "--fs-text-primary" in css + assert "#fff" not in css + + +def test_a_comment_explaining_a_rule_is_not_a_violation(tmp_path): + """LOAD-BEARING, and it fired on the first real run. A comment documenting + why a literal is avoided necessarily contains that literal — this codebase's + own stylesheet says so about `#fff`. A checker that flags the documentation + of a rule teaches people to stop documenting rules.""" + sfc = tmp_path / "Y.vue" + sfc.write_text( + "\n" + ) + css = check.style_source(sfc) + assert check._literal_pattern("#fff").search(css) is None + + +def test_a_plain_css_file_is_read_whole(tmp_path): + css_file = tmp_path / "shared.css" + css_file.write_text(".a { color: #fff; }") + assert "#fff" in check.style_source(css_file) + + +# --- the gate --------------------------------------------------------------- + +def _run(tmp_path, sheet: str, component: str, monkeypatch, capsys): + (tmp_path / "assets").mkdir(parents=True, exist_ok=True) + sheet_path = tmp_path / "assets" / "theme.css" + sheet_path.write_text(sheet) + (tmp_path / "C.vue").write_text(f"") + monkeypatch.setattr( + "sys.argv", + ["check", "--sheet", str(sheet_path), "--root", str(tmp_path)], + ) + code = check.main() + return code, capsys.readouterr().out + + +def test_an_unresolvable_reference_fails_the_build(tmp_path, monkeypatch, capsys): + """The one hard gate. It is safe to gate on because the count is zero today — + a ratchet holding a line already reached, not a backlog that keeps CI red.""" + code, out = _run(tmp_path, SHEET, ".a { color: var(--nope); }", monkeypatch, capsys) + assert code == 1 + assert "--nope" in out + + +def test_a_resolvable_reference_passes(tmp_path, monkeypatch, capsys): + code, out = _run( + tmp_path, SHEET, ".a { color: var(--fs-text-primary); }", monkeypatch, capsys + ) + assert code == 0 + assert "every var() reference resolves" in out + + +def test_a_locally_declared_property_is_not_unresolved(tmp_path, monkeypatch, capsys): + """A component may legitimately define its own custom property for local use — + a keyframe variable, a per-instance override. Only a reference to a name that + exists NOWHERE is broken.""" + code, _ = _run( + tmp_path, SHEET, ".a { --local: 4px; padding: var(--local); }", monkeypatch, capsys + ) + assert code == 0 + + +def test_a_superseded_literal_reports_but_does_not_fail(tmp_path, monkeypatch, capsys): + """Hundreds exist. Gating would make a permanently-red job, which is a check + nobody reads — worse than no check at all.""" + code, out = _run(tmp_path, SHEET, ".a { color: #fff; }", monkeypatch, capsys) + assert code == 0 + assert "#fff -> --fs-text-primary" in out + + +def test_a_missing_stylesheet_is_an_error_not_a_pass(tmp_path, monkeypatch, capsys): + """If the sheet moves, the check must fail loudly rather than silently + passing with zero tokens to compare against — which would look identical to + a clean run.""" + monkeypatch.setattr( + "sys.argv", ["check", "--sheet", str(tmp_path / "gone.css"), "--root", str(tmp_path)] + ) + assert check.main() == 2 From 6eedb0f6b955d99975d607bdb56b3371ecbd8968 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 20:39:20 -0400 Subject: [PATCH 02/10] fix(instructions): stop mandating a milestone for every non-trivial task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four product surfaces told the agent to call start_planning FIRST for any "non-trivial" work, while a fifth — the milestone bullet four lines up in the same file — had the criterion right: use one when the work has an arc. The loudest surface won, so sessions wrapped bug fixes and one-file changes in milestones that never meant anything. Mandating one project shape is what rule #115 forbids: some projects are milestone-shaped, others are a flat task list and always will be. Now the arc test is stated ONCE in full, in writing-plans, along with what to do when there is no arc (a task, driven by status and work-logs). The other surfaces name it and defer: - writing-plans/SKILL.md gains a "first decide whether this work wants a plan" section; its frontmatter trigger is the arc, not "non-trivial" - using-scribe reflex #4 points at the skill instead of restating it - server.py's Plan bullet adopts the milestone bullet's own criterion - server.py's planning paragraph drops from 11 lines to 6: it keeps the claim MCP instructions should make (a plan's HOME is a milestone, not a local .md) and drops the how, which the skill carries - start_planning's docstring gains the when Also removed "call start_planning FIRST — before any brainstorming, design, or plan-writing skill runs." That was the server asserting priority over the skill layer. Tools describe what they do; skills decide when they apply. The structural point outlasts the wording: a surface that restates a rule is a surface that will eventually contradict it, and nothing checks prose against prose. Closes #2322. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- plugin/skills/using-scribe/SKILL.md | 12 ++++++----- plugin/skills/writing-plans/SKILL.md | 30 ++++++++++++++++++++++------ src/scribe/mcp/server.py | 29 ++++++++++++--------------- src/scribe/mcp/tools/tasks.py | 6 ++++++ 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/plugin/skills/using-scribe/SKILL.md b/plugin/skills/using-scribe/SKILL.md index fab80e1..43184ea 100644 --- a/plugin/skills/using-scribe/SKILL.md +++ b/plugin/skills/using-scribe/SKILL.md @@ -64,11 +64,13 @@ Two constraints on *how* that's achieved: 3. **Update over duplicate.** When recording, prefer updating an existing note/rule/task over creating a new one. Search first; revise what's there. -4. **Plans live in Scribe.** For non-trivial work call `start_planning(project_id, - title)` FIRST — it creates a milestone whose `body` holds the design; each - step is its own task under that milestone (`create_task(milestone_id=...)`), - progress goes in work-logs (`add_task_log`). Read it back with `get_milestone`. - Do not write plans/specs to local `.md` files. +4. **When you plan, plan in Scribe.** Work with an *arc* — several steps toward + one goal — gets a plan, and a plan is a milestone: `start_planning(project_id, + title)` creates one whose `body` holds the design, each step is its own task + under it (`create_task(milestone_id=...)`), progress goes in work-logs + (`add_task_log`). Work without an arc (a fix, a one-file change, a question) + is just a task — don't wrap it in a milestone. Either way, do not write + plans/specs to local `.md` files. See the **writing-plans** skill. 5. **Keep state honest.** Set a task `in_progress` when you start it, `done` the moment it's complete; log progress as you go. diff --git a/plugin/skills/writing-plans/SKILL.md b/plugin/skills/writing-plans/SKILL.md index 8ef8d80..67bf6f7 100644 --- a/plugin/skills/writing-plans/SKILL.md +++ b/plugin/skills/writing-plans/SKILL.md @@ -1,6 +1,6 @@ --- name: writing-plans -description: Use before starting any non-trivial or multi-step piece of work — produce a clear plan BEFORE diving in. Triggers when the user asks you to plan, design an approach, scope an effort, or tackle work big enough to need ordered steps. The plan lives in a Scribe milestone (via start_planning), not a local file. +description: Use when a piece of work has an arc — several steps toward one goal, worth tracking as a unit — and you want the approach reviewable before you start. Triggers when the user asks you to plan, design an approach, or scope an effort, or when work is about to sprawl across several steps. Not for single-step work. The plan lives in a Scribe milestone (via start_planning), not a local file. --- # Writing plans @@ -9,12 +9,30 @@ A plan is **how** you'll execute a chunk of work — the design plus an ordered set of steps — written *before* you start, so the approach is reviewable and the work stays trackable. -## Start the plan in Scribe, not a file +## First decide whether this work wants a plan -For non-trivial work, call **`start_planning(project_id, title)` FIRST** — -before any design or implementation. It creates a **milestone** (the plan -container) seeded with a design template and returns the milestone id plus the -project's applicable rules. The plan lives in that milestone: +A plan lives in a milestone, and **a milestone earns its place when the work has +an arc**: several steps, one shared goal, a beginning and an end worth tracking +as a unit. That is the whole test, and it is a judgment about the *shape* of the +work — not about its size, difficulty, or importance. + +Plenty of real work has no arc. A bug fix, a one-file change, a question +answered, a setting changed. For those, a milestone is a container with one +thing in it: the ceremony costs more than it records, and it leaves the project +with milestones that never meant anything. **Use a task instead** — set it +`in_progress`, record what you find with `add_task_log`, set it `done`. That is +a complete, honest record of work that didn't need a plan. + +Some projects are milestone-shaped and some are a flat task list. Read the +project you are in rather than imposing a shape on it. + +## When it does: start the plan in Scribe, not a file + +Call **`start_planning(project_id, title)`** before designing or implementing — +so the milestone exists to write into, rather than being backfilled from work +already done. It creates a **milestone** (the plan container) seeded with a +design template and returns the milestone id plus the project's applicable +rules. The plan lives in that milestone: - The **design/intent** goes in the milestone `body` — edit it with `update_milestone(milestone_id, body=...)`. diff --git a/src/scribe/mcp/server.py b/src/scribe/mcp/server.py index a7f1633..79886dd 100644 --- a/src/scribe/mcp/server.py +++ b/src/scribe/mcp/server.py @@ -33,11 +33,12 @@ What each part is for, and when to reach for it: - Plan: a MILESTONE acting as a plan container — HOW you'll execute a chunk of work. The design/intent lives in the milestone `body`; each step is its own child task (create_task(milestone_id=...)), tracked with status + work-logs — - NOT a checkbox buried in the body. Start one with start_planning when - beginning non-trivial work, before you dive in; read it back with - get_milestone (body + steps). (The old kind=plan task is retired — some - historical plan-tasks still exist and remain readable, but don't create new - ones.) + NOT a checkbox buried in the body. Create one with start_planning when the + work has an arc (same test as a milestone, above) and you want the approach + reviewable before you start; read it back with get_milestone (body + steps). + Work without an arc is a task, not a plan. (The old kind=plan task is retired + — some historical plan-tasks still exist and remain readable, but don't + create new ones.) - Note: durable free-form knowledge — reference material, decisions, logs of what happened. No lifecycle, not actionable. Reach for one to CAPTURE something worth keeping. @@ -184,17 +185,13 @@ adopting or creating — never do either silently, and never guess a project int existence. Once a project is in scope, the enter_project handshake and the host-memory pointer step above both apply. -A plan is a MILESTONE, and Scribe is the canonical home for it. When you begin -non-trivial work, call start_planning(project_id, title) FIRST — before any -brainstorming, design, or plan-writing skill runs. start_planning creates the -milestone, seeds its `body` with the design template, returns the project's -applicable_rules, and gives you the milestone id you'll write into. Put the -design/intent in the milestone body via update_milestone(milestone_id, body=...); -create each step as a child task with create_task(milestone_id=...) and track it -with status + add_task_log — do NOT list steps as checkboxes in the body. Read -the plan back with get_milestone (body + steps). If a habit tells you to save a -plan or spec to a local `.md` file, that's superseded here: the milestone is the -record, not a local file. +When work DOES get a plan, Scribe is the plan's canonical home: it is a +milestone (see the Plan entry above), created with start_planning and written +into with update_milestone + child tasks. If a habit tells you to save a plan or +spec to a local `.md` file, that's superseded here — the milestone is the +record, not a file on disk. Whether a given piece of work wants a plan at all is +a separate question, answered by the arc test above and by the writing-plans +skill; these instructions do not mandate one. Deletes are recoverable: every delete_* tool moves the entity (and its descendants) to the trash and returns a deleted_batch_id. Use list_trash() to diff --git a/src/scribe/mcp/tools/tasks.py b/src/scribe/mcp/tools/tasks.py index ffbcb2a..8d633f0 100644 --- a/src/scribe/mcp/tools/tasks.py +++ b/src/scribe/mcp/tools/tasks.py @@ -276,6 +276,12 @@ async def add_task_log(task_id: int, content: str) -> dict: async def start_planning(project_id: int, title: str) -> dict: """Begin a plan in Scribe (the preferred home for plans — not a local .md file). + Reach for this when the work has an ARC — several steps toward one goal, + worth tracking as a unit. Work without one (a fix, a one-file change, a + question answered) is a task, not a plan: create_task, drive its status, and + record progress with add_task_log. A milestone holding a single step is + ceremony, and it leaves the project with a plan that never meant anything. + Creates a MILESTONE that IS the plan: its `body` is seeded with a design template (Goal/Approach/Verification) under the given project, and the call returns it together with the project's applicable Rulebook rules and brief From 1e139d0d183d2ea0039f6b3e308999633210f0c1 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 20:43:01 -0400 Subject: [PATCH 03/10] chore(plugin): bump to 0.1.21 for the planning-guidance edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two skills changed in 6eedb0f are shipped plugin content, and the installer compares manifest versions to decide whether to refresh the cache that actually executes. Without the bump the edits reach the repo and stop there (#2209) — which is exactly the silent no-op the check exists to catch. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- plugin/.claude-plugin/plugin.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 4b3cdd2..a486db8 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe system-of-record for Claude Code: MCP tools over your notes/tasks/projects/rules, a session-start push channel that surfaces your always-on rules + active-project context, process-skills (writing-plans, systematic-debugging, verification, brainstorming, reusing-code), and your saved Scribe Processes auto-surfaced as skills (/scribe:sync). Replaces superpowers + file-memory with one app-backed plugin.", - "version": "0.1.20", + "version": "0.1.21", "author": { "name": "Bryan Van Deusen" }, "mcpServers": { "scribe": { From 731ca284c30fc546ea2549093c3bc7182bdea1e2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 22:10:09 -0400 Subject: [PATCH 04/10] feat(design-systems): give a design system a way to reach the session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Storing a design system never made a session aware of one. Rules get pushed into every session by the SessionStart hook and returned by enter_project; a design system had neither, so its standards were reachable only by an agent that already knew to call resolve_design_system — the same silent failure as a token nobody declares. That gap was invisible while the operator's visual standards also lived in a rulebook. Retiring that rulebook (which is what this unblocks) would have deleted design guidance from every session with nothing to say so. - services/design_systems.design_context() — the delivery side. Guidance is chain-merged ANCESTOR-FIRST: a child system holds only what it CHANGES, so its own guidance describes a departure from a house style it never restates, and the leaf alone is a fragment. Tokens are summarised (count + group names), not listed — a hundred declarations would crowd out the context they are meant to inform. - enter_project returns `design_system`, null when the project has none. - The SessionStart context gains a Design system block with pointers to the values, alongside the always-on rules. - server.py's entity list gains Design system, including the negative: do NOT record one as a rulebook, because a token kept as prose cannot be resolved, inherited, rendered or checked. - The rulebook-tier passage used "a design-system rulebook" as its worked example of a subscribed rulebook — it now teaches the opposite, plus a new "is this a rule at all?" test pointing at design systems, processes and snippets. - using-scribe gains a section on building UI against the project's system. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- plugin/skills/using-scribe/SKILL.md | 22 ++++++- src/scribe/mcp/server.py | 25 +++++++- src/scribe/mcp/tools/projects.py | 17 ++++- src/scribe/services/design_systems.py | 62 ++++++++++++++++++ src/scribe/services/plugin_context.py | 32 ++++++++++ tests/test_mcp_tool_projects.py | 45 ++++++++++++- tests/test_services_design_systems.py | 91 +++++++++++++++++++++++++++ tests/test_services_plugin_context.py | 68 +++++++++++++++++++- 8 files changed, 356 insertions(+), 6 deletions(-) diff --git a/plugin/skills/using-scribe/SKILL.md b/plugin/skills/using-scribe/SKILL.md index 43184ea..8562d58 100644 --- a/plugin/skills/using-scribe/SKILL.md +++ b/plugin/skills/using-scribe/SKILL.md @@ -104,7 +104,7 @@ shared homes general: norms that bind *every* project. Cross-project standards only. - **Subscribed rulebook** (`create_rule` + `subscribe_project_to_rulebook`) — a reusable, *themed* module of general rules that binds only projects that opt - in (e.g. a design system → visual apps). Themed, but still project-agnostic. + in (e.g. a review checklist → every service). Themed, but project-agnostic. - **Project rule** (`create_project_rule`) — anything specific to one project (its files, paths, quirks). @@ -114,6 +114,26 @@ project rule; a standard a category shares → subscribed rulebook; a universal norm → always-on rulebook. Never put project-specific detail in a shared rulebook — it leaks to every other project that gets it. +**First ask whether it's a rule at all.** A rule is prose you have to remember +and apply; Scribe's other entities are structure a tool can resolve and check. +Visual standards belong in a **design system**, not a rulebook — a token can be +inherited, resolved per mode, rendered to a stylesheet and diffed against code, +and none of that survives being written as a rule. A repeatable procedure is a +**process**; reusable code is a **snippet**. Reach for a rule when the thing +really is a standing instruction about how to work. + +## Building UI: the project's design system binds + +`enter_project` returns a `design_system` when the project has one, with the +guidance **chain-merged** — the house style it inherits plus its own departures +from it. Treat it the way you treat a rule. + +Before writing a colour, size, radius, weight or duration by hand, reach for a +token: `resolve_design_system(id)` for the values, or +`get_design_system_stylesheet(id)` for the rendered sheet. A literal is a value +stated outside the system, so it can never follow a palette change — and +nothing will tell you it drifted. + ## Other Scribe process-skills This plugin also ships focused process-skills — writing-plans, systematic diff --git a/src/scribe/mcp/server.py b/src/scribe/mcp/server.py index 79886dd..7a7f55b 100644 --- a/src/scribe/mcp/server.py +++ b/src/scribe/mcp/server.py @@ -42,6 +42,18 @@ What each part is for, and when to reach for it: - Note: durable free-form knowledge — reference material, decisions, logs of what happened. No lifecycle, not actionable. Reach for one to CAPTURE something worth keeping. +- Design system: the visual standards a project's UI is built from — design + tokens (name + value per mode) plus the prose a token table cannot hold + (aesthetic, voice, what is out of scope). Systems INHERIT: a child holds only + what it changes and the chain supplies the rest, so a family's house style and + one app's departures from it are the same structure at two depths. A project + points at one with set_project_design_system, and enter_project then hands it + back with the guidance chain-merged. Treat it as binding for UI work: reach + for a token (resolve_design_system / get_design_system_stylesheet) before + writing a colour, size, radius or duration by hand. Do NOT record a design + system as a rulebook — rules are for behaviour, and tokens kept as prose + cannot be resolved, inherited, rendered to a stylesheet, or checked against + code. - System: a per-project, reusable, self-describing subsystem/area. Associate any record (note, task, issue) with it via system_ids so research, build-work, and fixes for the same area line up, and recurring problem-spots surface. Manage @@ -144,8 +156,8 @@ right altitude: subscribe_project_to_rulebook) — a reusable, THEMED module of general rules that binds only the projects which subscribe. Its rules must make sense for every project that could subscribe, never one specific project - (e.g. a design-system rulebook: design-specific but project-agnostic — no - rule names a single app). + (e.g. a code-review checklist, or a compliance regime a category of + projects shares — no rule names a single app). - Project rule (create_project_rule) — anything specific to ONE project. Both rulebook tiers are SHARED, so their rules stay general; the difference between them is REACH (all projects vs opt-in by theme), not generality. Rule @@ -153,6 +165,15 @@ of thumb: names a specific project's files/paths/quirks -> project rule; a standard a CATEGORY of projects shares -> subscribed rulebook; a universal norm -> always-on rulebook. Coordinate with the operator on which home fits. +Before writing a rule, check whether another entity already models the thing. +A rule is prose an agent must remember and apply; the other entities are +structure a tool can resolve, render and check. Visual standards are a DESIGN +SYSTEM, not a rulebook — a token can be inherited, resolved per mode, rendered +to a stylesheet and diffed against code, and none of that survives being +written as a rule. A repeatable procedure is a PROCESS. Reusable code is a +SNIPPET. Reach for a rule when the thing genuinely is a standing instruction +about how to work, and nothing else can hold it. + One thing NOT to do: don't bridge Scribe into a session by writing to the host's native memory. Rules are pull-only, so a fresh session won't reach for them unless its always-loaded context says to — but the bridge for that is the diff --git a/src/scribe/mcp/tools/projects.py b/src/scribe/mcp/tools/projects.py index 7557074..3d26ab1 100644 --- a/src/scribe/mcp/tools/projects.py +++ b/src/scribe/mcp/tools/projects.py @@ -17,6 +17,7 @@ keeps working. from __future__ import annotations from scribe.mcp._context import current_user_id +from scribe.services import design_systems as design_systems_svc from scribe.services import milestones as milestones_svc from scribe.services import notes as notes_svc from scribe.services import projects as projects_svc @@ -53,7 +54,13 @@ async def enter_project(project_id: int) -> dict: Returns a dict with keys: project, milestone_summary, applicable_rules, project_rules, subscribed_rulebooks, applicable_rules_truncated, - open_tasks, recent_notes. + open_tasks, recent_notes, design_system. + + `design_system` is null unless the project points at one. When present it + carries the chain-merged guidance (the house style AND this project's + departures from it) plus a summary of the token set — treat it as binding + for any UI you write, and pull the values with resolve_design_system or + get_design_system_stylesheet before reaching for a literal. """ uid = current_user_id() project = await projects_svc.get_project(uid, project_id) @@ -74,9 +81,17 @@ async def enter_project(project_id: int) -> dict: uid, is_task=False, project_id=project_id, sort="updated_at", limit=5, ) + # A project need not have one, and most installs won't — null is ordinary + # here, not a missing prerequisite. + design_system = None + if project.design_system_id: + design_system = await design_systems_svc.design_context( + uid, project.design_system_id, + ) return { "project": project.to_dict(), + "design_system": design_system, "milestone_summary": milestone_summary, "applicable_rules": applicable["rules"], "project_rules": applicable.get("project_rules", []), diff --git a/src/scribe/services/design_systems.py b/src/scribe/services/design_systems.py index 4c4ef35..e4bb63b 100644 --- a/src/scribe/services/design_systems.py +++ b/src/scribe/services/design_systems.py @@ -282,6 +282,68 @@ async def resolve_design_system( return resolve_tokens(design_system_id, parents, tokens_by_system) +async def design_context(user_id: int, design_system_id: int) -> dict | None: + """What a session needs to know about a design system, before it writes UI. + + This is the DELIVERY side of a design system, and it exists because storing + one does not make a session aware of it. Rules get pushed into every session + by the plugin's SessionStart hook; a design system had no such channel, so + the standards were reachable only by an agent that already knew to go + looking — which is the same silent failure as a token nobody declares. + + Guidance is chain-merged, ANCESTOR-FIRST, and that is the point rather than + a convenience. A child system holds only what it CHANGES, so its own + guidance describes a departure from a house style it never restates. Hand an + agent the leaf alone and it builds against a fragment, with no signal that + the rest exists. + + Tokens are summarised, not listed: the count and the group names are enough + to know what the system covers, and the full set is one call away. Sending + a hundred token values into every session start would crowd out the context + it is meant to inform. + + None when the caller may not read the system. + """ + tokens = await resolve_design_system(user_id, design_system_id) + if tokens is None: + return None + + async with async_session() as session: + system = await session.get(DesignSystem, design_system_id) + if system is None or system.deleted_at is not None: + return None + parents = await _parent_map(session, system.owner_user_id) + chain = ancestry(design_system_id, parents) + rows = ( + await session.execute( + select(DesignSystem).where(DesignSystem.id.in_(chain)) + ) + ).scalars().all() + + by_id = {s.id: s for s in rows} + return { + "id": system.id, + "title": system.title, + "description": system.description or "", + # Outermost ancestor first, so the reader meets the house style before + # the app's departures from it. + "inherits_from": [ + by_id[sid].title for sid in reversed(chain[1:]) if sid in by_id + ], + "guidance": [ + { + "design_system_id": sid, + "title": by_id[sid].title, + "guidance": (by_id[sid].guidance or "").strip(), + } + for sid in reversed(chain) + if sid in by_id and (by_id[sid].guidance or "").strip() + ], + "token_count": len(tokens), + "token_groups": sorted({t.group_name for t in tokens if t.group_name}), + } + + async def update_token( user_id: int, token_id: int, **fields: object ) -> DesignToken | None: diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 93eda36..f07d311 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -22,6 +22,7 @@ from sqlalchemy import select from scribe.models import async_session from scribe.models.rulebook import RulebookTopic +from scribe.services import design_systems as design_systems_svc from scribe.services import knowledge as knowledge_svc from scribe.services import notes as notes_svc from scribe.services import projects as projects_svc @@ -837,6 +838,37 @@ async def build_session_context( f"Goal: {goal[:200]}" if goal else "", f"Open todo tasks: {open_count}", ] + + # A design system binds the same way a rule does, and until this + # existed it had no push channel — the standards were reachable only + # by an agent that already knew to look for them. Summary only: the + # token VALUES are a tool call away, and pasting a hundred of them + # into every session would crowd out the context they inform. + if project.design_system_id: + design = await design_systems_svc.design_context( + user_id, project.design_system_id, + ) + if design: + inherits = ( + " (inherits " + " › ".join(design["inherits_from"]) + ")" + if design["inherits_from"] else "" + ) + groups = ", ".join(design["token_groups"]) + lines += [ + "", + f"## Design system: {design['title']} " + f"(id {design['id']}){inherits}", + f"{design['token_count']} tokens" + + (f" across {groups}" if groups else "") + + ". This project's UI is built from these, not from " + "literals — reach for a token before writing a colour, " + "size, radius or duration by hand.", + f"Values: `resolve_design_system({design['id']})` · " + f"stylesheet: `get_design_system_stylesheet({design['id']})` " + f"· the prose (aesthetic, voice, where the accent may " + f"appear): `enter_project` returns it, or " + f"`get_design_system({design['id']})`.", + ] elif unbound_repo: lines += [ "", diff --git a/tests/test_mcp_tool_projects.py b/tests/test_mcp_tool_projects.py index 5328f74..3b52c4e 100644 --- a/tests/test_mcp_tool_projects.py +++ b/tests/test_mcp_tool_projects.py @@ -17,12 +17,16 @@ def _bind_user(): _user_id_ctx.reset(token) -def _fake_project(**overrides) -> MagicMock: +def _fake_project(design_system_id=None, **overrides) -> MagicMock: p = MagicMock() base = {"id": 1, "title": "P", "description": "", "goal": "", "status": "active", "color": None} base.update(overrides) p.to_dict.return_value = base + # Explicit, because a bare MagicMock hands back a truthy auto-attribute — + # which would route every project in this file through the design-system + # branch and out to a real database. + p.design_system_id = design_system_id return p @@ -176,6 +180,45 @@ async def test_enter_project_composes_full_context(): assert out["open_tasks"][0]["id"] == 100 assert out["open_tasks"][0]["status"] == "in_progress" assert out["recent_notes"][0]["id"] == 200 + # No design system on this project -> the key is present and null, not + # absent. A caller that has to distinguish "no key" from "no system" will + # eventually get it wrong. + assert out["design_system"] is None + + +@pytest.mark.asyncio +async def test_enter_project_hands_back_the_design_system_when_the_project_has_one(): + """The handshake is where an agent learns what binds it, and a design + system binds the same way a rule does. Before this it was reachable only by + an agent that already knew to call resolve_design_system — so the standards + were present in the store and absent from the work.""" + p = _fake_project(id=5, design_system_id=9) + design = {"id": 9, "title": "App kit", "guidance": [{"title": "House"}], + "token_count": 95, "token_groups": ["surface"], + "inherits_from": ["House"], "description": ""} + + with patch( + "scribe.mcp.tools.projects.projects_svc.get_project", + AsyncMock(return_value=p), + ), patch( + "scribe.mcp.tools.projects.rulebooks_svc.get_applicable_rules", + AsyncMock(return_value={"rules": [], "truncated": False, + "subscribed_rulebooks": []}), + ), patch( + "scribe.mcp.tools.projects.milestones_svc.get_project_milestone_summary", + AsyncMock(return_value=[]), + ), patch( + "scribe.mcp.tools.projects.notes_svc.list_notes", + AsyncMock(side_effect=[([], 0), ([], 0)]), + ), patch( + "scribe.mcp.tools.projects.design_systems_svc.design_context", + AsyncMock(return_value=design), + ) as ctx: + out = await enter_project(project_id=5) + + assert out["design_system"]["token_count"] == 95 + assert out["design_system"]["inherits_from"] == ["House"] + assert ctx.await_args.args == (7, 9) # caller's id, the project's system @pytest.mark.asyncio diff --git a/tests/test_services_design_systems.py b/tests/test_services_design_systems.py index 83f0339..303ff31 100644 --- a/tests/test_services_design_systems.py +++ b/tests/test_services_design_systems.py @@ -321,3 +321,94 @@ async def test_stylesheet_reports_what_the_sheet_cannot_say_for_itself(): assert result["duplicates"] == {"#4a5d3f": ["--fs-moss", "--fs-success"]} assert "--fs-moss: #4a5d3f;" in result["css"] assert "FabledSword" in result["css"] + + +# --- design_context: the delivery side -------------------------------------- + +def _ctx_token(name: str, group: str | None): + from scribe.services.design_cascade import ResolvedToken + return ResolvedToken( + name=name, contributions={}, group_name=group, + purpose=None, rationale=None, supersedes=(), order_index=0, + ) + + +@pytest.mark.asyncio +async def test_design_context_merges_guidance_ANCESTOR_FIRST(): + """LOAD-BEARING. A child system holds only what it CHANGES, so its own + guidance describes a departure from a house style it never restates. An + agent handed the leaf alone builds against a fragment with no signal that + the rest exists — which is exactly the failure retiring the design rulebook + would otherwise have caused. + """ + family = MagicMock(id=1, deleted_at=None, owner_user_id=42, + description="the house", guidance="Dark-mode-first.") + family.title = "House" + app = MagicMock(id=3, deleted_at=None, owner_user_id=42, + description="one app", guidance="Accent on the wordmark.") + app.title = "App" + + mock_session = _make_mock_session() + mock_session.get = AsyncMock(return_value=app) + mock_session.execute = AsyncMock(return_value=MagicMock( + scalars=MagicMock(return_value=MagicMock(all=lambda: [app, family])) + )) + + with patch("scribe.services.design_systems.async_session") as mock_cls, \ + patch("scribe.services.design_systems._parent_map", + AsyncMock(return_value={3: 1, 1: None})), \ + patch("scribe.services.design_systems.resolve_design_system", + AsyncMock(return_value=[ + _ctx_token("--a", "surface"), _ctx_token("--b", "type"), + _ctx_token("--c", "surface"), _ctx_token("--d", None), + ])): + mock_cls.return_value = mock_session + from scribe.services.design_systems import design_context + out = await design_context(user_id=42, design_system_id=3) + + assert [g["title"] for g in out["guidance"]] == ["House", "App"] + assert out["inherits_from"] == ["House"] + assert out["token_count"] == 4 + # Group names deduped and sorted; an ungrouped token contributes nothing. + assert out["token_groups"] == ["surface", "type"] + + +@pytest.mark.asyncio +async def test_design_context_denied_returns_none(): + """It rides on resolve_design_system's ACL rather than re-deriving one — + a second permission path is a second thing to get wrong.""" + with patch("scribe.services.design_systems.resolve_design_system", + AsyncMock(return_value=None)): + from scribe.services.design_systems import design_context + assert await design_context(user_id=1, design_system_id=3) is None + + +@pytest.mark.asyncio +async def test_design_context_omits_systems_with_no_guidance(): + """A system that overrides one token and says nothing about it should not + contribute an empty section — a heading with nothing under it reads as + missing content rather than as an absence of content.""" + family = MagicMock(id=1, deleted_at=None, owner_user_id=42, + description="", guidance="Dark-mode-first.") + family.title = "House" + app = MagicMock(id=3, deleted_at=None, owner_user_id=42, + description="", guidance=" ") + app.title = "App" + + mock_session = _make_mock_session() + mock_session.get = AsyncMock(return_value=app) + mock_session.execute = AsyncMock(return_value=MagicMock( + scalars=MagicMock(return_value=MagicMock(all=lambda: [app, family])) + )) + + with patch("scribe.services.design_systems.async_session") as mock_cls, \ + patch("scribe.services.design_systems._parent_map", + AsyncMock(return_value={3: 1, 1: None})), \ + patch("scribe.services.design_systems.resolve_design_system", + AsyncMock(return_value=[])): + mock_cls.return_value = mock_session + from scribe.services.design_systems import design_context + out = await design_context(user_id=42, design_system_id=3) + + assert [g["title"] for g in out["guidance"]] == ["House"] + assert out["token_count"] == 0 diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index 42c19b6..e73ca26 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -130,7 +130,11 @@ async def test_build_session_context_renders_titles_grouped_by_topic(): @pytest.mark.asyncio async def test_build_session_context_includes_project_when_scoped(): - project = MagicMock(id=2, title="FabledScribe", goal="ship it") + # design_system_id explicitly None: a bare MagicMock would hand back a truthy + # auto-attribute and send this through the design branch, which is the + # opposite of what this test is about. + project = MagicMock(id=2, title="FabledScribe", goal="ship it", + design_system_id=None) with patch("scribe.services.plugin_context.rulebooks_svc.list_always_on_rules", AsyncMock(return_value=[_rule(1, "rule", 1)])), \ patch("scribe.services.plugin_context._topic_titles", @@ -145,6 +149,68 @@ async def test_build_session_context_includes_project_when_scoped(): assert out["project"] == {"id": 2, "title": "FabledScribe"} assert "## Active project: FabledScribe (id 2)" in out["context"] assert "Open todo tasks: 4" in out["context"] + # No design system on the project -> no design block at all. An install with + # none is the ordinary case, not a degraded one. + assert "## Design system" not in out["context"] + + +@pytest.mark.asyncio +async def test_build_session_context_pushes_the_projects_design_system(): + """The gap this closes: a design system had no push channel, so its + standards reached a session only if the agent already knew to go looking — + the same silent failure as a token nobody declares.""" + project = MagicMock(id=2, title="App", goal="", design_system_id=9) + design = { + "id": 9, "title": "App kit", "description": "", + "inherits_from": ["House"], + "guidance": [], "token_count": 95, + "token_groups": ["accent", "surface", "type"], + } + with patch("scribe.services.plugin_context.rulebooks_svc.list_always_on_rules", + AsyncMock(return_value=[_rule(1, "rule", 1)])), \ + patch("scribe.services.plugin_context._topic_titles", + AsyncMock(return_value={1: "git-workflow"})), \ + patch("scribe.services.plugin_context.projects_svc.get_project", + AsyncMock(return_value=project)), \ + patch("scribe.services.plugin_context.notes_svc.list_notes", + AsyncMock(return_value=([], 0))), \ + patch("scribe.services.plugin_context.design_systems_svc.design_context", + AsyncMock(return_value=design)): + from scribe.services.plugin_context import build_session_context + out = await build_session_context(user_id=7, project_id=2) + + ctx = out["context"] + assert "## Design system: App kit (id 9) (inherits House)" in ctx + assert "95 tokens across accent, surface, type" in ctx + # A pointer to the values, never the values themselves — a hundred token + # declarations would crowd out the context they are meant to inform. The + # summary carries counts and group NAMES only, which is why design_context + # returns those rather than the resolved tokens. + assert "resolve_design_system(9)" in ctx + assert "get_design_system_stylesheet(9)" in ctx + + +@pytest.mark.asyncio +async def test_build_session_context_survives_an_unreadable_design_system(): + """design_context returns None when the caller may not read the system. + That must degrade to "no design block", not to a crash that costs the + session its rules too.""" + project = MagicMock(id=2, title="App", goal="", design_system_id=9) + with patch("scribe.services.plugin_context.rulebooks_svc.list_always_on_rules", + AsyncMock(return_value=[_rule(1, "rule", 1)])), \ + patch("scribe.services.plugin_context._topic_titles", + AsyncMock(return_value={1: "git-workflow"})), \ + patch("scribe.services.plugin_context.projects_svc.get_project", + AsyncMock(return_value=project)), \ + patch("scribe.services.plugin_context.notes_svc.list_notes", + AsyncMock(return_value=([], 0))), \ + patch("scribe.services.plugin_context.design_systems_svc.design_context", + AsyncMock(return_value=None)): + from scribe.services.plugin_context import build_session_context + out = await build_session_context(user_id=7, project_id=2) + + assert "## Design system" not in out["context"] + assert "## Active project: App (id 2)" in out["context"] @pytest.mark.asyncio From 4c9a637507fb0abbf7b98c2ad47a413b020c8422 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 22:40:58 -0400 Subject: [PATCH 05/10] =?UTF-8?q?fix(theme):=20text=20on=20a=20filled=20co?= =?UTF-8?q?lour=20needs=20its=20own=20token=20=E2=80=94=20the=20old=20one?= =?UTF-8?q?=20inverts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 76 hardcoded `color: #fff` now resolve to --fs-text-on-action, a new token that is parchment in BOTH modes. The design system said they should supersede to --fs-text-primary, on the recorded reasoning that "there is no 'text on action' colour, there is just the text colour." That is true on dark and wrong on light. --fs-text-primary inverts to #14171A; the surfaces underneath it do not invert at all — every one of these 76 sits on an action colour, a semantic colour, the accent, or the CTA gradient, all of which hold a single value across modes. Sweeping as recorded would have put obsidian text on moss green: roughly 2.4:1, against a house style whose stated floor is WCAG AA. It would have looked correct to me, because I checked it in the mode where it was correct. --color-accent-fg had the same defect independently and is repointed too. The token check now reports zero superseded literals, down from 30 files, and raw colour literals drop 246 -> 169. Closes #2275. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- frontend/src/App.vue | 2 +- frontend/src/assets/editor-shared.css | 16 +++++++------- frontend/src/assets/theme.css | 14 +++++++----- frontend/src/components/HistoryPanel.vue | 2 +- frontend/src/components/InlineAssistPanel.vue | 2 +- frontend/src/components/NotificationBell.vue | 2 +- frontend/src/components/PaginationBar.vue | 2 +- frontend/src/components/ShareDialog.vue | 4 ++-- frontend/src/components/SystemsSection.vue | 6 ++--- frontend/src/components/TaskLogSection.vue | 4 ++-- frontend/src/components/ToastNotification.vue | 4 ++-- .../src/components/VersionHistorySection.vue | 2 +- .../src/components/WorkspaceNoteEditor.vue | 6 ++--- .../src/components/WorkspaceTaskPanel.vue | 4 ++-- frontend/src/views/DesignSystemsView.vue | 7 +----- frontend/src/views/ForgotPasswordView.vue | 2 +- frontend/src/views/KnowledgeView.vue | 2 +- frontend/src/views/LoginView.vue | 2 +- frontend/src/views/NoteEditorView.vue | 2 +- frontend/src/views/NoteViewerView.vue | 8 +++---- frontend/src/views/ProjectListView.vue | 6 ++--- frontend/src/views/ProjectView.vue | 22 +++++++++---------- frontend/src/views/RegisterInviteView.vue | 2 +- frontend/src/views/RegisterView.vue | 2 +- frontend/src/views/ResetPasswordView.vue | 2 +- frontend/src/views/SettingsView.vue | 20 ++++++++--------- frontend/src/views/SnippetDetailView.vue | 2 +- frontend/src/views/SnippetEditorView.vue | 2 +- frontend/src/views/SnippetListView.vue | 6 ++--- frontend/src/views/TaskEditorView.vue | 2 +- frontend/src/views/TaskViewerView.vue | 8 +++---- frontend/src/views/UserManagementView.vue | 6 ++--- 32 files changed, 86 insertions(+), 87 deletions(-) diff --git a/frontend/src/App.vue b/frontend/src/App.vue index d84293e..25b887f 100644 --- a/frontend/src/App.vue +++ b/frontend/src/App.vue @@ -255,7 +255,7 @@ onUnmounted(() => { z-index: 9999; padding: 0.4rem 0.75rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border-radius: 0 0 4px 4px; font-size: 0.875rem; text-decoration: none; diff --git a/frontend/src/assets/editor-shared.css b/frontend/src/assets/editor-shared.css index bfff033..dcd044a 100644 --- a/frontend/src/assets/editor-shared.css +++ b/frontend/src/assets/editor-shared.css @@ -56,7 +56,7 @@ .btn-save { padding: 0.45rem 1.1rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -76,7 +76,7 @@ .btn-delete { padding: 0.45rem 1rem; background: var(--color-action-destructive); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -187,12 +187,12 @@ } .tag-pill:hover:not(:disabled) { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); } .tag-pill.applied { background: var(--color-success, #2ecc71); border-color: var(--color-success, #2ecc71); - color: #fff; + color: var(--fs-text-on-action); cursor: default; } .tag-check { @@ -345,7 +345,7 @@ .btn-generate { padding: 0.4rem 0.9rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -478,7 +478,7 @@ .btn-accept { padding: 0.4rem 1rem; background: var(--color-success); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -537,7 +537,7 @@ } .modal-btn-danger { background: var(--color-danger); - color: #fff; + color: var(--fs-text-on-action); border-color: var(--color-danger); } @@ -548,7 +548,7 @@ transform: translateX(-50%); padding: 0.3rem 0.75rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/assets/theme.css b/frontend/src/assets/theme.css index 0484367..2b65328 100644 --- a/frontend/src/assets/theme.css +++ b/frontend/src/assets/theme.css @@ -132,9 +132,10 @@ --fs-table-stripe: color-mix(in srgb, var(--fs-text-primary) 3%, transparent); /* text */ - --fs-text-primary: #E8E4D8; /* also the text colour on filled action buttons */ + --fs-text-primary: #E8E4D8; /* body, headings, labels — inverts by mode */ --fs-text-secondary: #C2BFB4; --fs-text-tertiary: #9C9A92; + --fs-text-on-action: #E8E4D8; /* text on a filled colour — NOT mode-dependent */ /* type */ --fs-size-display: 40px; @@ -179,9 +180,9 @@ } /* SUPERSEDES — write the token, not the literal. - * #fff -> --fs-text-primary - * #ffffff -> --fs-text-primary - * white -> --fs-text-primary + * #fff -> --fs-text-on-action + * #ffffff -> --fs-text-on-action + * white -> --fs-text-on-action * bold -> --fs-weight-medium * bolder -> --fs-weight-medium */ @@ -290,7 +291,10 @@ visual shifts exactly where a fallback had drifted; that shift IS the fix. ------------------------------------------------------------------ */ --color-accent: var(--fs-accent); - --color-accent-fg: var(--fs-text-primary); + /* Foreground ON the accent, so it follows the accent's mode-independence, + not the page text's. Pointing this at --fs-text-primary made it invert to + obsidian on light — over a mid-tone accent, well under the AA floor. */ + --color-accent-fg: var(--fs-text-on-action); --color-hover: var(--fs-surface-hover); --color-bg-hover: var(--fs-surface-hover); --color-bg-tertiary: var(--fs-surface-hover); diff --git a/frontend/src/components/HistoryPanel.vue b/frontend/src/components/HistoryPanel.vue index 5e4cd21..c91a9c8 100644 --- a/frontend/src/components/HistoryPanel.vue +++ b/frontend/src/components/HistoryPanel.vue @@ -396,7 +396,7 @@ onMounted(loadVersions); .btn-restore { padding: 0.45rem 1rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/components/InlineAssistPanel.vue b/frontend/src/components/InlineAssistPanel.vue index 4fcaffc..28a7340 100644 --- a/frontend/src/components/InlineAssistPanel.vue +++ b/frontend/src/components/InlineAssistPanel.vue @@ -192,7 +192,7 @@ const markers: Record = { } .iap-btn-accept { background: var(--color-success, #22c55e); - color: #fff; + color: var(--fs-text-on-action); } .iap-btn-accept:hover { opacity: 0.85; } diff --git a/frontend/src/components/NotificationBell.vue b/frontend/src/components/NotificationBell.vue index 7f4f25d..d24c659 100644 --- a/frontend/src/components/NotificationBell.vue +++ b/frontend/src/components/NotificationBell.vue @@ -82,7 +82,7 @@ onUnmounted(() => { top: -5px; right: -5px; background: var(--color-danger, #ef4444); - color: #fff; + color: var(--fs-text-on-action); font-size: 0.6rem; font-weight: 700; min-width: 16px; diff --git a/frontend/src/components/PaginationBar.vue b/frontend/src/components/PaginationBar.vue index 05145d9..f55f702 100644 --- a/frontend/src/components/PaginationBar.vue +++ b/frontend/src/components/PaginationBar.vue @@ -90,7 +90,7 @@ function goToPage(page: number) { } .page-btn.active { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border-color: var(--color-primary); } .ellipsis { diff --git a/frontend/src/components/ShareDialog.vue b/frontend/src/components/ShareDialog.vue index 305f8b9..f68d8b8 100644 --- a/frontend/src/components/ShareDialog.vue +++ b/frontend/src/components/ShareDialog.vue @@ -268,7 +268,7 @@ onMounted(async () => { .share-tab.active { background: var(--color-primary); border-color: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); } .share-target-form { @@ -339,7 +339,7 @@ onMounted(async () => { .btn-add-share { padding: 0.45rem 1rem; background: var(--gradient-cta); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: 6px; font-size: 0.85rem; diff --git a/frontend/src/components/SystemsSection.vue b/frontend/src/components/SystemsSection.vue index 46ebb2a..cb9d63c 100644 --- a/frontend/src/components/SystemsSection.vue +++ b/frontend/src/components/SystemsSection.vue @@ -390,7 +390,7 @@ async function confirmDelete() { .btn-confirm { padding: 0.35rem 0.8rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -405,7 +405,7 @@ async function confirmDelete() { padding: 0.35rem 0.8rem; background: var(--color-action-secondary); border: none; - color: #fff; + color: var(--fs-text-on-action); border-radius: var(--radius-sm); cursor: pointer; font-size: 0.82rem; @@ -555,6 +555,6 @@ async function confirmDelete() { font-family: inherit; } .modal-btn:hover { background: var(--color-bg); } -.modal-btn-danger { background: var(--color-action-destructive); border-color: var(--color-action-destructive); color: #fff; } +.modal-btn-danger { background: var(--color-action-destructive); border-color: var(--color-action-destructive); color: var(--fs-text-on-action); } .modal-btn-danger:hover { background: var(--color-action-destructive-hover); border-color: var(--color-action-destructive-hover); } diff --git a/frontend/src/components/TaskLogSection.vue b/frontend/src/components/TaskLogSection.vue index a0eed59..3f734e0 100644 --- a/frontend/src/components/TaskLogSection.vue +++ b/frontend/src/components/TaskLogSection.vue @@ -217,7 +217,7 @@ onMounted(loadLogs); .log-duration-badge { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border-radius: 99px; padding: 0.1rem 0.5rem; font-size: 0.72rem; @@ -312,7 +312,7 @@ onMounted(loadLogs); margin-left: auto; padding: 0.3rem 0.75rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/components/ToastNotification.vue b/frontend/src/components/ToastNotification.vue index 341cd05..b82e02b 100644 --- a/frontend/src/components/ToastNotification.vue +++ b/frontend/src/components/ToastNotification.vue @@ -36,7 +36,7 @@ const toastStore = useToastStore(); gap: 0.5rem; padding: 0.75rem 1rem; border-radius: 6px; - color: #fff; + color: var(--fs-text-on-action); font-size: 0.9rem; box-shadow: 0 2px 8px var(--color-shadow); min-width: 200px; @@ -54,7 +54,7 @@ const toastStore = useToastStore(); padding: 0 0.15rem; } .toast-close:hover { - color: #fff; + color: var(--fs-text-on-action); } .toast--success { background: var(--color-toast-success); diff --git a/frontend/src/components/VersionHistorySection.vue b/frontend/src/components/VersionHistorySection.vue index 1faf4fd..6113d25 100644 --- a/frontend/src/components/VersionHistorySection.vue +++ b/frontend/src/components/VersionHistorySection.vue @@ -241,7 +241,7 @@ function restore() { border-radius: var(--radius-sm); padding: 0.25rem 0.6rem; font-size: 0.78rem; - color: #fff; + color: var(--fs-text-on-action); cursor: pointer; font-family: inherit; } diff --git a/frontend/src/components/WorkspaceNoteEditor.vue b/frontend/src/components/WorkspaceNoteEditor.vue index 5ad88f5..975c71e 100644 --- a/frontend/src/components/WorkspaceNoteEditor.vue +++ b/frontend/src/components/WorkspaceNoteEditor.vue @@ -643,7 +643,7 @@ defineExpose({ reload: loadProjectNotes }); border-radius: 3px; transition: background 0.15s, color 0.15s; } -.btn-confirm-delete:hover:not(:disabled) { background: var(--color-action-destructive); color: #fff; } +.btn-confirm-delete:hover:not(:disabled) { background: var(--color-action-destructive); color: var(--fs-text-on-action); } .btn-confirm-delete:disabled { opacity: 0.5; cursor: default; } .btn-cancel-delete { @@ -671,7 +671,7 @@ defineExpose({ reload: loadProjectNotes }); .btn-confirm { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: 4px; padding: 0.15rem 0.35rem; @@ -733,7 +733,7 @@ defineExpose({ reload: loadProjectNotes }); /* Moss action-primary per Hybrid */ .btn-save { background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: 5px; padding: 0.25rem 0.7rem; diff --git a/frontend/src/components/WorkspaceTaskPanel.vue b/frontend/src/components/WorkspaceTaskPanel.vue index 8634ee8..9504478 100644 --- a/frontend/src/components/WorkspaceTaskPanel.vue +++ b/frontend/src/components/WorkspaceTaskPanel.vue @@ -398,7 +398,7 @@ defineExpose({ reload: loadAll }); .btn-add { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: 5px; padding: 0.28rem 0.55rem; @@ -590,7 +590,7 @@ defineExpose({ reload: loadAll }); margin-left: 0.25rem; transition: background 0.15s, color 0.15s; } -.btn-delete-confirm:hover:not(:disabled) { background: var(--color-action-destructive); color: #fff; } +.btn-delete-confirm:hover:not(:disabled) { background: var(--color-action-destructive); color: var(--fs-text-on-action); } .btn-delete-confirm:disabled { opacity: 0.5; cursor: default; } .btn-delete-cancel { diff --git a/frontend/src/views/DesignSystemsView.vue b/frontend/src/views/DesignSystemsView.vue index a11b09f..76fd9f8 100644 --- a/frontend/src/views/DesignSystemsView.vue +++ b/frontend/src/views/DesignSystemsView.vue @@ -1273,14 +1273,9 @@ textarea.input { border: 1px solid transparent; } -/* Background matches the house convention (--color-action-primary, as in - ProjectListView and four others). The text colour deliberately does NOT: - every existing copy uses `color: #fff`, which is 67 live violations of the - rule that pure white is never text (#2275). Parchment is what the rulebook - actually specifies. */ .btn-primary { background: var(--color-action-primary); - color: #E8E4D8; + color: var(--fs-text-on-action); } .btn-primary:not(:disabled):hover { diff --git a/frontend/src/views/ForgotPasswordView.vue b/frontend/src/views/ForgotPasswordView.vue index 2a69590..e274c38 100644 --- a/frontend/src/views/ForgotPasswordView.vue +++ b/frontend/src/views/ForgotPasswordView.vue @@ -141,7 +141,7 @@ async function handleSubmit() { width: 100%; padding: 0.6rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/KnowledgeView.vue b/frontend/src/views/KnowledgeView.vue index 7d5d54d..11b37aa 100644 --- a/frontend/src/views/KnowledgeView.vue +++ b/frontend/src/views/KnowledgeView.vue @@ -574,7 +574,7 @@ onUnmounted(() => { border-radius: 10px; border: none; background: var(--gradient-cta); - color: #fff; + color: var(--fs-text-on-action); cursor: pointer; font-size: 0.85rem; font-weight: 500; diff --git a/frontend/src/views/LoginView.vue b/frontend/src/views/LoginView.vue index 0b06bba..4098d42 100644 --- a/frontend/src/views/LoginView.vue +++ b/frontend/src/views/LoginView.vue @@ -180,7 +180,7 @@ function loginWithOAuth() { width: 100%; padding: 0.6rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/NoteEditorView.vue b/frontend/src/views/NoteEditorView.vue index f439afa..8dd5890 100644 --- a/frontend/src/views/NoteEditorView.vue +++ b/frontend/src/views/NoteEditorView.vue @@ -748,7 +748,7 @@ onUnmounted(() => assist.clearSelection()); cursor: pointer; font-family: inherit; } -.btn-link-all:hover { background: var(--color-primary); color: #fff; } +.btn-link-all:hover { background: var(--color-primary); color: var(--fs-text-on-action); } .link-suggest-list { display: flex; diff --git a/frontend/src/views/NoteViewerView.vue b/frontend/src/views/NoteViewerView.vue index ab9e47f..f12200d 100644 --- a/frontend/src/views/NoteViewerView.vue +++ b/frontend/src/views/NoteViewerView.vue @@ -354,7 +354,7 @@ async function convertToTask() { border: none; border-radius: var(--radius-sm); background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); text-decoration: none; cursor: pointer; font-size: 0.875rem; @@ -363,14 +363,14 @@ async function convertToTask() { } .btn-edit:hover { background: var(--color-action-primary-hover); - color: #fff; + color: var(--fs-text-on-action); } /* Convert + Share: Bronze action-secondary — alternate paths */ .btn-convert { margin-left: auto; padding: 0.3rem 0.75rem; background: var(--color-action-secondary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -388,7 +388,7 @@ async function convertToTask() { background: var(--color-action-secondary); border: none; border-radius: var(--radius-sm); - color: #fff; + color: var(--fs-text-on-action); cursor: pointer; font-size: 0.85rem; font-family: inherit; diff --git a/frontend/src/views/ProjectListView.vue b/frontend/src/views/ProjectListView.vue index 4c38dfe..f4f3d8a 100644 --- a/frontend/src/views/ProjectListView.vue +++ b/frontend/src/views/ProjectListView.vue @@ -287,7 +287,7 @@ function overallPct(project: Project): { total: number; pct: number } { .btn-primary { padding: 0.45rem 1rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -341,7 +341,7 @@ function overallPct(project: Project): { total: number; pct: number } { .empty-title { font-size: 1rem; font-weight: 500; color: var(--color-text-secondary); margin: 0 0 0.35rem; } .empty-sub { font-size: 0.85rem; margin: 0 0 1rem; } .empty-action { display: inline-block; padding: 0.4rem 1rem; border: 1px solid var(--color-primary); border-radius: var(--radius-sm); color: var(--color-primary); background: none; cursor: pointer; font-size: 0.85rem; transition: background 0.15s, color 0.15s; } -.empty-action:hover { background: var(--color-primary); color: #fff; } +.empty-action:hover { background: var(--color-primary); color: var(--fs-text-on-action); } .skeleton-card { height: 140px; @@ -594,7 +594,7 @@ function overallPct(project: Project): { total: number; pct: number } { .modal-btn-primary { background: var(--color-primary); border-color: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); } .modal-btn-primary:hover:not(:disabled) { opacity: 0.9; diff --git a/frontend/src/views/ProjectView.vue b/frontend/src/views/ProjectView.vue index 14aeca2..9785df1 100644 --- a/frontend/src/views/ProjectView.vue +++ b/frontend/src/views/ProjectView.vue @@ -879,7 +879,7 @@ async function confirmDelete() { gap: 0.35rem; padding: 0.45rem 1rem; background: var(--gradient-cta); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); font-size: 0.875rem; @@ -888,14 +888,14 @@ async function confirmDelete() { box-shadow: var(--glow-cta); transition: box-shadow 0.15s, opacity 0.15s; } -.btn-workspace:hover { box-shadow: var(--glow-cta-hover); opacity: 0.95; color: #fff; } +.btn-workspace:hover { box-shadow: var(--glow-cta-hover); opacity: 0.95; color: var(--fs-text-on-action); } /* Share: Bronze action-secondary — alternate path */ .btn-share { padding: 0.4rem 0.8rem; background: var(--color-action-secondary); border: none; - color: #fff; + color: var(--fs-text-on-action); border-radius: var(--radius-sm); cursor: pointer; font-size: 0.85rem; @@ -917,7 +917,7 @@ async function confirmDelete() { font-family: inherit; transition: background 0.15s, color 0.15s; } -.btn-danger-outline:hover { background: var(--color-action-destructive); color: #fff; } +.btn-danger-outline:hover { background: var(--color-action-destructive); color: var(--fs-text-on-action); } .error-msg { color: var(--color-danger); font-size: 0.9rem; } @@ -1065,7 +1065,7 @@ async function confirmDelete() { .btn-save-panel { padding: 0.45rem 0.9rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -1151,7 +1151,7 @@ async function confirmDelete() { .btn-ms-confirm { padding: 0.3rem 0.65rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -1165,7 +1165,7 @@ async function confirmDelete() { padding: 0.3rem 0.65rem; background: var(--color-action-secondary); border: none; - color: #fff; + color: var(--fs-text-on-action); border-radius: var(--radius-sm); cursor: pointer; font-size: 0.78rem; @@ -1224,7 +1224,7 @@ async function confirmDelete() { cursor: pointer; border: 1px solid var(--color-border); } -.ms-plan-actions .btn-primary { background: var(--color-primary); color: #fff; border-color: var(--color-primary); } +.ms-plan-actions .btn-primary { background: var(--color-primary); color: var(--fs-text-on-action); border-color: var(--color-primary); } .ms-plan-actions .btn-primary:disabled { opacity: 0.6; cursor: default; } .ms-plan-actions .btn-secondary { background: var(--color-bg-card); color: var(--color-text); } @@ -1393,8 +1393,8 @@ async function confirmDelete() { line-height: 1; } .task-card:hover .task-advance-btn { opacity: 1; } -.task-advance-btn:hover { background: var(--color-primary); border-color: var(--color-primary); color: #fff; } -.task-advance-btn--done:hover { background: var(--color-success, #22c55e); border-color: var(--color-success, #22c55e); color: #fff; } +.task-advance-btn:hover { background: var(--color-primary); border-color: var(--color-primary); color: var(--fs-text-on-action); } +.task-advance-btn--done:hover { background: var(--color-success, #22c55e); border-color: var(--color-success, #22c55e); color: var(--fs-text-on-action); } .task-advance-btn:disabled { opacity: 0.4; cursor: default; } .priority-dot { @@ -1475,7 +1475,7 @@ async function confirmDelete() { font-family: inherit; } .modal-btn:hover { background: var(--color-bg); } -.modal-btn-danger { background: var(--color-action-destructive); border-color: var(--color-action-destructive); color: #fff; } +.modal-btn-danger { background: var(--color-action-destructive); border-color: var(--color-action-destructive); color: var(--fs-text-on-action); } .modal-btn-danger:hover { background: var(--color-action-destructive-hover); border-color: var(--color-action-destructive-hover); } /* ── Skeleton ────────────────────────────────────────────────── */ diff --git a/frontend/src/views/RegisterInviteView.vue b/frontend/src/views/RegisterInviteView.vue index 6ee9eed..de7d162 100644 --- a/frontend/src/views/RegisterInviteView.vue +++ b/frontend/src/views/RegisterInviteView.vue @@ -251,7 +251,7 @@ async function handleSubmit() { width: 100%; padding: 0.6rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/RegisterView.vue b/frontend/src/views/RegisterView.vue index 3d71ebe..24112a4 100644 --- a/frontend/src/views/RegisterView.vue +++ b/frontend/src/views/RegisterView.vue @@ -220,7 +220,7 @@ async function handleSubmit() { width: 100%; padding: 0.6rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/ResetPasswordView.vue b/frontend/src/views/ResetPasswordView.vue index a7e2816..92a4af8 100644 --- a/frontend/src/views/ResetPasswordView.vue +++ b/frontend/src/views/ResetPasswordView.vue @@ -199,7 +199,7 @@ async function handleSubmit() { width: 100%; padding: 0.6rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index fbcf524..390ecca 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -2567,7 +2567,7 @@ function formatUserDate(iso: string): string { .btn-save { padding: 0.4rem 0.9rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -2595,7 +2595,7 @@ function formatUserDate(iso: string): string { } .btn-danger-outline:hover:not(:disabled) { background: var(--color-action-destructive); - color: #fff; + color: var(--fs-text-on-action); } .btn-danger-outline:disabled { opacity: 0.5; cursor: default; } @@ -2603,7 +2603,7 @@ function formatUserDate(iso: string): string { .btn-danger { padding: 0.4rem 0.9rem; background: var(--color-action-destructive); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -2620,7 +2620,7 @@ function formatUserDate(iso: string): string { .btn-secondary { padding: 0.4rem 0.9rem; background: var(--color-action-secondary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -2685,7 +2685,7 @@ function formatUserDate(iso: string): string { .db-health-table tr.dh-warn td:first-child code { color: var(--color-warning); } .btn-warn:hover:not(:disabled) { background: var(--color-warning); - color: #fff; + color: var(--fs-text-on-action); } .saved-msg { @@ -3000,7 +3000,7 @@ function formatUserDate(iso: string): string { .btn-confirm-delete { padding: 0.25rem 0.6rem; background: var(--color-action-destructive); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -3033,7 +3033,7 @@ function formatUserDate(iso: string): string { transition: background 0.15s; } .btn-toggle:disabled { opacity: 0.6; cursor: default; } -.btn-toggle-open { background: var(--color-action-primary); color: #fff; } +.btn-toggle-open { background: var(--color-action-primary); color: var(--fs-text-on-action); } .btn-toggle-open:hover:not(:disabled) { background: var(--color-action-primary-hover); } .btn-toggle-close { background: var(--color-bg-secondary); @@ -3151,7 +3151,7 @@ function formatUserDate(iso: string): string { .btn-primary { padding: 0.4rem 0.9rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -3391,7 +3391,7 @@ function formatUserDate(iso: string): string { } .unit-btn.active { background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); } .unit-btn:hover:not(.active) { color: var(--color-text); @@ -3863,7 +3863,7 @@ function formatUserDate(iso: string): string { } .btn-danger-outline:hover:not(:disabled) { background: var(--color-action-destructive); - color: #fff; + color: var(--fs-text-on-action); } .btn-danger-outline:disabled { opacity: 0.5; cursor: default; } @keyframes va-dot-bounce { diff --git a/frontend/src/views/SnippetDetailView.vue b/frontend/src/views/SnippetDetailView.vue index 82bea87..70cd7cd 100644 --- a/frontend/src/views/SnippetDetailView.vue +++ b/frontend/src/views/SnippetDetailView.vue @@ -268,7 +268,7 @@ async function confirmDelete() { padding: 0.35rem 0.8rem; border: none; background: var(--color-action-destructive, #6B2118); - color: #fff; + color: var(--fs-text-on-action); border-radius: var(--radius-sm); cursor: pointer; font-size: 0.85rem; diff --git a/frontend/src/views/SnippetEditorView.vue b/frontend/src/views/SnippetEditorView.vue index 7420b6d..c672fe6 100644 --- a/frontend/src/views/SnippetEditorView.vue +++ b/frontend/src/views/SnippetEditorView.vue @@ -575,7 +575,7 @@ function cancel() { .btn-primary { padding: 0.5rem 1.1rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/SnippetListView.vue b/frontend/src/views/SnippetListView.vue index 411df7f..6f7606b 100644 --- a/frontend/src/views/SnippetListView.vue +++ b/frontend/src/views/SnippetListView.vue @@ -546,7 +546,7 @@ function usageTitle(s: SnippetListItem): string { .btn-primary { padding: 0.45rem 1rem; background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -679,7 +679,7 @@ function usageTitle(s: SnippetListItem): string { } .empty-action:hover { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); } .skeleton-grid, @@ -1021,7 +1021,7 @@ function usageTitle(s: SnippetListItem): string { .modal-btn-primary { background: var(--color-action-primary); border-color: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); } .modal-btn-primary:hover:not(:disabled) { background: var(--color-action-primary-hover); diff --git a/frontend/src/views/TaskEditorView.vue b/frontend/src/views/TaskEditorView.vue index 4617777..6032a11 100644 --- a/frontend/src/views/TaskEditorView.vue +++ b/frontend/src/views/TaskEditorView.vue @@ -971,7 +971,7 @@ useEditorGuards(dirty, save); .btn-subtask-confirm { padding: 0.25rem 0.5rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; diff --git a/frontend/src/views/TaskViewerView.vue b/frontend/src/views/TaskViewerView.vue index f9065ef..44e7fbf 100644 --- a/frontend/src/views/TaskViewerView.vue +++ b/frontend/src/views/TaskViewerView.vue @@ -501,7 +501,7 @@ const subTaskProgress = computed(() => { border: none; border-radius: var(--radius-sm); background: var(--color-action-primary); - color: #fff; + color: var(--fs-text-on-action); text-decoration: none; cursor: pointer; font-size: 0.875rem; @@ -511,14 +511,14 @@ const subTaskProgress = computed(() => { .btn-edit:hover, .btn-advance:hover { background: var(--color-action-primary-hover); - color: #fff; + color: var(--fs-text-on-action); } /* Convert + Share: Bronze action-secondary — alternate paths */ .btn-convert { margin-left: auto; padding: 0.3rem 0.75rem; background: var(--color-action-secondary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -536,7 +536,7 @@ const subTaskProgress = computed(() => { background: var(--color-action-secondary); border: none; border-radius: var(--radius-sm); - color: #fff; + color: var(--fs-text-on-action); cursor: pointer; font-size: 0.85rem; font-family: inherit; diff --git a/frontend/src/views/UserManagementView.vue b/frontend/src/views/UserManagementView.vue index 95a165d..1fb1721 100644 --- a/frontend/src/views/UserManagementView.vue +++ b/frontend/src/views/UserManagementView.vue @@ -331,7 +331,7 @@ function formatDate(iso: string): string { .btn-invite { padding: 0.45rem 1rem; background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; @@ -395,7 +395,7 @@ function formatDate(iso: string): string { } .btn-toggle-open { background: var(--color-primary); - color: #fff; + color: var(--fs-text-on-action); } .btn-toggle-open:hover:not(:disabled) { opacity: 0.9; @@ -498,7 +498,7 @@ function formatDate(iso: string): string { .btn-confirm-delete { padding: 0.25rem 0.6rem; background: var(--color-danger); - color: #fff; + color: var(--fs-text-on-action); border: none; border-radius: var(--radius-sm); cursor: pointer; From 5c51e29f269805894dc9d527c1c936b78d42d600 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 22:46:57 -0400 Subject: [PATCH 06/10] fix(embeddings): embed in the service, so every caller gets it (#2056) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A note or task created through MCP was not semantically searchable until the next restart's backfill ran. Embedding fired at the five REST route handlers and nowhere else; the MCP tools call the service directly, so they skipped it. The shape of this bug is the reason to care: it is invisible on an instance that redeploys constantly (this one does, per rule 46) and permanent on one that doesn't. Rule 115 — the product has to stand up for the install that restarts twice a year, not just for the one that restarts hourly. Moved to services/notes.embed_note(), called from create_note and update_note, and deleted from all five routes. Every caller — REST, MCP, recurrence, snippets — now gets it by construction rather than by remembering. Two things fall out of having one implementation instead of six: - It uses note.user_id, the OWNER. The routes were inconsistent: some passed the caller's uid, some the owner's. On a shared record the caller's id mints a second embedding row that nothing reads. - services/snippets.py's _embed_snippet existed only because snippets are created via MCP and the routes couldn't cover them. Every one of its four call sites goes through notes_svc, so the helper and its four calls are gone, along with the eight test patches that existed to neutralise it. RuntimeError (no running loop — unit tests, scripts) and any indexing failure are both swallowed: a write that succeeded must not be failed by its index. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- src/scribe/routes/notes.py | 11 ----- src/scribe/routes/tasks.py | 8 ---- src/scribe/services/notes.py | 38 +++++++++++++++ src/scribe/services/snippets.py | 26 ----------- tests/test_services_notes.py | 64 ++++++++++++++++++++++++++ tests/test_shared_write_access.py | 6 +-- tests/test_snippet_merge_provenance.py | 9 ++-- tests/test_snippet_unmerge.py | 3 -- 8 files changed, 107 insertions(+), 58 deletions(-) create mode 100644 tests/test_services_notes.py diff --git a/src/scribe/routes/notes.py b/src/scribe/routes/notes.py index b70cc44..3dcca5a 100644 --- a/src/scribe/routes/notes.py +++ b/src/scribe/routes/notes.py @@ -1,8 +1,6 @@ -import asyncio import logging import re -from scribe.services.embeddings import upsert_note_embedding from quart import Blueprint, jsonify, request @@ -113,9 +111,6 @@ async def create_note_route(): ) except ValueError as e: return jsonify({"error": str(e)}), 400 - text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") - if text: - asyncio.create_task(upsert_note_embedding(note.id, uid, text)) return jsonify(note.to_dict()), 201 @@ -221,9 +216,6 @@ async def update_note_route(note_id: int): return jsonify({"error": str(e)}), 400 if note is None: return not_found("Note") - text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") - if text: - asyncio.create_task(upsert_note_embedding(note.id, owner_uid, text)) return jsonify(note.to_dict()) @@ -259,9 +251,6 @@ async def patch_note_route(note_id: int): return jsonify({"error": str(e)}), 400 if note is None: return not_found("Note") - text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") - if text: - asyncio.create_task(upsert_note_embedding(note.id, owner_uid, text)) return jsonify(note.to_dict()) diff --git a/src/scribe/routes/tasks.py b/src/scribe/routes/tasks.py index c76f4f5..d564b55 100644 --- a/src/scribe/routes/tasks.py +++ b/src/scribe/routes/tasks.py @@ -1,4 +1,3 @@ -import asyncio from datetime import date from quart import Blueprint, jsonify, request @@ -8,7 +7,6 @@ from scribe.models.note import TaskPriority, TaskStatus from scribe.routes.utils import not_found, parse_iso_date, parse_pagination from scribe.services.access import can_write_note from scribe.services import systems as systems_svc -from scribe.services.embeddings import upsert_note_embedding from scribe.services.notes import ( create_note, get_note_for_user, @@ -149,9 +147,6 @@ async def create_task_route(): ) if data.get("system_ids") is not None: await systems_svc.set_record_systems(uid, task.id, data["system_ids"]) - text = f"{task.title}\n{task.body}".strip() if task.body else (task.title or "") - if text: - asyncio.create_task(upsert_note_embedding(task.id, uid, text)) out = task.to_dict() out["systems"] = [s.to_dict() for s in await systems_svc.list_record_systems(uid, task.id)] return jsonify(out), 201 @@ -255,9 +250,6 @@ async def update_task_route(task_id: int): return not_found("Task") if data.get("system_ids") is not None: await systems_svc.set_record_systems(uid, task_id, data["system_ids"]) - text = f"{task.title}\n{task.body}".strip() if task.body else (task.title or "") - if text: - asyncio.create_task(upsert_note_embedding(task.id, task_note.user_id, text)) out = task.to_dict() out["systems"] = [s.to_dict() for s in await systems_svc.list_record_systems(uid, task_id)] return jsonify(out) diff --git a/src/scribe/services/notes.py b/src/scribe/services/notes.py index f36d08b..1644787 100644 --- a/src/scribe/services/notes.py +++ b/src/scribe/services/notes.py @@ -10,6 +10,40 @@ from scribe.models.note import Note, TaskPriority, TaskStatus logger = logging.getLogger(__name__) +def embed_note(note) -> None: + """Refresh a note's embedding, fire-and-forget. + + Lives HERE — at the service, not the route — so every caller gets it by + construction. Previously each REST route made this call itself and the MCP + tools did not, so a record created through MCP stayed out of semantic search + and auto-inject until the next restart's backfill ran (#2056). That is + invisible on an instance that redeploys constantly and permanent on one that + doesn't, which is the worst shape a bug can have: it only appears where + nobody is looking. + + Uses `note.user_id` — the OWNER — rather than the caller. Embeddings belong + to the record, and a collaborator editing a shared note must refresh the + owner's row rather than mint a second one under their own id. + + Import is lazy so importing this module doesn't pull in the embedding model; + exceptions are swallowed because a record that saved must not fail on its + index refresh. No running loop (unit tests, scripts) is an ordinary case, + not an error. + """ + text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") + if not text: + return + try: + import asyncio + + from scribe.services.embeddings import upsert_note_embedding + asyncio.create_task(upsert_note_embedding(note.id, note.user_id, text)) + except RuntimeError: + pass # no running loop — a sync caller, not a failure + except Exception: # noqa: BLE001 - never let indexing break a write + logger.exception("embedding refresh failed for note %s", note.id) + + def _normalize_tags(tags: list[str]) -> list[str]: """Lowercase, strip, deduplicate, and drop empty tags.""" seen: set[str] = set() @@ -115,6 +149,8 @@ async def create_note( await session.commit() await session.refresh(note) + embed_note(note) + if project_id is not None: await _maybe_reactivate_project(project_id) @@ -329,6 +365,8 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No from scribe.services.note_versions import create_version await create_version(user_id, note_id, old_body, old_title, old_tags) + embed_note(note) + if note.project_id is not None: await _maybe_reactivate_project(note.project_id) diff --git a/src/scribe/services/snippets.py b/src/scribe/services/snippets.py index fc95425..5be773a 100644 --- a/src/scribe/services/snippets.py +++ b/src/scribe/services/snippets.py @@ -28,7 +28,6 @@ came from. """ from __future__ import annotations -import asyncio import hashlib import logging import re @@ -52,25 +51,6 @@ SNIPPET_TAG = "snippet" UNSET: object = object() -def _embed_snippet(note) -> None: - """Fire-and-forget embedding refresh for a snippet. - - A snippet's whole value is *immediate* recall — it must join the semantic / - auto-inject pool the moment it's recorded, not wait for the startup backfill. - Unlike a plain note (embedded at the REST-route boundary only, so its MCP - create path defers to restart-backfill), a snippet is recorded primarily via - MCP, so we embed here in the service — covering BOTH the MCP tool and the - REST route by construction. Mirrors the route pattern: fire-and-forget, - text = title + body. Import lazily so the pure serialize/parse helpers can be - imported without pulling in the embedding model. - """ - text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "") - if not text: - return - from scribe.services.embeddings import upsert_note_embedding - asyncio.create_task(upsert_note_embedding(note.id, note.user_id, text)) - - # --- serialize: structured fields -> note (title/body/tags) ------------------ def compose_title(name: str, when_to_use: str = "") -> str: @@ -640,7 +620,6 @@ async def create_snippet( language=language, code=code, locations=locations, ), ) - _embed_snippet(note) return note @@ -801,9 +780,6 @@ async def update_snippet( # As the OWNER: update_note is owner-scoped, so a shared editor's own id # would find nothing. The write was authorised by can_write_note above. updated = await notes_svc.update_note(note.user_id, snippet_id, **fields) - if updated is not None: - # Title/body changed → refresh the embedding so recall reflects the edit. - _embed_snippet(updated) return updated @@ -1025,7 +1001,6 @@ async def merge_snippets(user_id: int, target_id: int, source_ids: list[int]): if batch is not None: merged_ids.append(s.id) - _embed_snippet(updated) return updated, merged_ids @@ -1134,5 +1109,4 @@ async def unmerge_snippet(user_id: int, survivor_id: int, source_id: int): ) if updated is None: return None - _embed_snippet(updated) return updated, restored diff --git a/tests/test_services_notes.py b/tests/test_services_notes.py new file mode 100644 index 0000000..da74997 --- /dev/null +++ b/tests/test_services_notes.py @@ -0,0 +1,64 @@ +"""services/notes.py — the inline embedding moved here from the routes (#2056). + +Why it moved: embedding was triggered at each REST route and nowhere else, so a +record created through MCP stayed out of semantic search and auto-inject until +the next restart's backfill. Putting it in the service means every caller — REST, +MCP, recurrence, snippets — gets it by construction rather than by remembering. + +These test the helper directly. The point of the change is that there is now ONE +place to test. +""" +from scribe.services import notes as notes_svc + +# --- inline embedding (#2056) ----------------------------------------------- + +def test_embed_note_uses_the_OWNER_not_the_caller(): + """LOAD-BEARING for shared records. An embedding belongs to the record; a + collaborator editing a shared note must refresh the owner's row rather than + mint a second one under their own id. The routes this replaced passed the + caller's uid on one path and the owner's on another — exactly the kind of + inconsistency that moving it to one place removes.""" + from unittest.mock import MagicMock, patch + + note = MagicMock(id=5, user_id=42, title="T", body="B") + with patch("scribe.services.embeddings.upsert_note_embedding") as upsert, \ + patch("asyncio.create_task") as create_task: + notes_svc.embed_note(note) + + assert create_task.called + upsert.assert_called_once() + assert upsert.call_args.args[0] == 5 + assert upsert.call_args.args[1] == 42 # owner, never the caller + assert upsert.call_args.args[2] == "T\nB" + + +def test_embed_note_skips_a_record_with_no_text(): + """An empty embedding is worse than none — it is a row that matches nothing + and hides the fact that the record was never indexed.""" + from unittest.mock import MagicMock, patch + + note = MagicMock(id=5, user_id=42, title="", body="") + with patch("asyncio.create_task") as create_task: + notes_svc.embed_note(note) + assert not create_task.called + + +def test_embed_note_without_a_running_loop_is_not_an_error(): + """Unit tests and scripts call create_note with no event loop. That must be + an ordinary case: a write that succeeded cannot be failed by its index + refresh.""" + from unittest.mock import MagicMock, patch + + note = MagicMock(id=5, user_id=42, title="T", body="B") + with patch("asyncio.create_task", side_effect=RuntimeError("no running loop")): + notes_svc.embed_note(note) # must not raise + + +def test_embed_note_swallows_an_indexing_failure(): + """Same reason, wider net: the embedding model being unavailable must not + turn a successful save into a 500.""" + from unittest.mock import MagicMock, patch + + note = MagicMock(id=5, user_id=42, title="T", body="B") + with patch("asyncio.create_task", side_effect=ValueError("model gone")): + notes_svc.embed_note(note) # must not raise diff --git a/tests/test_shared_write_access.py b/tests/test_shared_write_access.py index 99e498a..50c90bb 100644 --- a/tests/test_shared_write_access.py +++ b/tests/test_shared_write_access.py @@ -40,8 +40,7 @@ async def test_editor_share_can_update_and_writes_as_the_owner(): with patch.object(svc, "get_snippet", AsyncMock(return_value=note)), \ patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \ patch.object(svc.notes_svc, "update_note", - AsyncMock(return_value=updated)) as mock_update, \ - patch.object(svc, "_embed_snippet", MagicMock()): + AsyncMock(return_value=updated)) as mock_update: got = await svc.update_snippet(7, 1, name="formatDuration") assert got is updated @@ -107,8 +106,7 @@ async def test_merge_skips_sources_owned_by_someone_else(): with patch.object(svc, "get_snippet", AsyncMock(side_effect=fake_get)), \ patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \ patch.object(svc.notes_svc, "update_note", AsyncMock(return_value=target)), \ - patch("scribe.services.trash.delete", AsyncMock(return_value=object())), \ - patch.object(svc, "_embed_snippet", MagicMock()): + patch("scribe.services.trash.delete", AsyncMock(return_value=object())): _note, merged_ids = await svc.merge_snippets(7, 1, [2, 3]) assert merged_ids == [2] diff --git a/tests/test_snippet_merge_provenance.py b/tests/test_snippet_merge_provenance.py index cda60ab..b465f1b 100644 --- a/tests/test_snippet_merge_provenance.py +++ b/tests/test_snippet_merge_provenance.py @@ -39,8 +39,7 @@ async def _run_merge(target, sources, source_ids): patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \ patch.object(s.notes_svc, "update_note", AsyncMock(return_value=target)) as mock_update, \ - patch("scribe.services.trash.delete", AsyncMock(return_value=object())), \ - patch.object(s, "_embed_snippet", MagicMock()): + patch("scribe.services.trash.delete", AsyncMock(return_value=object())): await s.merge_snippets(7, target.id, source_ids) return mock_update.await_args.kwargs @@ -88,8 +87,7 @@ async def test_an_ordinary_edit_carries_provenance_forward(): with patch.object(s, "get_snippet", AsyncMock(return_value=note)), \ patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \ patch.object(s.notes_svc, "update_note", - AsyncMock(return_value=note)) as mock_update, \ - patch.object(s, "_embed_snippet", MagicMock()): + AsyncMock(return_value=note)) as mock_update: await s.update_snippet(7, 1, signature="f(ms) -> string") kwargs = mock_update.await_args.kwargs @@ -106,8 +104,7 @@ async def test_a_pre_0070_row_keeps_provenance_through_the_body(): with patch.object(s, "get_snippet", AsyncMock(return_value=note)), \ patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \ patch.object(s.notes_svc, "update_note", - AsyncMock(return_value=note)) as mock_update, \ - patch.object(s, "_embed_snippet", MagicMock()): + AsyncMock(return_value=note)) as mock_update: await s.update_snippet(7, 1, when_to_use="humanize a ms count") assert "**Merged from:** #5" in mock_update.await_args.kwargs["body"] diff --git a/tests/test_snippet_unmerge.py b/tests/test_snippet_unmerge.py index 4470392..1009504 100644 --- a/tests/test_snippet_unmerge.py +++ b/tests/test_snippet_unmerge.py @@ -44,7 +44,6 @@ async def _run_unmerge(survivor, *, source_alive=None, restore=1): patch("scribe.services.trash.restore_entity", AsyncMock(return_value=restore)), patch.object(s.notes_svc, "update_note", AsyncMock(return_value=survivor)) as upd, - patch.object(s, "_embed_snippet", MagicMock()), ): await s.unmerge_snippet(7, 1, 2) return upd.await_args.kwargs @@ -127,7 +126,6 @@ async def test_a_purged_source_is_refused_and_the_survivor_is_untouched(): patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), patch("scribe.services.trash.restore_entity", AsyncMock(return_value=None)), patch.object(s.notes_svc, "update_note", AsyncMock()) as upd, - patch.object(s, "_embed_snippet", MagicMock()), ): with pytest.raises(s.UnmergeError, match="purged"): await s.unmerge_snippet(7, 1, 2) @@ -150,7 +148,6 @@ async def test_an_entry_without_attribution_is_refused_not_guessed(): patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)), patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), patch.object(s.notes_svc, "update_note", AsyncMock()) as upd, - patch.object(s, "_embed_snippet", MagicMock()), ): with pytest.raises(s.UnmergeError, match="provenance"): await s.unmerge_snippet(7, 1, 2) From f8522fb28f56f43d779ed98b7dab9d0b90f23236 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 22:56:31 -0400 Subject: [PATCH 07/10] fix(retrieval): give reuse a slot, and let experience reach the write path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two mirror-image scoping mistakes, neither deliberate (#2246). AUTO-INJECT let every kind compete on raw cosine. That is fatal rather than merely imperfect here, because Scribe's project records are ABOUT software work: a task titled "surface snippets before the agent writes code" is a near-perfect lexical match for "write a function…" while answering none of it. Measured live, a prompt asking for a helper returned three records about BUILDING the retrieval system and zero snippets. Snippets are ~0.5% of the corpus, and the ratio worsens as the project record grows — which is the direction Scribe is meant to grow, so no threshold tuning fixes it. Now the best snippet or process takes the LAST slot when none won on score. Deliberately NOT held to the margin band: that band measures distance from the top overall score, and the top score is the very thing snippets lose to. It still must clear the configured threshold, so a weak snippet cannot buy the slot — silence stays the default. Skipped entirely when reuse already won, so the fix is invisible in the case it isn't needed. WRITE-PATH was snippets-only — the same mistake inverted. An issue recording "we tried this and it deadlocked" could never reach the moment that code was about to be written, though it is arguably the better prior art: it says what NOT to do. Widened to snippets plus recorded experience. That needed a filter the search layer couldn't express. "Experience" is issues plus dev-logs, which differ on is_task, so neither note_type nor is_task alone covers it. semantic_search_notes gains task_kind, which restricts TASKS to the given kinds while leaving non-task notes untouched — so note_type=("snippet", "note") + task_kind="issue" yields snippets, fixed problems and durable notes, without the open to-do list. note_type now accepts a sequence too. Non-snippet hits are labelled with their kind, because an unlabelled issue on that menu reads as "here is code to reuse", the opposite of what it says. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- src/scribe/services/embeddings.py | 37 ++++-- src/scribe/services/plugin_context.py | 96 ++++++++++++++- tests/test_services_plugin_context.py | 171 ++++++++++++++++++++++++++ 3 files changed, 292 insertions(+), 12 deletions(-) diff --git a/src/scribe/services/embeddings.py b/src/scribe/services/embeddings.py index b870433..5df0494 100644 --- a/src/scribe/services/embeddings.py +++ b/src/scribe/services/embeddings.py @@ -14,7 +14,9 @@ import logging import math import os -from sqlalchemy import delete, select +from collections.abc import Sequence + +from sqlalchemy import delete, or_, select from scribe.models import async_session from scribe.models.embedding import NoteEmbedding @@ -114,7 +116,8 @@ async def semantic_search_notes( threshold: float = _SIMILARITY_THRESHOLD, project_id: int | None = None, is_task: bool | None = None, - note_type: str | None = None, + note_type: str | Sequence[str] | None = None, + task_kind: str | Sequence[str] | None = None, orphan_only: bool = False, scope: str = "own", ) -> list[tuple[float, Note]]: @@ -123,8 +126,15 @@ async def semantic_search_notes( Scores are cosine similarities in [-1, 1]; only notes at or above *threshold* are returned, sorted highest-first. - `note_type` narrows to a single record kind (e.g. "snippet"), for callers - that want prior art rather than everything embedded. + `note_type` narrows to a record kind, or several (e.g. "snippet", or + ("snippet", "note")), for callers that want prior art rather than everything + embedded. + + `task_kind` restricts TASKS to the given kinds while leaving non-task notes + untouched. That asymmetry is the point: "recorded experience" is issues plus + dev-logs, and those differ on `is_task`, so neither `note_type` nor `is_task` + alone can express it. With `note_type="note", task_kind="issue"` a caller + gets fixed problems and durable notes without the open to-do list. `scope` ("own" | "browse" | "read", see access.notes_visibility_clause) decides how far this may see. It exists because this one function serves @@ -179,11 +189,22 @@ async def semantic_search_notes( stmt = stmt.where(Note.status.isnot(None)) elif is_task is False: stmt = stmt.where(Note.status.is_(None)) - # Narrow to one kind of record. Composes with is_task rather than - # replacing it — 'snippet' is a non-task note_type, so a caller asking - # for prior art gets snippets and not the dev-log that mentions them. + # Narrow to one kind of record, or several. Composes with is_task + # rather than replacing it — 'snippet' is a non-task note_type, so a + # caller asking for prior art gets snippets and not the dev-log that + # mentions them. if note_type: - stmt = stmt.where(Note.note_type == note_type) + kinds = [note_type] if isinstance(note_type, str) else list(note_type) + stmt = stmt.where(Note.note_type.in_(kinds)) + # Restrict TASKS to certain kinds while leaving notes alone. A note + # has no task_kind that means anything, so a plain `.in_()` would + # drop every dev-log — which is exactly the record a caller asking + # for prior experience wants most. + if task_kind: + tkinds = [task_kind] if isinstance(task_kind, str) else list(task_kind) + stmt = stmt.where( + or_(Note.status.is_(None), Note.task_kind.in_(tkinds)) + ) if exclude_ids: stmt = stmt.where(NoteEmbedding.note_id.notin_(exclude_ids)) stmt = stmt.where(distance <= max_distance).order_by(distance.asc()).limit(limit) diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index f07d311..32ecbe7 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -290,6 +290,65 @@ def _record_kind(note) -> str: return note.note_type or "note" +_REUSE_KINDS = ("snippet", "process") + + +async def _reserve_slot_for_reuse( + user_id: int, + query: str, + kept: list, + cfg: dict, + *, + project_id: int | None, + exclude_ids: set[int], +) -> list: + """Guarantee the reuse-shaped kinds one slot, if one clears threshold (#2246). + + Ranking by raw cosine is blind to what KIND of record answers what kind of + ask, and the corpus makes that fatal rather than merely imperfect: Scribe's + project records are *about software work*, so a task titled "surface snippets + before the agent writes code" is a near-perfect lexical match for "write a + function…" while being useless as an answer to it. Measured live, a prompt + asking for a helper returned three records about BUILDING the retrieval + system and zero snippets. + + The bias is structural and gets WORSE as the project record grows — which is + the direction Scribe is supposed to grow. Snippets are ~0.5% of the corpus + here; no threshold tuning fixes a 200:1 ratio. + + So the reserved hit is deliberately NOT held to the margin band. The band + measures distance from the top overall score, and that top score is the very + thing snippets lose to. It still has to clear the configured threshold, so a + weak snippet cannot buy the slot — silence stays the default. + """ + if any(_record_kind(n) in _REUSE_KINDS for _s, n in kept): + return kept # reuse already represented; nothing to do + + top_k = cfg["top_k"] + reuse = await semantic_search_notes( + user_id, query, + limit=1, + threshold=cfg["threshold"], + project_id=project_id, + exclude_ids=exclude_ids | {int(n.id) for _s, n in kept}, + note_type=_REUSE_KINDS, + scope="browse", + ) + # Belt and braces on top of exclude_ids: a record already on the menu must + # never appear twice, and one slot is all this is entitled to. + kept_ids = {int(n.id) for _s, n in kept} + fresh = [(s, n) for s, n in reuse if int(n.id) not in kept_ids][:1] + if not fresh: + return kept + + # Take the LAST slot, never the first: the strongest overall hit is still the + # best answer to the prompt, and displacing it would trade one blindness for + # another. + if len(kept) >= top_k: + return kept[:top_k - 1] + fresh + return (kept + fresh)[:top_k] + + async def build_autoinject_hint( user_id: int, query: str, @@ -341,6 +400,10 @@ async def build_autoinject_hint( # Margin gate: keep only hits close to the strongest one. top_score = hits[0][0] kept = [(s, n) for s, n in hits if s >= top_score - _AUTOINJECT_BAND] + kept = await _reserve_slot_for_reuse( + user_id, q, kept, cfg, project_id=(project_id or None), + exclude_ids=set(exclude_ids or []), + ) # A collaborator's note can reach this menu via a shared project, and the # operator never asked for it — so say whose it is. Unattributed, it reads as @@ -685,7 +748,20 @@ async def build_write_path_hint( threshold=cfg["threshold"], project_id=scope_project, exclude_ids=seen, - note_type="snippet", + # Snippets AND recorded experience (#2246). This arm was + # snippets-only, which is auto-inject's mistake inverted: an issue + # saying "we tried this and it deadlocked", or a dev-log recording + # how a problem was solved, is prior art for the code about to be + # written — arguably better prior art than a resembling helper, + # because it says what NOT to do. + # + # `task_kind="issue"` keeps the open to-do list out. A task titled + # "add debouncing to the search box" resembles the code being + # written and answers nothing; an ISSUE is corrective work with a + # root cause in it, and a non-task note is durable knowledge. Both + # earned their place; a todo did not. + note_type=("snippet", "note"), + task_kind="issue", # Same reasoning as auto-inject: nobody asked for this, so it takes # the browse scope and never surfaces a one-to-one direct share. scope="browse", @@ -693,7 +769,10 @@ async def build_write_path_hint( record_retrieval( user_id=user_id, source="write_path", query=query, threshold=cfg["threshold"], limit=remaining, - project_id=scope_project, is_task=False, results=hits, + # is_task is None, not False: this arm now returns issues too, and + # recording it as a notes-only retrieval would misdescribe the + # candidate set the threshold is being tuned against. + project_id=scope_project, is_task=None, results=hits, duration_ms=(time.perf_counter() - t0) * 1000.0, ) if hits: @@ -701,8 +780,15 @@ async def build_write_path_hint( for score, note in hits: if score < top_score - _AUTOINJECT_BAND: continue + # Name the kind unless it's a snippet — the menu's default and + # the header's default reading. An issue or a dev-log offered + # here is a different KIND of claim ("this was already tried") + # and an unlabelled line would be read as "here is code to + # reuse", which is the opposite of what it says. + kind = _record_kind(note) scored.append(( - f"similar {score:.2f}", + f"similar {score:.2f}" if kind == "snippet" + else f"similar {score:.2f} · {kind}", { "id": int(note.id), "title": note.title, "user_id": note.user_id, # Carried so the line can disclose a cross-language hit @@ -733,7 +819,9 @@ async def build_write_path_hint( lines = [ f"> Prior art already recorded in Scribe for `{path}` — open one with " - "`get_snippet(id)` and reuse it rather than writing a fresh one-off " + "`get_snippet(id)` for a snippet, `get_task(id)` for an issue, " + "`get_note(id)` otherwise. Reuse a snippet rather than writing a fresh " + "one-off; read an issue before repeating what it records " "(titles only; shown once per session):", ] # Say what a language tag MEANS, and only when one is actually on the menu. diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index e73ca26..5754686 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -295,3 +295,174 @@ async def test_build_session_context_caps_length(): assert len(out["context"]) <= 9000 + 60 # cap + truncation note assert "truncated" in out["context"] + + +# --- the reuse slot (#2246) -------------------------------------------------- + +_CFG = {"enabled": True, "threshold": 0.55, "top_k": 3} + + +async def _autoinject(main_hits, reuse_hits, cfg=None): + """Run build_autoinject_hint with the two semantic queries stubbed in order: + the unscoped pool first, then the reserved reuse query.""" + calls: list[dict] = [] + + async def fake_search(*_a, **kw): + calls.append(kw) + return reuse_hits if kw.get("note_type") else main_hits + + with patch("scribe.services.plugin_context.get_autoinject_config", + AsyncMock(return_value=dict(cfg or _CFG))), \ + patch("scribe.services.plugin_context.semantic_search_notes", + AsyncMock(side_effect=fake_search)), \ + patch("scribe.services.plugin_context.record_retrieval", MagicMock()), \ + patch("scribe.services.plugin_context.record_surfaced", MagicMock()), \ + patch("scribe.services.plugin_context.owner_names_for", + AsyncMock(return_value={})): + from scribe.services.plugin_context import build_autoinject_hint + out = await build_autoinject_hint(1, "write a debounce helper") + return out, calls + + +@pytest.mark.asyncio +async def test_a_snippet_takes_the_last_slot_when_none_won_on_score(): + """The measured failure: a prompt asking for a helper returned three project + records ABOUT building the retrieval system, and zero snippets. Scribe's own + records are about software work, so they share vocabulary with any coding + prompt while answering none of them.""" + main = [(0.66, _note(1, "Step 3: title-first auto-inject")), + (0.65, _note(2, "Task-reminder dedup query crashes", is_task=True)), + (0.64, _note(3, "Drafter hardening · write-path trigger", is_task=True))] + reuse = [(0.58, _note(9, "debounce — collapse rapid calls", note_type="snippet"))] + + out, calls = await _autoinject(main, reuse) + + assert out["note_ids"] == [1, 2, 9] # last slot displaced, not the first + assert "#9" in out["context"] and "[snippet]" in out["context"] + # The reserved query is scoped to the reuse kinds and asks for exactly one. + assert calls[1]["note_type"] == ("snippet", "process") + assert calls[1]["limit"] == 1 + + +@pytest.mark.asyncio +async def test_the_reserved_query_is_skipped_when_a_snippet_already_won(): + """No second query, and no slot spent twice, when ranking already did the + right thing — the fix must be invisible in the case it isn't needed.""" + main = [(0.81, _note(9, "debounce helper", note_type="snippet")), + (0.80, _note(1, "some task", is_task=True))] + + out, calls = await _autoinject(main, []) + + assert len(calls) == 1 # reserved query never ran + assert out["note_ids"] == [9, 1] + + +@pytest.mark.asyncio +async def test_a_weak_snippet_does_not_buy_the_slot(): + """The reserved hit skips the MARGIN band — that band is what snippets lose + to — but never the threshold. Silence stays the default; a slot spent on an + irrelevant snippet is how a menu teaches people to ignore it.""" + main = [(0.66, _note(1, "a task", is_task=True))] + + out, calls = await _autoinject(main, []) # threshold returned nothing + + assert out["note_ids"] == [1] + assert len(calls) == 2 # it asked, and got nothing + + +@pytest.mark.asyncio +async def test_the_reserved_hit_is_not_held_to_the_margin_band(): + """0.58 is 0.08 below the top hit. Under the band it would survive; the point + is that it must survive even when it wouldn't — a snippet losing to a + same-vocabulary project record by a wide margin is the whole bug.""" + main = [(0.90, _note(1, "a task", is_task=True))] + reuse = [(0.58, _note(9, "debounce", note_type="snippet"))] + + out, _calls = await _autoinject(main, reuse) + + assert 9 in out["note_ids"] + + +@pytest.mark.asyncio +async def test_a_process_counts_as_reuse_too(): + """A stored process answers 'how do we do X here' the same way a snippet + answers 'what do we already have' — both lose to the same project records.""" + main = [(0.70, _note(1, "a task", is_task=True))] + reuse = [(0.60, _note(8, "DRY pass process", note_type="process"))] + + out, _ = await _autoinject(main, reuse) + assert 8 in out["note_ids"] + + # …and one already on the menu suppresses the reserved query. + out2, calls2 = await _autoinject( + [(0.70, _note(8, "DRY pass process", note_type="process"))], []) + assert len(calls2) == 1 + + +# --- write-path widened beyond snippets (#2246, the mirror half) ------------- + +@pytest.mark.asyncio +async def test_write_path_semantic_arm_asks_for_experience_not_just_snippets(): + """The inverse of auto-inject's mistake. This arm was snippets-only, so an + issue saying "we tried this and it deadlocked" could never reach the moment + that code was about to be written — arguably the better prior art, because + it says what NOT to do.""" + from scribe.services import plugin_context as pc + + hits = [(0.72, _note(9, "debounce helper", note_type="snippet")), + (0.70, _note(7, "Debounce dropped the trailing call", is_task=True, + task_kind="issue"))] + search = AsyncMock(return_value=hits) + rec = MagicMock() + with patch.object(pc, "get_writepath_config", + AsyncMock(return_value={"enabled": True, "threshold": 0.6, + "top_k": 3})), \ + patch.object(pc.snippets_svc, "list_snippets", + AsyncMock(return_value=([], 0))), \ + patch.object(pc, "semantic_search_notes", search), \ + patch.object(pc, "record_retrieval", rec), \ + patch.object(pc, "record_surfaced", MagicMock()), \ + patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \ + patch.object(pc, "concept_query", MagicMock(return_value="debounce a callback")): + out = await pc.build_write_path_hint( + 1, "src/utils/debounce.ts", code="x" * 400, + ) + + kw = search.await_args.kwargs + assert kw["note_type"] == ("snippet", "note") + # An open to-do resembling the code answers nothing; an ISSUE carries a root + # cause and a NOTE carries durable knowledge. Only the todo is excluded. + assert kw["task_kind"] == "issue" + # Telemetry must not claim this was a notes-only retrieval any more. + assert rec.call_args.kwargs["is_task"] is None + + +@pytest.mark.asyncio +async def test_write_path_labels_a_non_snippet_hit_with_its_kind(): + """An unlabelled issue on this menu reads as "here is code to reuse", which + is the opposite of what it says. A snippet stays unlabelled — it is the + menu's default and the header's default reading.""" + from scribe.services import plugin_context as pc + + hits = [(0.72, _note(9, "debounce helper", note_type="snippet")), + (0.71, _note(7, "Debounce dropped the trailing call", is_task=True, + task_kind="issue"))] + with patch.object(pc, "get_writepath_config", + AsyncMock(return_value={"enabled": True, "threshold": 0.6, + "top_k": 3})), \ + patch.object(pc.snippets_svc, "list_snippets", + AsyncMock(return_value=([], 0))), \ + patch.object(pc, "semantic_search_notes", AsyncMock(return_value=hits)), \ + patch.object(pc, "record_retrieval", MagicMock()), \ + patch.object(pc, "record_surfaced", MagicMock()), \ + patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \ + patch.object(pc, "concept_query", MagicMock(return_value="debounce a callback")): + out = await pc.build_write_path_hint( + 1, "src/utils/debounce.ts", code="x" * 400, + ) + + ctx = out["context"] + assert "· issue]" in ctx # the issue says what it is + assert '[similar 0.72] "debounce helper"' in ctx # the snippet does not + # The header now names the right opener for each kind. + assert "get_task(id)" in ctx and "get_snippet(id)" in ctx From da2383b0794d540307492fb517b02636cea8c949 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 23:01:31 -0400 Subject: [PATCH 08/10] fix(retrieval): verify the reserved slot's kind instead of trusting the query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught six failures on f8522fb. Five were fixtures; one was a real assumption. THE REAL ONE: _reserve_slot_for_reuse trusted that a query filtered by note_type could only return reuse kinds. It now checks _record_kind on the way in. That slot exists FOR snippets and processes — one silently spent on something else is worse than no slot at all, because the resulting line is indistinguishable from one that earned its place on score. THE FIXTURES, all the same shape: MagicMock notes with is_task left to auto-create. It is truthy, and _record_kind reads task-ness FIRST — so every mock snippet in three test modules was rendering as "task". Two of those fixtures already carried a comment explaining this exact hazard about `.data`; the same reasoning applies to `.is_task` and nobody had needed it until the menu started naming kinds. One assertion was genuinely stale rather than broken: test_write_path_trigger pinned note_type == "snippet", which was the behaviour the widening replaced. Updated to the new contract, including the task_kind="issue" filter that keeps the open to-do list out. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- src/scribe/services/plugin_context.py | 11 ++++++++--- tests/test_note_usage.py | 3 +++ tests/test_services_plugin_context.py | 3 +++ tests/test_write_path_trigger.py | 13 ++++++++++--- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 32ecbe7..192c3b8 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -334,10 +334,15 @@ async def _reserve_slot_for_reuse( note_type=_REUSE_KINDS, scope="browse", ) - # Belt and braces on top of exclude_ids: a record already on the menu must - # never appear twice, and one slot is all this is entitled to. + # Verify the kind rather than trusting the query that asked for it, and + # dedup on top of exclude_ids. This slot exists FOR reuse kinds — a slot + # silently spent on something else is worse than no slot, because the line + # is indistinguishable from one that earned its place on score. kept_ids = {int(n.id) for _s, n in kept} - fresh = [(s, n) for s, n in reuse if int(n.id) not in kept_ids][:1] + fresh = [ + (s, n) for s, n in reuse + if _record_kind(n) in _REUSE_KINDS and int(n.id) not in kept_ids + ][:1] if not fresh: return kept diff --git a/tests/test_note_usage.py b/tests/test_note_usage.py index 92dd7d9..b57f6b3 100644 --- a/tests/test_note_usage.py +++ b/tests/test_note_usage.py @@ -25,6 +25,9 @@ def _note(nid, title, user_id=1): # See the same note in test_write_path_trigger: an auto-created mock on # `.data` is truthy and would leak into a rendered menu line (#2244). n.data = None + # And on `.is_task`, which the kind marker reads FIRST — truthy there makes + # every one of these snippets read as a task (#2246). + n.is_task, n.task_kind = False, "work" return n diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index 5754686..82b96e1 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -19,6 +19,9 @@ def _note(nid, title, user_id=1, note_type="note", is_task=False, task_kind="wor # auto-MagicMock is truthy, so every line would read as another user's task. n.user_id = user_id n.note_type, n.is_task, n.task_kind = note_type, is_task, task_kind + # The write-path menu reads note.data for a language tag; an auto-mock there + # is truthy and renders its repr into the marker. + n.data = None return n diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 2426315..6f709b5 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -23,13 +23,17 @@ def _snippet_item(nid, title, user_id=1): return {"id": nid, "title": title, "user_id": user_id, "note_type": "snippet"} -def _note(nid, title, user_id=1): +def _note(nid, title, user_id=1, note_type="snippet", is_task=False, task_kind="work"): n = MagicMock() n.id, n.title, n.user_id = nid, title, user_id # Explicitly None, not left to MagicMock's auto-attribute: the semantic arm # reads `note.data` for the snippet's language (#2244), and an auto-created # mock there is truthy, so it would render its repr into the menu line. n.data = None + # Same reasoning, second instance: since #2246 this arm returns issues and + # dev-logs too, so the line names the kind. An auto-mock `is_task` is truthy, + # which would label every snippet here "task". + n.note_type, n.is_task, n.task_kind = note_type, is_task, task_kind return n @@ -183,8 +187,11 @@ async def test_semantic_arm_is_snippet_only_and_browse_scoped(): patch.object(pc, "record_retrieval", MagicMock()): await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE) kwargs = search.await_args.kwargs - # Prior art means snippets — not the dev-log that happens to mention one. - assert kwargs["note_type"] == "snippet" + # Prior art is snippets AND recorded experience (#2246) — an issue saying + # "we tried this and it broke" belongs here. What stays out is the open + # to-do list, which resembles the code and answers nothing. + assert kwargs["note_type"] == ("snippet", "note") + assert kwargs["task_kind"] == "issue" # Nobody asked for this, so it must not reach a one-to-one direct share. assert kwargs["scope"] == "browse" From 84541f392b6612b9c2110b641d67c2a478f199a5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 23:09:25 -0400 Subject: [PATCH 09/10] fix(backup): six tables were silently absent, and nothing would catch a seventh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit services/backup.py enumerated its tables as hand-maintained literals with nothing tying them to the schema. Tables added since that list was last extended were absent from every backup — no error, no warning, and a restore that reports success. Missing: systems, record_systems (0065), note_usage_events (0071), design_systems, design_tokens (0072), and repo_bindings — which the issue itself had not spotted, found only by diffing the model tablenames against the two lists instead of trusting either. _NOT_INCLUDED was worse than incomplete: it named "embeddings", "invitations" and "password_resets", none of which are tables. It read as coverage while naming nothing the schema could confirm. Now real names, plus retrieval_logs — observational telemetry that grows per query and that nothing reads for correctness. THE DELIVERABLE IS THE GUARD, not the six sections. Extending a list fixes today and changes nothing about the next table; a new one now fails a test until someone either backs it up or states that it shouldn't be. It checks both directions — an unaccounted table, and a listed name that no longer exists, which is what the three phantom entries above would have tripped. Design systems need ordering care: parent_id is a self-FK. The export orders parent-first (parent_id NULLS FIRST, then id — a parent always has the smaller id), so restore resolves each parent from the map as it goes, with no second pass. A child whose parent is missing lands as a root rather than failing the whole restore. Usage events are kept because pull-through is the evidence base for whether recall works, and it only ever accumulates — a restore that dropped it would reset that measurement to zero while everything still looked fine. BACKUP_VERSION 4 -> 5. Every new restore section is data.get()-guarded, so v2/v3/v4 payloads restore unchanged. Closes #2293. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- src/scribe/services/backup.py | 258 +++++++++++++++++++++++++++++++++- tests/test_services_backup.py | 44 +++++- 2 files changed, 292 insertions(+), 10 deletions(-) diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index 32d5cf6..1549d60 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -8,7 +8,10 @@ from scribe.models.milestone import Milestone from scribe.models.note import Note from scribe.models.note_draft import NoteDraft from scribe.models.note_version import NoteVersion +from scribe.models.design_system import DesignSystem, DesignToken +from scribe.models.note_usage import NoteUsageEvent from scribe.models.project import Project +from scribe.models.repo_binding import RepoBinding from scribe.models.rulebook import ( Rule, Rulebook, @@ -18,6 +21,7 @@ from scribe.models.rulebook import ( project_topic_suppressions, ) from scribe.models.setting import Setting +from scribe.models.system import RecordSystem, System from scribe.models.task_log import TaskLog from scribe.models.user import User @@ -26,17 +30,44 @@ logger = logging.getLogger(__name__) # Backup format version. v3 (2026-06) added rulebooks/topics/rules + their # project subscription/suppression join tables. v4 (2026-07) dropped events # when the calendar surface was retired — old v3 events are skipped on restore. +# v5 (2026-08) added the six tables that had accumulated outside the backup +# entirely (#2293), and the coverage guard that stops the seventh. # Bump when the serialized schema changes. -BACKUP_VERSION = 4 +BACKUP_VERSION = 5 + +# Every table this backup carries, by its REAL name. Paired with _NOT_INCLUDED +# below, these two lists must together account for the entire schema — which is +# what tests/test_services_backup.py asserts against Base.metadata. +# +# The point is the ABSENCE case. A new table gets a model and a migration, both +# of which fail loudly if wrong, and then silently never gets a backup section: +# no error, no warning, and a restore that reports success. Naming the coverage +# explicitly turns "someone forgot" into a failing test (#2293). +_BACKED_UP = [ + "users", "projects", "milestones", "notes", "task_logs", "note_drafts", + "note_versions", "settings", "rulebooks", "rulebook_topics", "rules", + "project_rulebook_subscriptions", "project_rule_suppressions", + "project_topic_suppressions", + # v5 (2026-08): the five-year gap this list was written to stop. + "systems", "record_systems", "design_systems", "design_tokens", + "note_usage_events", "repo_bindings", +] # Tables intentionally NOT in the backup, surfaced in the payload so the gap is # explicit rather than silent. ACL (groups/shares) is a coherent follow-up; -# embeddings are derived (regenerated from note bodies); api_keys are sensitive -# credentials; the rest are transient/operational. +# note_embeddings are derived (regenerated from note bodies); api_keys are +# sensitive credentials; retrieval_logs is observational telemetry that nothing +# reads for correctness and that grows per query; the rest are +# transient/operational. +# +# REAL table names, deliberately. This list used to read "embeddings", +# "invitations", "password_resets" — none of which are tables — so it looked +# like coverage while naming nothing the schema could confirm. _NOT_INCLUDED = [ "groups", "group_memberships", "project_shares", "note_shares", - "api_keys", "embeddings", "app_logs", "notifications", "invitations", - "password_resets", "user_profiles", + "api_keys", "note_embeddings", "app_logs", "notifications", + "invitation_tokens", "password_reset_tokens", "user_profiles", + "retrieval_logs", ] @@ -60,12 +91,73 @@ def _topic_suppression_rows(rows) -> list[dict]: return [{"project_id": r.project_id, "topic_id": r.topic_id} for r in rows] +# The v5 sections. Pure row-builders like the join-table helpers above, for the +# same reason: CI has no database, so a serialiser that is a plain function is +# one that can actually be tested. + +def _system_rows(rows) -> list[dict]: + return [ + { + "id": r.id, "user_id": r.user_id, "project_id": r.project_id, + "name": r.name, "description": r.description, "color": r.color, + "status": r.status, "order_index": r.order_index, + } + for r in rows + ] + + +def _record_system_rows(rows) -> list[dict]: + return [{"note_id": r.note_id, "system_id": r.system_id} for r in rows] + + +def _design_system_rows(rows) -> list[dict]: + return [ + { + "id": r.id, "owner_user_id": r.owner_user_id, "title": r.title, + "description": r.description, "guidance": r.guidance, + "parent_id": r.parent_id, + } + for r in rows + ] + + +def _design_token_rows(rows) -> list[dict]: + return [ + { + "id": r.id, "design_system_id": r.design_system_id, "name": r.name, + "value_by_mode": r.value_by_mode or {}, + "group_name": r.group_name, "purpose": r.purpose, + "rationale": r.rationale, "supersedes": r.supersedes or [], + "order_index": r.order_index, + } + for r in rows + ] + + +def _usage_event_rows(rows) -> list[dict]: + return [ + { + "user_id": r.user_id, "note_id": r.note_id, "event": r.event, + "source": r.source, + "created_at": r.created_at.isoformat() if r.created_at else None, + } + for r in rows + ] + + +def _repo_binding_rows(rows) -> list[dict]: + return [ + {"user_id": r.user_id, "project_id": r.project_id, "repo_key": r.repo_key} + for r in rows + ] + + # --------------------------------------------------------------------------- # Export # --------------------------------------------------------------------------- async def export_full_backup() -> dict: - """Export all data as a version-3 JSON backup.""" + """Export all data as a version-5 JSON backup.""" async with async_session() as session: users = (await session.execute(select(User))).scalars().all() projects = (await session.execute(select(Project))).scalars().all() @@ -77,6 +169,18 @@ async def export_full_backup() -> dict: select(NoteVersion).order_by(NoteVersion.note_id, NoteVersion.id) )).scalars().all() settings = (await session.execute(select(Setting))).scalars().all() + systems = (await session.execute(select(System))).scalars().all() + record_systems = (await session.execute(select(RecordSystem))).scalars().all() + # Parent-first, so a restore can resolve parent_id as it goes rather + # than needing a second pass — the self-FK is the only ordering + # constraint in this payload. + design_systems = (await session.execute( + select(DesignSystem).order_by(DesignSystem.parent_id.nullsfirst(), + DesignSystem.id) + )).scalars().all() + design_tokens = (await session.execute(select(DesignToken))).scalars().all() + usage_events = (await session.execute(select(NoteUsageEvent))).scalars().all() + repo_bindings = (await session.execute(select(RepoBinding))).scalars().all() rulebooks = (await session.execute(select(Rulebook))).scalars().all() topics = (await session.execute(select(RulebookTopic))).scalars().all() rules = (await session.execute(select(Rule))).scalars().all() @@ -244,11 +348,17 @@ async def export_full_backup() -> dict: "rulebook_subscriptions": _subscription_rows(subscriptions), "rule_suppressions": _rule_suppression_rows(rule_suppressions), "topic_suppressions": _topic_suppression_rows(topic_suppressions), + "systems": _system_rows(systems), + "record_systems": _record_system_rows(record_systems), + "design_systems": _design_system_rows(design_systems), + "design_tokens": _design_token_rows(design_tokens), + "note_usage_events": _usage_event_rows(usage_events), + "repo_bindings": _repo_binding_rows(repo_bindings), } async def export_user_backup(user_id: int) -> dict: - """Export a single user's data as a version-3 JSON backup.""" + """Export a single user's data as a version-5 JSON backup.""" async with async_session() as session: user = await session.get(User, user_id) projects = (await session.execute( @@ -274,6 +384,32 @@ async def export_user_backup(user_id: int) -> dict: settings = (await session.execute( select(Setting).where(Setting.user_id == user_id) )).scalars().all() + systems = (await session.execute( + select(System).where(System.user_id == user_id) + )).scalars().all() + system_ids = [sy.id for sy in systems] + note_ids = [n.id for n in notes] + # Scoped by the user's SYSTEMS, not their notes: a shared note carrying + # this user's system tag belongs in their backup, and a note of theirs + # tagged with someone else's system does not — that row is the other + # user's to keep. + record_systems = (await session.execute( + select(RecordSystem).where(RecordSystem.system_id.in_(system_ids)) + )).scalars().all() if system_ids else [] + design_systems = (await session.execute( + select(DesignSystem).where(DesignSystem.owner_user_id == user_id) + .order_by(DesignSystem.parent_id.nullsfirst(), DesignSystem.id) + )).scalars().all() + ds_ids = [d.id for d in design_systems] + design_tokens = (await session.execute( + select(DesignToken).where(DesignToken.design_system_id.in_(ds_ids)) + )).scalars().all() if ds_ids else [] + usage_events = (await session.execute( + select(NoteUsageEvent).where(NoteUsageEvent.note_id.in_(note_ids)) + )).scalars().all() if note_ids else [] + repo_bindings = (await session.execute( + select(RepoBinding).where(RepoBinding.user_id == user_id) + )).scalars().all() rulebooks = (await session.execute( select(Rulebook).where(Rulebook.owner_user_id == user_id) )).scalars().all() @@ -455,6 +591,12 @@ async def export_user_backup(user_id: int) -> dict: "rulebook_subscriptions": _subscription_rows(subscriptions), "rule_suppressions": _rule_suppression_rows(rule_suppressions), "topic_suppressions": _topic_suppression_rows(topic_suppressions), + "systems": _system_rows(systems), + "record_systems": _record_system_rows(record_systems), + "design_systems": _design_system_rows(design_systems), + "design_tokens": _design_token_rows(design_tokens), + "note_usage_events": _usage_event_rows(usage_events), + "repo_bindings": _repo_binding_rows(repo_bindings), } @@ -556,6 +698,8 @@ async def _restore_v2(data: dict) -> dict: "settings": 0, "rulebooks": 0, "rulebook_topics": 0, "rules": 0, "rulebook_subscriptions": 0, "rule_suppressions": 0, "topic_suppressions": 0, + "systems": 0, "record_systems": 0, "design_systems": 0, + "design_tokens": 0, "note_usage_events": 0, "repo_bindings": 0, } async with async_session() as session: @@ -814,6 +958,106 @@ async def _restore_v2(data: dict) -> dict: )) stats["topic_suppressions"] += 1 + # --- v5 sections. Every one is data.get()-guarded, so a v2/v3/v4 + # payload restores without them rather than failing on an absent key. + + # 15. Systems + system_id_map: dict[int, int] = {} + for sy_data in data.get("systems", []): + mapped_uid = user_id_map.get(sy_data.get("user_id", 0)) + mapped_pid = project_id_map.get(sy_data.get("project_id", 0)) + if mapped_uid is None or mapped_pid is None: + continue + system = System( + user_id=mapped_uid, project_id=mapped_pid, + name=sy_data.get("name", ""), + description=sy_data.get("description"), + color=sy_data.get("color"), + status=sy_data.get("status", "active"), + order_index=sy_data.get("order_index", 0), + ) + session.add(system) + await session.flush() + system_id_map[sy_data["id"]] = system.id + stats["systems"] += 1 + + # 16. Record↔system links + for rs in data.get("record_systems", []): + mapped_nid = note_id_map.get(rs.get("note_id", 0)) + mapped_sid = system_id_map.get(rs.get("system_id", 0)) + if mapped_nid is None or mapped_sid is None: + continue + session.add(RecordSystem(note_id=mapped_nid, system_id=mapped_sid)) + stats["record_systems"] += 1 + + # 17. Design systems. The export orders these parent-first, so a + # parent's new id is always in the map by the time a child needs it — + # no second pass, and a child whose parent is missing lands as a root + # rather than failing the whole restore. + design_system_id_map: dict[int, int] = {} + for ds_data in data.get("design_systems", []): + mapped_uid = user_id_map.get(ds_data.get("owner_user_id", 0)) + if mapped_uid is None: + continue + design = DesignSystem( + owner_user_id=mapped_uid, + title=ds_data.get("title", ""), + description=ds_data.get("description"), + guidance=ds_data.get("guidance"), + parent_id=design_system_id_map.get(ds_data.get("parent_id") or 0), + ) + session.add(design) + await session.flush() + design_system_id_map[ds_data["id"]] = design.id + stats["design_systems"] += 1 + + # 18. Design tokens + for t_data in data.get("design_tokens", []): + mapped_dsid = design_system_id_map.get(t_data.get("design_system_id", 0)) + if mapped_dsid is None: + continue + session.add(DesignToken( + design_system_id=mapped_dsid, + name=t_data.get("name", ""), + value_by_mode=t_data.get("value_by_mode") or {}, + group_name=t_data.get("group_name"), + purpose=t_data.get("purpose"), + rationale=t_data.get("rationale"), + supersedes=t_data.get("supersedes") or [], + order_index=t_data.get("order_index", 0), + )) + stats["design_tokens"] += 1 + + # 19. Usage events. Kept because pull-through is the evidence base for + # whether recall works at all, and it is only ever accumulated — a + # restore that dropped it would silently reset that measurement to zero + # while everything still looked fine. + for ev in data.get("note_usage_events", []): + mapped_nid = note_id_map.get(ev.get("note_id", 0)) + if mapped_nid is None: + continue + session.add(NoteUsageEvent( + user_id=user_id_map.get(ev.get("user_id") or 0), + note_id=mapped_nid, + event=ev.get("event", ""), + source=ev.get("source", ""), + created_at=_dt(ev.get("created_at")), + )) + stats["note_usage_events"] += 1 + + # 20. Repo bindings — small, but losing them means every bound repo + # quietly stops loading its project at session start. + for rb_data in data.get("repo_bindings", []): + mapped_uid = user_id_map.get(rb_data.get("user_id", 0)) + mapped_pid = project_id_map.get(rb_data.get("project_id", 0)) + if mapped_uid is None or mapped_pid is None: + continue + session.add(RepoBinding( + user_id=mapped_uid, project_id=mapped_pid, + repo_key=rb_data.get("repo_key", ""), + )) + stats["repo_bindings"] += 1 + await session.commit() logger.info("Restored v2/v3 backup: %s", stats) diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index 67486be..fb5c64d 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -13,16 +13,54 @@ import pytest from scribe.services import backup -def test_backup_version_is_v4(): - assert backup.BACKUP_VERSION == 4 +def test_backup_version_is_v5(): + assert backup.BACKUP_VERSION == 5 def test_not_included_lists_the_known_gaps(): # The deferred tables must be surfaced explicitly, not silently dropped. - for table in ("groups", "project_shares", "note_shares", "api_keys", "embeddings"): + for table in ("groups", "project_shares", "note_shares", "api_keys", + "note_embeddings", "retrieval_logs"): assert table in backup._NOT_INCLUDED +def test_every_table_is_either_backed_up_or_explicitly_excluded(): + """THE GUARD (#2293), and the only shape of test that catches an ABSENCE. + + A new table gets a model and a migration — both fail loudly if wrong — and + then silently never gets a backup section. No error, no warning, and a + restore that reports success. That is how `systems`, `record_systems`, + `note_usage_events`, `design_systems`, `design_tokens` and `repo_bindings` + all went missing, over five migrations, with nothing to notice. + + Extending the export fixes today. THIS fixes the next one: adding a table + now fails here until someone either backs it up or states in + `_NOT_INCLUDED` that it shouldn't be. Either is fine; silence is not. + """ + from scribe.models import Base + + schema = set(Base.metadata.tables) + accounted = set(backup._BACKED_UP) | set(backup._NOT_INCLUDED) + + unaccounted = schema - accounted + assert not unaccounted, ( + f"{len(unaccounted)} table(s) are neither backed up nor explicitly " + f"excluded: {sorted(unaccounted)}. Add each to backup._BACKED_UP (and " + f"give it an export + restore section) or to backup._NOT_INCLUDED with " + f"a reason in the comment above it." + ) + + # And the reverse: a name in either list that no longer exists is a lie the + # guard would otherwise keep telling. This half is what caught "embeddings", + # "invitations" and "password_resets" — three entries that named nothing. + phantom = accounted - schema + assert not phantom, ( + f"backup lists table(s) that are not in the schema: {sorted(phantom)}. " + f"Renamed or dropped — fix the list rather than leaving it to read as " + f"coverage." + ) + + def test_join_table_row_helpers_are_pure(): subs = [SimpleNamespace(project_id=1, rulebook_id=2)] rsup = [SimpleNamespace(project_id=1, rule_id=9)] From 2cc9e1380e5ec44b12e9fb58f328bc0fd762039d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 31 Jul 2026 23:13:16 -0400 Subject: [PATCH 10/10] test(backup): assert the version constant, not a copy of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The export test pinned `out["version"] == 4`, so bumping BACKUP_VERSION broke a test that was only ever checking the payload carries the version — which it still did. Asserts against backup.BACKUP_VERSION now, and covers the six v5 sections alongside the v3 ones. The guard itself passed on the first run: every table in Base.metadata was accounted for, in both directions. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs --- tests/test_services_backup.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index fb5c64d..6908c32 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -95,16 +95,18 @@ class _CM: @pytest.mark.asyncio -async def test_export_full_backup_contains_v3_sections(): +async def test_export_full_backup_contains_every_declared_section(): with patch("scribe.services.backup.async_session", lambda: _CM()): out = await backup.export_full_backup() - assert out["version"] == 4 + assert out["version"] == backup.BACKUP_VERSION assert out["scope"] == "full" assert "api_keys" in out["_not_included"] - # The sections v2 silently dropped must now be present (empty here). + # The sections v2 silently dropped, plus the six v5 added (empty here). for key in ("rulebooks", "rulebook_topics", "rules", "rulebook_subscriptions", "rule_suppressions", - "topic_suppressions"): - assert key in out, f"missing v3 section: {key}" + "topic_suppressions", + "systems", "record_systems", "design_systems", + "design_tokens", "note_usage_events", "repo_bindings"): + assert key in out, f"missing export section: {key}" assert out[key] == []