diff --git a/frontend/src/assets/components.css b/frontend/src/assets/components.css index 1b412ef..fa8b549 100644 --- a/frontend/src/assets/components.css +++ b/frontend/src/assets/components.css @@ -297,3 +297,36 @@ background: var(--fs-action-destructive-hover); border-color: var(--fs-action-destructive-hover); } + +/* ── Page container ───────────────────────────────────────────────────────── + The one wrapper a top-level view sits in: page width from the layout + tokens, centred, clipped horizontally so a wide child (a kanban, a table) + scrolls inside itself instead of the page. ProjectListView, ProjectView and + SnippetListView each carried this rule under their own name until #2903 + (milestone 299). */ +.page-container { + max-width: var(--fs-layout-page-max); + margin: 2rem auto; + padding: 0 var(--fs-layout-page-pad); + overflow-x: clip; +} + +/* ── Form input (fs-surfaces, snippet #2336) ──────────────────────────────── + Inputs sit DARKER than the page they're on — an inset well rather than a + raised panel; that inversion is what makes a field read as writable. The + design system's recipe, verbatim; width/box-sizing stay the caller's + (an inline select and a full-width textarea differ there). Three scoped + copies of an older input recipe were folded into this in #2903. */ +.fs-input { + background: var(--fs-surface-page); + border: var(--fs-border); + border-radius: var(--fs-radius-md); + padding: var(--fs-space-2) var(--fs-space-3); /* 8px 12px */ + color: var(--fs-text-primary); + font-family: var(--fs-font-body); + font-size: var(--fs-size-body); + transition: box-shadow var(--fs-dur-fast) var(--fs-ease); +} +.fs-input::placeholder { color: var(--fs-text-tertiary); } +.fs-input:focus { outline: none; box-shadow: var(--fs-focus-ring); } +.fs-input:disabled { opacity: var(--fs-disabled-opacity); cursor: not-allowed; } diff --git a/frontend/src/assets/dup-report.css b/frontend/src/assets/dup-report.css new file mode 100644 index 0000000..d7d1785 --- /dev/null +++ b/frontend/src/assets/dup-report.css @@ -0,0 +1,50 @@ +/* The near-duplicate report, shared by KnowledgeView (notes/tasks) and + SnippetListView (snippets) so the two reports read as one feature. Load + with diff --git a/frontend/src/components/SystemsSection.vue b/frontend/src/components/SystemsSection.vue index f1ef37b..afb4897 100644 --- a/frontend/src/components/SystemsSection.vue +++ b/frontend/src/components/SystemsSection.vue @@ -176,7 +176,7 @@ async function confirmDelete() {
+
- +
- @@ -891,7 +890,7 @@ async function confirmDelete() {
- @@ -1202,13 +1201,6 @@ async function confirmDelete() { diff --git a/frontend/src/views/UserManagementView.vue b/frontend/src/views/UserManagementView.vue deleted file mode 100644 index d4fce94..0000000 --- a/frontend/src/views/UserManagementView.vue +++ /dev/null @@ -1,441 +0,0 @@ - - - - - diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 3636f37..6799e70 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.39", + "version": "0.1.41", "author": { "name": "Bryan Van Deusen" }, "mcpServers": { "scribe": { diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index c9713f2..61991bc 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -58,6 +58,13 @@ scribe_defs() { if (match(line, /^(function|def|class|func|fun|fn|sub|struct|trait|interface|enum|object|protocol|type)[[:space:]]+[A-Za-z_$]/)) { t = line; sub(/^[a-z]+[[:space:]]+/, "", t) sub(/[^A-Za-z0-9_$].*$/, "", t) + # `type` defines only when something follows the name (= or {); an + # import specifier `type Foo,` is the same two words and defines + # nothing (mirror of coverage.py, #2904). + if (line ~ /^type[[:space:]]/) { + rest = line; sub(/^type[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*/, "", rest) + if (rest !~ /[={]/) next + } if (t != "" && t !~ /^__.*__$/) print "sym\t" t; next } # Arrow/expression assignment: const name = (…) / let name = async ( diff --git a/plugin/skills/reusing-code/SKILL.md b/plugin/skills/reusing-code/SKILL.md index 08db67d..9560c39 100644 --- a/plugin/skills/reusing-code/SKILL.md +++ b/plugin/skills/reusing-code/SKILL.md @@ -45,8 +45,9 @@ through recall/auto-inject; this skill is the active reflex around that. duplicate — reuse it and drop yours — or it isn't, and the record needs the new location adding. Both are cheaper now than after the duplicate settles in. - **A `Shape ledger at …` line is the ledger speaking, not the record.** It - names a duplicate family ("identical body in N other files, no canon") or a - canon elsewhere for a name you just wrote — for edits made through Bash + names a duplicate family ("identical body in N other files, no canon"), a + repeated name ("defined in N other files") or a canon elsewhere for a name + you just wrote — for edits made through Bash (sed, heredocs, scripts) as much as through Write/Edit. Derive the family or reuse the canon *now*; a family that is convention rather than copies is dismissed with `classify_shapes(..., status="exempt", diff --git a/plugin/skills/shape-accounting/SKILL.md b/plugin/skills/shape-accounting/SKILL.md index c62025b..8de6d67 100644 --- a/plugin/skills/shape-accounting/SKILL.md +++ b/plugin/skills/shape-accounting/SKILL.md @@ -94,7 +94,8 @@ the last sweep left it. Three surfaces say so without anyone running an audit - **At the write** — the prior-art hint (the Write/Edit hook, and since 0.1.39 the after-write hook on Bash, so sed/heredoc/script edits count too) carries a `Shape ledger at ` line when a name just written is a known - **duplicate family** ("identical body in N other files, no canon") or a + **duplicate family** ("identical body in N other files, no canon"), a + **repeated name** ("defined in N other files, no canon") or a **canon elsewhere** ("snippet #N at — reuse, don't redefine"). Act on it *then*: pull the canon and build from it, or derive the family now — `create_snippet` the dominant form, repoint the copies, `classify_shapes` @@ -109,6 +110,14 @@ the last sweep left it. Three surfaces say so without anyone running an audit reason_code="convention-plumbing", reason=…)` (or `classify_shapes_by_rule` for a whole family) removes it from the queue. Dismissal is a judgment and it is recorded; silence is not. +- **CSS is watched by name, never by body** (note 2917). Classes serving + different purposes share declarations because the style system makes them + alike — `.text-muted` and `.pin-badge-auto` carrying the same `color:` are + two meanings, not two copies — so a CSS family is the *same class defined + in ≥2 files* (a recipe living in several places), and identical bodies + under different names are never a family. Derive a CSS family by moving + the recipe to the shared sheet and recording it; a class name reused for + genuinely different things is dismissed with `reason_code="scoped-css"`. After the one-time pay-down the derive queue reads empty; anything in it afterwards is drift of the moment, and the hint already said so at the write. diff --git a/src/scribe/services/coverage.py b/src/scribe/services/coverage.py index 6dfd381..a1b8bc3 100644 --- a/src/scribe/services/coverage.py +++ b/src/scribe/services/coverage.py @@ -123,6 +123,12 @@ def _definition_on(raw: str) -> tuple[str, str] | None: name = m.group(1) if name.startswith("__") and name.endswith("__"): return None + # `type` announces a definition only when something is declared after + # the name (`type Foo = …`, `type Foo struct {`); an import specifier + # (`import { type Foo, bar }`) is the same two words and defines + # nothing — it showed up as a two-file "identical body" family (#2904). + if line.startswith("type") and not re.search(r"[={]", line[m.end():]): + return None return ("sym", name) if m := _ARROW_RE.match(line): return ("sym", m.group(1)) @@ -156,6 +162,12 @@ def _block_sha(lines: list[str]) -> str: return hashlib.sha1("\n".join(kept).encode("utf-8")).hexdigest()[:16] +def _declaration_count(lines: list[str]) -> int: + """How many `prop: value` declarations a CSS block body carries.""" + body = " ".join(lines) + return sum(1 for part in body.replace("}", "").split(";") if ":" in part) + + def extract_definitions(text: str) -> list[Definition]: """Every definition this text makes, with signature + fingerprint. @@ -186,11 +198,13 @@ def extract_definitions(text: str) -> list[Definition]: break block = lines[i:end] # A CSS rule's fingerprint is its DECLARATIONS, not its selector - # (#2872): the row's identity already carries the selector, and the - # question the fingerprint answers for derive grouping is "is this the - # same rule under another name?" — .closed-msg / .error-block / - # .success-msg with identical bodies are one dup group, not three - # lonely rows. Sym blocks keep their signature line in the hash. + # (#2872): the row's identity already carries the selector. Since + # note 2917 the derive grouping no longer reads CSS bodies at all (a + # class is grouped by name only), so for CSS the fingerprint is the + # recheck identity — "did this rule's body change since it was + # judged?" — and nothing more. The shape of the hash is kept as-is on + # purpose: changing it would flip every judged CSS row to recheck on + # the next sync. Sym blocks keep their signature line in the hash. if kind == "css": # One-line rules (`.x { color: red; }`) carry their declarations on # the selector line itself; a block that is only the selector plus @@ -202,6 +216,14 @@ def extract_definitions(text: str) -> list[Definition]: hashed = head + block[1:] if not any(x.strip() for x in hashed): hashed = block + # A SINGLE declaration is not a shape (#2903): `color: var(--fs- + # text-tertiary)` under .text-muted, .task-mark and .pin-badge-auto + # is three meanings sharing one line, not three copies of one + # rule. Keep the selector in the hash for one-liners; two + # declarations and up stay selector-agnostic. (Moot for grouping + # since note 2917, kept for fingerprint stability — see above.) + elif _declaration_count(hashed) < 2: + hashed = block else: hashed = block out.append(Definition( diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 5b7eefb..ee4c5c5 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -1061,17 +1061,27 @@ def _derive_line(path: str, derive: list[dict]) -> str: ) continue f = d["family"] - how = "identical body" if f.get("identical") else "same name defined" files = ", ".join(f"`{x}`" for x in f.get("files") or []) more = f.get("file_count", 0) - len(f.get("files") or []) if more > 0: files += f" +{more} more" + n = f.get("file_count", 0) + if f.get("identical"): + what = f"is a duplicate family with no canon — identical body in {n} other file(s)" + else: + # A name family: the same definition name living in several + # files. CSS is only ever grouped this way (note 2917) — a class + # is a recipe, and the recipe is what gets derived or dismissed. + what = f"is a repeated name with no canon — defined in {n} other file(s)" + # The dismissal reason the family most likely earns: a class name + # reused for different purposes is scoped styling; a code name reused + # across modules is convention plumbing. + dismiss = "scoped-css" if d.get("kind") == "css" else "convention-plumbing" parts.append( - f"`{f['label']}` is a duplicate family with no canon — {how} in " - f"{f.get('file_count', 0)} other file(s): {files}; derive it now: " + f"`{f['label']}` {what}: {files}; derive it now: " "record the canon (create_snippet) and make the copies instances " "(classify_shapes) — or, if these are convention not copies, " - "`classify_shapes(..., status=\"exempt\", reason_code=\"convention-plumbing\")` " + f"`classify_shapes(..., status=\"exempt\", reason_code=\"{dismiss}\")` " "dismisses the family — rather than adding another copy" ) return f"> Shape ledger at `{path}`: " + "; ".join(parts) + "." diff --git a/src/scribe/services/shape_ledger.py b/src/scribe/services/shape_ledger.py index e7520d1..b48d986 100644 --- a/src/scribe/services/shape_ledger.py +++ b/src/scribe/services/shape_ledger.py @@ -923,6 +923,13 @@ async def stamp_write_path_instances( # recur by convention, not by duplication). _DERIVE_MIN_DUP = 2 _DERIVE_MIN_NAME = 3 +# CSS is never grouped by body (note 2917): classes for different purposes +# share declarations because the style system makes them alike — `.text-muted` +# and `.pin-badge-auto` carrying the same `color: var(--fs-text-tertiary)` are +# two meanings, not two copies. A CSS family is a NAME defined in more than +# one file: that is a recipe living in several places, and two is already +# the signal (a class name is deliberate in a way `setup`/`load` are not). +_DERIVE_MIN_NAME_CSS = 2 # Semantic checks per repo per refresh — an embedding each (local fastembed), # bounded so a 4,000-row ledger is worked through over refreshes, not in one. _SEMANTIC_CAP = 150 @@ -1298,15 +1305,16 @@ def derive_groups( rows: Iterable[tuple[str, str, str, str]] ) -> dict[tuple[str, str, str], str]: """The derive-first grouping over (path, kind, symbol, body_sha) rows - that matched no canon: {(path, kind, symbol): group_key}. Identical - bodies in ≥2 places group as `dup:`; the same name defined in ≥3 - files groups as `name::`; a row joins at most one group, - the copy before the name.""" + that matched no canon: {(path, kind, symbol): group_key}. For code + (kind `sym`) identical bodies in ≥2 places group as `dup:` and the + same name defined in ≥3 files groups as `name:sym:`, the copy + before the name. CSS groups by name only — the same class defined in + ≥2 files is `name:css:`; its body never groups it (note 2917).""" by_sha: dict[str, list[tuple[str, str, str]]] = {} by_name: dict[tuple[str, str], list[tuple[str, str, str]]] = {} for path, kind, symbol, sha in rows: key = (path, kind, symbol) - if sha: + if sha and kind != "css": by_sha.setdefault(sha, []).append(key) by_name.setdefault((kind, _norm_symbol(symbol)), []).append(key) out: dict[tuple[str, str, str], str] = {} @@ -1315,7 +1323,8 @@ def derive_groups( for key in keys: out.setdefault(key, f"dup:{sha}") for (kind, symbol), keys in by_name.items(): - if len({k[0] for k in keys}) >= _DERIVE_MIN_NAME: + floor = _DERIVE_MIN_NAME_CSS if kind == "css" else _DERIVE_MIN_NAME + if len({k[0] for k in keys}) >= floor: for key in keys: out.setdefault(key, f"name:{kind}:{symbol}") return out @@ -1606,9 +1615,10 @@ async def write_time_derive( named at ``path``, what the ledger already knows about that name elsewhere in the project — - family the name sits in a derive-first group (identical body in N - files, or the same name in ≥3): "this is a known duplicate - family with no canon — derive it now, don't add a copy"; + family the name sits in a derive-first group (code: identical body + in N files or the same name in ≥3; CSS: the same class in + ≥2 files, note 2917): "this is a known family with no canon + — derive it now, don't add a copy"; canon a `canonical` row of that name at another path: "this is canon #N at — reuse, don't redefine". diff --git a/tests/helpers.py b/tests/helpers.py index 862c781..757e542 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -6,6 +6,7 @@ them; a module imports what it needs with ``from tests.helpers import ...``. """ from __future__ import annotations +from contextlib import contextmanager from datetime import datetime, timezone from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock @@ -182,3 +183,39 @@ def design_token_stub(name, value_by_mode, group_name=None, purpose=None, name=name, value_by_mode=value_by_mode, group_name=group_name, purpose=purpose, order_index=order_index, supersedes=supersedes or [], ) + + +@contextmanager +def http_sink(reply: bytes = b'{"context":"","note_ids":[]}'): + """A throwaway local HTTP listener for hook end-to-end tests: yields + ``(port, seen)`` where ``seen`` collects every GET's parsed query string + (one dict per request, in order). Lets the shell be tested end to end — + the extraction, the encoding, the URL — without a Scribe instance. + + Three test modules each carried their own ``_Sink`` handler before #2904 + consolidated them here; pass ``reply`` for the body the hook should see. + """ + import http.server + import threading + import urllib.parse + + seen: list[dict] = [] + + class _Sink(http.server.BaseHTTPRequestHandler): + def do_GET(self): + seen.append(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query)) + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(reply) + + def log_message(self, *a): + pass + + server = http.server.HTTPServer(("127.0.0.1", 0), _Sink) + threading.Thread(target=server.serve_forever, daemon=True).start() + try: + yield server.server_port, seen + finally: + server.shutdown() + server.server_close() diff --git a/tests/test_after_write_hook.py b/tests/test_after_write_hook.py index f536a42..5a8ec7e 100644 --- a/tests/test_after_write_hook.py +++ b/tests/test_after_write_hook.py @@ -6,17 +6,16 @@ the pre-write hook's end-to-end tests. Skips where the hook's tools are missing; asserts on content where they are present.""" from __future__ import annotations -import http.server import json import os import shutil import subprocess -import threading -import urllib.parse from pathlib import Path import pytest +from tests.helpers import http_sink + PLUGIN = Path(__file__).resolve().parents[1] / "plugin" HOOK = PLUGIN / "hooks" / "scribe_after_write.sh" @@ -53,66 +52,43 @@ def _run(repo, env, session="s-after-1", tool="Bash"): return out.stdout -class _Sink(http.server.BaseHTTPRequestHandler): - seen: list[dict] = [] - reply = b'{"context":"> family named","note_ids":[],"sync_note_ids":[],"derive_keys":["dup:483a"]}' - - def do_GET(self): - type(self).seen.append(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query)) - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.end_headers() - self.wfile.write(type(self).reply) - - def log_message(self, *a): - pass +SINK_REPLY = b'{"context":"> family named","note_ids":[],"sync_note_ids":[],"derive_keys":["dup:483a"]}' -@pytest.fixture -def sink(): - _Sink.seen = [] - server = http.server.HTTPServer(("127.0.0.1", 0), _Sink) - threading.Thread(target=server.serve_forever, daemon=True).start() - try: - yield server - finally: - server.shutdown() +def test_after_write_names_what_bash_just_wrote_then_stays_quiet_until_the_next_change(tmp_path): + with http_sink(SINK_REPLY) as (port, seen): + env = _env(tmp_path, url=f"http://127.0.0.1:{port}") + repo = _repo(tmp_path, env) + # "A Bash call" wrote an untracked stylesheet and appended to a tracked file. + (repo / "a.css").write_text(".log-empty {\n color: red;\n}\n") + (repo / "b.py").write_text("def one():\n return 1\n\ndef slug(t):\n return t\n") + out = _run(repo, env) + by_path = {q["path"][0]: q for q in seen} + assert set(by_path) == {"a.css", "b.py"} # repo-relative, like the pre hook + assert by_path["a.css"]["shapes"] == ["css:log-empty"] + assert by_path["b.py"]["shapes"] == ["sym:slug"] + # Added lines only for the tracked file — the existing def is not "just written". + assert "def slug" in by_path["b.py"]["code"][0] and "def one" not in by_path["b.py"]["code"][0] + ctx = json.loads(out)["hookSpecificOutput"] + assert ctx["hookEventName"] == "PostToolUse" + assert "> family named" in ctx["additionalContext"] + # The local by-name arm rides along: `slug` already lives in c.py. + assert "`slug` is already defined in 1 other file(s): c.py" in ctx["additionalContext"] + # Derive keys landed on the SHARED channel the pre-write hook reads. + state = tmp_path / "scribe-priorart" / "s-after-1.derive.ids" + assert "dup:483a" in state.read_text().split() + # Nothing changed → one git status, no request, no output. + seen.clear() + assert _run(repo, env) == "" + assert seen == [] -def test_after_write_names_what_bash_just_wrote_then_stays_quiet_until_the_next_change(tmp_path, sink): - env = _env(tmp_path, url=f"http://127.0.0.1:{sink.server_port}") - repo = _repo(tmp_path, env) - # "A Bash call" wrote an untracked stylesheet and appended to a tracked file. - (repo / "a.css").write_text(".log-empty {\n color: red;\n}\n") - (repo / "b.py").write_text("def one():\n return 1\n\ndef slug(t):\n return t\n") - out = _run(repo, env) - by_path = {q["path"][0]: q for q in _Sink.seen} - assert set(by_path) == {"a.css", "b.py"} # repo-relative, like the pre hook - assert by_path["a.css"]["shapes"] == ["css:log-empty"] - assert by_path["b.py"]["shapes"] == ["sym:slug"] - # Added lines only for the tracked file — the existing def is not "just written". - assert "def slug" in by_path["b.py"]["code"][0] and "def one" not in by_path["b.py"]["code"][0] - ctx = json.loads(out)["hookSpecificOutput"] - assert ctx["hookEventName"] == "PostToolUse" - assert "> family named" in ctx["additionalContext"] - # The local by-name arm rides along: `slug` already lives in c.py. - assert "`slug` is already defined in 1 other file(s): c.py" in ctx["additionalContext"] - # Derive keys landed on the SHARED channel the pre-write hook reads. - state = tmp_path / "scribe-priorart" / "s-after-1.derive.ids" - assert "dup:483a" in state.read_text().split() - - # Nothing changed → one git status, no request, no output. - _Sink.seen = [] - assert _run(repo, env) == "" - assert _Sink.seen == [] - - # Another change → only that file, and the dedup channel goes back up. - (repo / "a.css").write_text(".log-empty {\n color: red;\n}\n.other {\n margin: 0;\n}\n") - _run(repo, env) - assert [q["path"][0] for q in _Sink.seen] == ["a.css"] - assert _Sink.seen[0]["exclude_derive"] == ["dup:483a"] - assert set(_Sink.seen[0]["shapes"][0].split(",")) == {"css:log-empty", "css:other"} - + # Another change → only that file, and the dedup channel goes back up. + (repo / "a.css").write_text(".log-empty {\n color: red;\n}\n.other {\n margin: 0;\n}\n") + _run(repo, env) + assert [q["path"][0] for q in seen] == ["a.css"] + assert seen[0]["exclude_derive"] == ["dup:483a"] + assert set(seen[0]["shapes"][0].split(",")) == {"css:log-empty", "css:other"} def test_after_write_is_silent_where_it_has_nothing_to_say(tmp_path): env = _env(tmp_path) diff --git a/tests/test_integration_shape_classify.py b/tests/test_integration_shape_classify.py index ad9b231..4d24c43 100644 --- a/tests/test_integration_shape_classify.py +++ b/tests/test_integration_shape_classify.py @@ -588,21 +588,30 @@ async def test_derive_groups_land_on_rows_and_in_the_summary(seeded): assert summary["derive_groups"][1]["label"] == ".card" assert summary["derive_groups"][0]["size"] == 2 and summary["derive_groups"][1]["size"] == 3 - # One of the css copies gets judged → the group shrinks on the next pass. + # One of the css copies gets judged → the group shrinks on the next pass + # but stays a family: a class in two files is already a recipe living in + # two places (css name floor 2, note 2917). Judge the second and it's gone. await classify_shapes(owner, pid, [ {"path": "b/z.css", "symbol": "card", "status": "exempt", "reason": "print sheet"}, ]) await apply_derive_groups(pid) rows, _ = await list_project_shapes(owner, pid, proposal="derive") - assert {r.symbol for r in rows} == {"slug"} # 2 files < the name floor + assert {r.symbol for r in rows} == {"slug", "card"} + assert {r.path for r in rows if r.symbol == "card"} == {"b/x.css", "b/y.css"} + await classify_shapes(owner, pid, [ + {"path": "b/y.css", "symbol": "card", "status": "exempt", "reason": "print sheet"}, + ]) + await apply_derive_groups(pid) + rows, _ = await list_project_shapes(owner, pid, proposal="derive") + assert {r.symbol for r in rows} == {"slug"} # 1 file < the css name floor @pytest.mark.integration async def test_write_time_derive_names_the_family_or_the_canon_for_a_name(seeded): - """#2900: against real rows — a name in a dup family → the family (other - files, count); a name whose canonical row lives elsewhere → that canon; - a judged row at the path, the canon's own file, or an unknown name → - silence.""" + """#2900: against real rows — a name in a family → the family (other + files, count; for CSS a NAME family, never a body one — note 2917); a + name whose canonical row lives elsewhere → that canon; a judged row at + the path, the canon's own file, or an unknown name → silence.""" from scribe.services.shape_ledger import apply_derive_groups, write_time_derive owner, pid, sid = seeded["owner"], seeded["pid"], seeded["snippet"] @@ -622,9 +631,10 @@ async def test_write_time_derive_names_the_family_or_the_canon_for_a_name(seeded out = await write_time_derive(pid, "v/D.vue", [("css", "log-empty"), ("css", "unknown")]) assert len(out) == 1 and out[0]["symbol"] == "log-empty" and out[0]["kind"] == "css" fam = out[0]["family"] - assert fam["identical"] is True and fam["label"] == ".log-empty" + # Three identical bodies, and still a NAME family: CSS never groups by body. + assert fam["identical"] is False and fam["label"] == ".log-empty" assert fam["files"] == ["v/A.vue", "v/B.vue", "v/C.vue"] and fam["file_count"] == 3 - assert out[0]["key"] == fam["group"] and fam["group"].startswith("dup:") + assert out[0]["key"] == fam["group"] == "name:css:log-empty" # Editing one existing member still names the OTHER members. out = await write_time_derive(pid, "v/A.vue", [("css", "log-empty")]) assert out[0]["family"]["files"] == ["v/B.vue", "v/C.vue"] and out[0]["family"]["size"] == 3 diff --git a/tests/test_pattern_coverage.py b/tests/test_pattern_coverage.py index ab70205..54fa989 100644 --- a/tests/test_pattern_coverage.py +++ b/tests/test_pattern_coverage.py @@ -149,6 +149,21 @@ def test_largest_gaps_ranks_by_unclassified_and_drops_clean_dirs(): assert gaps == [{"dir": "src", "unclassified": 2, "total": 3}] +def test_type_import_specifiers_are_not_definitions(): + """#2904: `import { type Foo, bar }` is the same two words as `type Foo =` + and defines nothing; only a `type` line with a declaration after the + name counts (TS alias, Go/Rust type).""" + from scribe.services.coverage import extract_shapes + src = ( + 'import { type DesignSystem, fetchDesignSystems } from "@/api/designSystems";\n' + 'import { type Project } from "./x";\n' + "type Baz = { a: number };\n" + "type Wide = T | null;\n" + "type Point struct {\n\tX int\n}\n" + ) + assert extract_shapes(src) == [("sym", "Baz"), ("sym", "Wide"), ("sym", "Point")] + + def test_coverage_line_is_evidence_carrying_and_labeled_estimate(): line = coverage_line({ "total": 4573, "accounted": 3100, "unclassified": 1473, @@ -484,12 +499,18 @@ def test_extract_definitions_fingerprints_each_block(): d = {x.name: x for x in extract_definitions(css)} assert d["closed-msg"].body_sha == d["error-block"].body_sha != d["other"].body_sha # One-line rules hash their own declarations — never the empty string - # (first deploy grouped 68 unrelated one-liners as one copy). - one = ".a { color: red; }\n\n.b { color: red; }\n\n.c { color: blue; }\n\n.d {\n color: red;\n}\n" + # (first deploy grouped 68 unrelated one-liners as one copy) — and a + # SINGLE declaration is not a shape (#2903): it keeps its selector in the + # hash, so `.a { color: red }` groups only with another `.a`, never with + # `.b { color: red }`. Two declarations and up stay selector-agnostic. + one = ".a { color: red; }\n\n.b { color: red; }\n\n.c { color: blue; }\n\n.a {\n color: red;\n}\n" e = {x.name: x for x in extract_definitions(one)} import hashlib - assert e["a"].body_sha == e["b"].body_sha != e["c"].body_sha + assert e["a"].body_sha != e["b"].body_sha != e["c"].body_sha assert e["a"].body_sha != hashlib.sha1(b"").hexdigest()[:16] + two = ".a {\n color: red;\n margin: 0;\n}\n.b {\n color: red;\n margin: 0;\n}\n" + f = {x.name: x for x in extract_definitions(two)} + assert f["a"].body_sha == f["b"].body_sha def test_coverage_line_names_the_proposers_standing(): diff --git a/tests/test_shape_ledger.py b/tests/test_shape_ledger.py index faffe11..c56ac33 100644 --- a/tests/test_shape_ledger.py +++ b/tests/test_shape_ledger.py @@ -308,6 +308,12 @@ def test_derive_groups_copy_before_name_with_floors(): ("d.css", "css", "btn", "s1"), ("e.css", "css", "btn", "s2"), ("f.css", "css", "btn", "s3"), ("g.py", "sym", "main", "s4"), ("h.py", "sym", "main", "s5"), # only 2 files → no name group ("i.py", "sym", "one", "s6"), + # CSS (note 2917): identical bodies under different names are NOT a + # copy — two meanings sharing the style system's look; the same + # class in two files IS a family (the name floor is 2 for css). + ("j.css", "css", "muted", "same"), ("k.css", "css", "pin-auto", "same"), + ("l.css", "css", "card", "c1"), ("m.css", "css", "card", "c2"), + ("n.css", "css", "alone", "c3"), ] g = derive_groups(rows) assert g[("a.py", "sym", "helper")] == "dup:sha1" == g[("b.py", "sym", "helper")] @@ -315,6 +321,10 @@ def test_derive_groups_copy_before_name_with_floors(): assert g[("d.css", "css", "btn")] == "name:css:btn" assert ("g.py", "sym", "main") not in g assert ("i.py", "sym", "one") not in g + assert ("j.css", "css", "muted") not in g and ("k.css", "css", "pin-auto") not in g + assert g[("l.css", "css", "card")] == "name:css:card" == g[("m.css", "css", "card")] + assert ("n.css", "css", "alone") not in g + assert not any(v.startswith("dup:") for k, v in g.items() if k[1] == "css") def test_derive_new_summary_counts_copies_first_seen_since_the_previous_refresh(): diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index c71fb15..626663f 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -13,7 +13,7 @@ from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest -from tests.helpers import fake_note +from tests.helpers import fake_note, http_sink PLUGIN = Path(__file__).resolve().parents[1] / "plugin" HOOK = PLUGIN / "hooks" / "scribe_prior_art.sh" @@ -1149,6 +1149,23 @@ def test_route_stamps_only_for_a_caller_allowed_to_write(): assert "&shapes=" in hook +def test_hook_does_not_name_a_type_import_specifier_as_a_shape(tmp_path): + """#2904, the awk mirror of the server rule: `type Foo,` inside an import + list is not a definition; `type Baz = …` on its own line is.""" + env = _hook_runtime_env() + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init", "-q"], cwd=repo, check=True, env=env) + seen = _run_hook_against_sink(tmp_path, { + "session_id": "s-type", "cwd": str(repo), "tool_name": "Write", + "tool_input": {"file_path": str(repo / "x.ts"), + "content": 'import { type Foo, bar } from "./y";\n' + "type Baz = { a: number };\n" + "export function use(): Baz {\n return { a: 1 };\n}\n"}, + }) + assert seen["shapes"] == ["sym:Baz,sym:use"] + + def test_hook_names_the_shapes_being_written(): """The feed's two inputs: every definition in the payload, or — for an Edit that changes a body, not a signature — the definition enclosing the edit, @@ -1165,39 +1182,15 @@ def test_hook_names_the_shapes_being_written(): def _run_hook_against_sink(tmp_path, payload): """Run the hook with SCRIBE_URL pointed at a throwaway local listener and - return the query the hook sent. Lets the shell be tested end to end — - the extraction, the encoding, the URL — without a Scribe instance.""" - import http.server - import threading - import urllib.parse - - seen: dict = {} - - class _Sink(http.server.BaseHTTPRequestHandler): - def do_GET(self): - seen.update(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query)) - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.end_headers() - self.wfile.write(b'{"context":"","note_ids":[]}') - - def log_message(self, *a): - pass - - server = http.server.HTTPServer(("127.0.0.1", 0), _Sink) - thread = threading.Thread(target=server.serve_forever, daemon=True) - thread.start() - try: - env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{server.server_port}") + return the query the hook sent (tests.helpers.http_sink).""" + with http_sink() as (port, seen): + env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{port}") out = subprocess.run( ["bash", str(HOOK)], input=json.dumps(payload), capture_output=True, text=True, env=env, ) assert out.returncode == 0, out.stderr - finally: - server.shutdown() - server.server_close() - return seen + return seen[0] if seen else {} def test_hook_sends_every_definition_in_a_write(tmp_path): @@ -1257,13 +1250,16 @@ async def test_the_write_time_derive_check_names_a_family_or_a_canon_in_band(): channel silences a family already named.""" from scribe.services import plugin_context as pc found = [ - {"symbol": "log-empty", "kind": "css", "key": "dup:483a", - "family": {"group": "dup:483a", "label": ".log-empty", "identical": True, + {"symbol": "log-empty", "kind": "css", "key": "name:css:log-empty", + "family": {"group": "name:css:log-empty", "label": ".log-empty", "identical": False, "files": ["a/TaskLogSection.vue", "a/WorkspaceTaskPanel.vue"], "file_count": 5, "size": 6}}, {"symbol": "btn-primary", "kind": "css", "key": "canon:2855", "canon": {"snippet_id": 2855, "path": "frontend/src/assets/components.css", "label": ".btn-primary"}}, + {"symbol": "slugify", "kind": "sym", "key": "dup:483a", + "family": {"group": "dup:483a", "label": "slugify", "identical": True, + "files": ["a/x.py", "a/y.py"], "file_count": 2, "size": 3}}, {"symbol": "load", "kind": "sym", "key": "name:sym:load", "family": {"group": "name:sym:load", "label": "load", "identical": False, "files": ["a/X.vue", "a/Y.vue", "a/Z.vue"], "file_count": 3, "size": 4}}, @@ -1281,20 +1277,27 @@ async def test_the_write_time_derive_check_names_a_family_or_a_canon_in_band(): patch.object(pc.shape_ledger_svc, "write_time_derive", check): out = await pc.build_write_path_hint( 1, "frontend/src/components/New.vue", code=REAL_CODE, project_id=24, - stamp_shapes=[("css", "log-empty"), ("css", "btn-primary"), ("sym", "load")], + stamp_shapes=[("css", "log-empty"), ("css", "btn-primary"), ("sym", "slugify"), + ("sym", "load")], exclude_derive=["name:sym:load"], ) check.assert_awaited_once_with(24, "frontend/src/components/New.vue", - [("css", "log-empty"), ("css", "btn-primary"), ("sym", "load")]) - # The excluded family is gone; the other two render and are keyed. - assert [d["key"] for d in out["derive"]] == ["dup:483a", "canon:2855"] - assert out["derive_keys"] == ["dup:483a", "canon:2855"] + [("css", "log-empty"), ("css", "btn-primary"), + ("sym", "slugify"), ("sym", "load")]) + # The excluded family is gone; the other three render and are keyed. + assert [d["key"] for d in out["derive"]] == ["name:css:log-empty", "canon:2855", "dup:483a"] + assert out["derive_keys"] == ["name:css:log-empty", "canon:2855", "dup:483a"] ctx = out["context"] assert "Shape ledger at `frontend/src/components/New.vue`" in ctx - assert "`.log-empty` is a duplicate family with no canon — identical body in 5 other file(s): " \ + # A CSS family is a repeated NAME (note 2917) and its dismissal is scoped-css; + # a code dup family is an identical body and dismisses as convention-plumbing. + assert "`.log-empty` is a repeated name with no canon — defined in 5 other file(s): " \ "`a/TaskLogSection.vue`, `a/WorkspaceTaskPanel.vue` +3 more; derive it now" in ctx + assert "`slugify` is a duplicate family with no canon — identical body in 2 other file(s): " \ + "`a/x.py`, `a/y.py`; derive it now" in ctx assert "`.btn-primary` is canon — snippet #2855 at `frontend/src/assets/components.css`" in ctx - assert "convention-plumbing" in ctx + assert 'reason_code=\"scoped-css\")` dismisses' in ctx + assert 'reason_code=\"convention-plumbing\")` dismisses' in ctx assert "`load`" not in ctx # No project → no check; a failing check never sinks the hint. check.reset_mock() @@ -1321,28 +1324,10 @@ def test_the_hook_keeps_a_derive_channel_and_sends_it_back(tmp_path): """#2900: derive keys the server returns land in the session's own `.derive.ids` file and go back as `exclude_derive` on the next write — a family is named once per session, not at every edit.""" - import http.server - import threading - import urllib.parse - - seen: list[dict] = [] - - class _Sink(http.server.BaseHTTPRequestHandler): - def do_GET(self): - seen.append(urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query)) - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.end_headers() - self.wfile.write(b'{"context":"> family","note_ids":[],"sync_note_ids":[],' - b'"derive_keys":["dup:483a","canon:2855"]}') - - def log_message(self, *a): - pass - - server = http.server.HTTPServer(("127.0.0.1", 0), _Sink) - threading.Thread(target=server.serve_forever, daemon=True).start() - try: - env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{server.server_port}", + reply = (b'{"context":"> family","note_ids":[],"sync_note_ids":[],' + b'"derive_keys":["dup:483a","canon:2855"]}') + with http_sink(reply) as (port, seen): + env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{port}", TMPDIR=str(tmp_path)) payload = {"session_id": "s-derive-1", "cwd": str(tmp_path), "tool_name": "Write", "tool_input": {"file_path": str(tmp_path / "a.css"), @@ -1351,8 +1336,6 @@ def test_the_hook_keeps_a_derive_channel_and_sends_it_back(tmp_path): out = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), capture_output=True, text=True, env=env) assert out.returncode == 0, out.stderr - finally: - server.shutdown() assert "exclude_derive" not in seen[0] assert seen[1]["exclude_derive"] == ["dup:483a,canon:2855"] state = tmp_path / "scribe-priorart" / "s-derive-1.derive.ids"