diff --git a/tools/agents/preferred-tooling.md b/tools/agents/preferred-tooling.md index 4c9249f2..a5422afb 100644 --- a/tools/agents/preferred-tooling.md +++ b/tools/agents/preferred-tooling.md @@ -41,12 +41,13 @@ entity-level tools (tree-sitter entities instead of lines) over raw git: | Question | Prefer | Fall back to | |---|---|---| | What changed, structurally? | `sem diff` | `git diff` | -| What breaks if I change X? | `sem impact X` / `sem callers X` | `rg` callers + reading | -| Who last touched this function? | `sem blame ` | `git blame` | +| What breaks if I change X? | `sem impact X` / `sem find X --callers` | `rg` callers + reading | +| Who last touched this function? | `sem history --blame ` | `git blame` | | Merge conflict (esp. parallel agents) | `weave` / `jj resolve --tool weave` | manual resolution | -| Where's the risk in this big diff/PR? | `inspect diff` / `inspect pr` | reading the full diff | +| Where's the risk in this big diff/PR? | `sem certify ` / `inspect diff` | reading the full diff | +| Show me what X reaches (diagram) | skill's `scripts/sem-mermaid.py` | don't draw edges by hand | -Full usage, guardrails (`sem setup`/`weave setup` mutate config — propose, +Full usage, guardrails (`sem config setup`/`weave setup` mutate config — propose, don't run), and jj integration live in the **entity-level-git** skill. The nix-summon trick below works for weave only (`nix run nixpkgs#weave`). **`nixpkgs#sem` is an unrelated program** (the Semaphore CI CLI) — never diff --git a/tools/agents/skills/entity-level-git/SKILL.md b/tools/agents/skills/entity-level-git/SKILL.md index 8bb0638f..846c299d 100644 --- a/tools/agents/skills/entity-level-git/SKILL.md +++ b/tools/agents/skills/entity-level-git/SKILL.md @@ -11,18 +11,25 @@ description: > formatting commit wrecked line-based blame); merge conflicts in code files, above all false conflicts where branches or parallel agents edited different functions in the same file. Also use whenever sem, weave, or inspect is - named, and to interpret the sem entity-diff comment CI posts on PRs. The - entity-level answer is cheaper and more precise than reading whole files. + named, to interpret the sem entity-diff comment CI posts on PRs, and to + draw a change's dependents as a Mermaid diagram. The entity-level answer is + cheaper and more precise than reading whole files. # Read-only subcommands only. Deliberately NOT `Bash(sem *)` / `Bash(weave *)`: -# a wildcard would pre-approve sem setup/login/cloud/update and weave setup, -# which the body says never to run unprompted - the permission prompt is the -# backstop for those rules, so the allowlist must not remove it. Also excluded: +# a wildcard would pre-approve `sem config setup|update`, `sem cloud *` and +# weave setup, which the body says never to run unprompted - the permission +# prompt is the backstop for those rules, so the allowlist must not remove it. +# `sem check` is left out too: it runs the project's test suite and checkers, +# which is executing the repo's code, not reading it. Also excluded: # `inspect review` (sends code to an LLM API) and `inspect comment` (posts to # GitHub). `weave apply` and `weave patch apply` write working files, so they # are excluded too; `weave explain`/`check` and `sem graph` are read-only and # are listed because the body tells the reader to run them - explain and check # exist only from 0.5.x, so on an older weave they simply do not resolve. -allowed-tools: Bash(sem diff *), Bash(sem impact *), Bash(sem callers *), Bash(sem refs *), Bash(sem find *), Bash(sem blame *), Bash(sem log *), Bash(sem entities *), Bash(sem graph *), Bash(sem context *), Bash(weave preview *), Bash(weave summary *), Bash(weave explain *), Bash(weave check *), Bash(inspect diff *), Bash(inspect predict *), Bash(inspect pr *), Bash(inspect file *), Bash(git status), Bash(git log *), Bash(git diff *) +# sem 0.27 renamed callers/refs/context/entities to `find` flags and log/blame +# to `history`; both spellings are listed because the old ones still work. +# `sem certify` only reads the repo and writes to stdout (its temp trees are +# its own), checked in the v0.27.0 source. +allowed-tools: Bash(sem diff *), Bash(sem impact *), Bash(sem callers *), Bash(sem refs *), Bash(sem find *), Bash(sem blame *), Bash(sem log *), Bash(sem history *), Bash(sem entities *), Bash(sem graph *), Bash(sem context *), Bash(sem certify *), Bash(weave preview *), Bash(weave summary *), Bash(weave explain *), Bash(weave check *), Bash(inspect diff *), Bash(inspect predict *), Bash(inspect pr *), Bash(inspect file *), Bash(git status), Bash(git log *), Bash(git diff *) --- # Entity-Level Git (sem, weave, inspect) @@ -69,37 +76,41 @@ yourself. rule: `nix run` weave if nix is present; for sem and inspect don't hand-install in a throwaway environment — fall back to `git diff` / reading the diff, and say which path you took. -- sem has a `sem update` self-updater. Don't run it: the binary is - brew-managed, and a self-update fights the package manager. +- sem has a self-updater (`sem config update`; `sem update` before 0.27). + Don't run it: the binary is brew-managed, and a self-update fights the + package manager. - Check with `command -v sem weave inspect` before building a plan around them. - **Check versions against upstream before trusting a capability verdict** - (`gh release list -R Ataraxy-Labs/`). As of 2026-09-22 sem 0.25.0 is - current and inspect 0.1.1 is the latest upstream has. weave is the awkward + (`gh release list -R Ataraxy-Labs/`). As of 2026-10-14 sem 0.27.0 is + both upstream's latest and what Homebrew core's `sem-cli` ships, and + inspect 0.1.1 is the latest upstream has. weave is the awkward one: nixpkgs tracks 0.3.6 (2026-06-05) against upstream's 0.5.4, so `lib/weave.nix` pins past it — onto `alycda/weave`'s ARB branch rather than the upstream tag. A missing weave feature is therefore about *which build you have*, not about weave. Run `weave --version` first. -## Language coverage: Dart/Flutter (verified 2026-09-21) +## Language coverage: Dart/Flutter (verified 2026-09-21; sem re-verified 2026-10-14) Measured on `alycda/DittoXCactus`, a Flutter app (~80 Dart entities). Dart is the worked example of the uneven-coverage caveat above, and it fails quietly in all three tools — one of them dangerously: -- **sem extracts Dart entities but builds no edges between them.** `diff`, - `blame`, `log`, `entities` and `context` are all correct and worth using. - The dependency graph is empty: in a two-file Dart repo where `app.dart` - constructs a class from `greeter.dart`, `sem graph` reports - `3 entities, 0 edges`, `sem impact` on the class answers - `✓ No other entities are affected`, and `callers`/`refs` both say "none". - The identical shapes in TypeScript produce the edge and the correct - `← depended on by: function run (ts/app.ts)`. So this is Dart reference - extraction, not a sem-wide limit. **In a Dart repo that checkmark is a - false negative, not permission to refactor — `rg` for the callers.** - Worse, in a mixed-language repo the name matching leaks: `sem callers - hello --file dart/greeter.dart` returned `function run ts/app.ts:3`, a - caller in a different file *and a different language*. Treat any - graph-derived Dart answer as unusable in both directions. +- **sem: fixed in 0.26.0, now at parity with TypeScript.** On 0.25.0 sem + extracted Dart entities but built no edges: a two-file repo where + `app.dart` constructs a class from `greeter.dart` gave `0 edges` and a + green `✓ No other entities are affected`, and `callers` leaked a TS caller + into a Dart answer. 0.26.0 fixed Dart call resolution (upstream #491). + Re-run on 0.27.0 against the same shape plus a typed parameter, in a repo + holding identical TS twins: `impact` on the Dart class returns + `← depended on by: function run (dart/app.dart)`, and + `String typed(Greeter g) => g.hello()` resolves as a caller of `hello` + exactly as the TS twin does. What is still unresolved — + `Greeter('x').hello()`, an untyped receiver — is unresolved in TS too. + sem now says so: an incomplete answer prints `INCOMPLETE` with the reason + and a "possible callers" list instead of a green check, and that list is + where a cross-language name match still shows up, labelled as unresolved. + Re-verified synthetically, not yet on DittoXCactus itself — run `sem + graph` there and confirm a non-zero edge count before relying on it. - **inspect does not parse Dart at all.** `inspect diff` on a 7-file branch returned 61 `chunk` entities (`lines 161-180`) plus 1 `section` — zero functions, zero classes — under a headline of "0 critical, 0 high, 48 @@ -125,12 +136,14 @@ in all three tools — one of them dangerously: 0.5.4 both report `weave 0.5.4`. Use the store-path or behaviour probe above, and assume stock unless one of them says otherwise. -Net: in a Dart repo, use `sem diff` / `blame` / `log` / `entities` / -`context` and `weave` freely, never trust `sem impact` / `callers` / `refs`, -and skip inspect. Dart may not be the only language in this state — before -trusting a graph-dependent answer in an untested language, check that the -tool returned real entity kinds (`method`, `class`) rather than `chunk`s, -and that `sem graph` reports a non-zero edge count. +Net: in a Dart repo on sem ≥ 0.26, use sem and weave as in any other +language. Read `INCOMPLETE` and the possible-caller list before acting on +an `impact` answer, as you would in TS, and skip inspect. On an older sem the +0.25.0 behaviour above applies: never trust `impact` / `callers` / `refs`. +Another language may be in the state Dart was — before trusting a +graph-dependent answer in an untested language, check that the tool +returned real entity kinds (`method`, `class`) rather than `chunk`s, and +that `sem graph` reports a non-zero edge count. **Don't repeat my measurement mistake.** The first version of this section claimed weave couldn't parse Dart, on the evidence of a `weave preview` @@ -141,10 +154,11 @@ nothing to three-way merge. **`git merge-base A B` and check the file exists there before concluding anything from a `weave preview`** — and prefer a synthetic two-branch repo for a real language-support verdict. -**Naming wart** (`sem 0.25.0`, not Dart-specific): the `Class::method` form -resolves for `impact` but is rejected by `callers` -(`error: no entity named 'DittoService::initialize'`). A bare name prints a -disambiguation list; `--entity-id '::class::'` works everywhere. +**Naming** (`sem 0.27.0`): the qualified form is `Class.method` (also +`module.func`, `name@line`), accepted by `find` and `impact`. `Class::method` +is not, but the error now names the `Owner.method` spelling to retry with. When +the name exists in several files, pass `--file` (a directory works too) or +`--entity-id '::class::'`, which works everywhere. ## sem — entity diffs, blame, impact @@ -158,42 +172,123 @@ sem diff --json # machine-readable; carries entity IDs for `impac sem impact # deps, dependents, transitive impact, tests sem impact --tests # only the affected test entities sem impact --depth 0 # unlimited transitive depth (default is 2) -sem callers # direct callers only — cheaper than full impact -sem refs # what the entity itself calls +sem impact --diff # the same for a whole change (e.g. --diff HEAD~1) sem find # locate an entity's definition -sem blame # who last modified each entity in the file -sem log # history of one function/class (-v shows content diffs) -sem log --limit 200 # no entity: repo hotspots + co-change pairs (default scans 50) -sem entities --json # list parsed entities -sem context # token-budgeted LLM context for an entity +sem find --callers # direct callers only — cheaper than full impact +sem find --refs # what the entity itself calls +sem find --context # token-budgeted LLM context for an entity +sem find --in --json # list parsed entities +sem history # history of one function/class (-v shows content diffs) +sem history --limit 200 # no entity: repo hotspots + co-change pairs (default scans 50) +sem history --blame # who last modified each entity in the file +sem graph --json # every entity and edge (fromEntity/toEntity/refType) +sem certify # review certificate: signature changes, callers left behind +sem certify --arch --view # at most 10 ranked architecture items +sem certify --html > f.html # module-level architecture view, one page ``` -Commands checked against `sem 0.25.0`. When an entity name is ambiguous, pass -`--file ` to disambiguate. +Commands checked against `sem 0.27.0`, which folded the old verbs into +these. `callers`, `refs`, `context`, `entities`, `log` and `blame` still +work with identical output (at a terminal they print a one-line note naming +the new spelling), so either form in older notes is fine. When an entity +name is ambiguous, pass `--file ` to disambiguate. Notes: - **Before large refactors**, run `sem impact` on the entities you're about to change — it's the cheap version of "read every caller". For a rename, - `sem callers` is usually the whole answer. **Caveat: the call graph is - only as good as the language support** — in Dart it is file-local and - "no other entities are affected" is routinely wrong. See - **Language coverage: Dart/Flutter** above. -- `sem setup` rewires `git diff` output globally and `sem unsetup` reverts it. - That mutates the user's git config: **propose it, never run it unprompted.** + `sem find --callers` is usually the whole answer. **Caveat: the call graph + is only as good as the language support.** Since 0.26 sem says when its + answer is partial: `INCOMPLETE` plus a "possible callers" list means + "none resolved" is *not* "none". Check that list before you refactor. +- **`sem certify` is the "what should a human review" verb** and is local and + read-only. Use the plain certificate to list signature changes with the + callers a change left unmodified. Use `--arch --view` for architecture + deltas (new data paths, side effects, dependencies, cycles). Its `--html` + page is a *module*-level graph — boxes per directory, not per function — so + it complements the entity diagram below and does not replace it. +- `sem check` runs the project's own checkers and tests (exit 0 pass, 1 fail, + 2 undecided). It executes repo code, so treat it like running the test + suite, not like a read. +- `sem config setup` (`sem setup` before 0.27) rewires `git diff` output + globally and `sem config unsetup` reverts it. That mutates the user's git + config: **propose it, never run it unprompted.** - **Everything above runs locally, and that is the default** — cloud is "off until you enable it" and telemetry is "local by default — nothing uploaded" - (sem's own help text). `sem login`, `sem cloud`, `sem review`, `sem xref` - and `sem telemetry on` change that by sending repo-derived data to Ataraxy's - servers. On a machine holding work code that is the user's call alone: - never run them, and don't suggest them as a performance fix. -- `sem mcp` serves these as MCP tools (`sem_impact`, `sem_context`, `sem_diff`, - `sem_entities`, `sem_blame`, `sem_log`). If a `sem` MCP server is already - registered in the session, prefer its tools over shelling out. + (sem's own help text). `sem cloud login|review|xref|enable` and + `sem config telemetry on` change that by sending repo-derived data to + Ataraxy's servers. On a machine holding work code that is the user's call + alone: never run them, and don't suggest them as a performance fix. +- `sem mcp` serves the verbs as MCP tools (`sem_find`, `sem_grep`, + `sem_impact`, `sem_check`, `sem_certify`, `sem_diff`, `sem_graph`, + `sem_history`; the pre-0.27 names still answer when called by name). If a + `sem` MCP server is already registered in the session, prefer its tools + over shelling out — but not `sem_check` unprompted, for the reason above. - **In the dotfiles repo, CI already posts a sticky sem entity-diff comment on every PR** (`.github/workflows/entity-diff.yml`, `Ataraxy-Labs/sem/action`). Don't post duplicate entity-diff comments there — read the existing one. +## Drawing it — Mermaid from sem's graph, never from reading code + +When a picture explains a change better than a list ("what does touching +`Greeter` reach?", "show me the blast radius"), draw it from sem's data. +humanlayer's `show-me` skill draws call trees from the model's reading of +the code; that is the failure to avoid here, because an invented edge looks +exactly as authoritative as a real one. **Every node and edge in the diagram +comes from sem output.** + +**Impact → flowchart:** run the bundled renderer (in this skill's +directory): + +```bash +scripts/sem-mermaid.py -- --file # args after -- go to sem impact +scripts/sem-mermaid.py --max-nodes 30 -C ../repo -- --entity-id '' --depth 1 +``` + +It takes nodes from `sem impact --json` and edges from `sem graph --json`, +filtered to those nodes. Impact's JSON gives each entity's depth but not +which entity it was reached *through*, so depth-2 arrows drawn from impact +alone would be guesses. Output: target bold, resolved edges solid (non-call +edges labelled, e.g. `typeref`), sem's possible callers dotted and labelled +`unresolved`, and `%%` comments for `INCOMPLETE` reasons and "no test +reaches the target". Paste it in a ```` ```mermaid ```` block. It refuses +rather than drawing something misleading: + +- **exit 2** when `sem graph` has no edges in the target's language. An + empty graph drawn as a diagram reads as "nothing depends on this", which + is exactly the false negative the Dart section warns about. Fall back to + `rg`. +- **exit 3** above `--max-nodes` (default 20). Past about 20 nodes a Mermaid + flowchart stops being readable. Lower `--depth` instead of raising the cap + where you can. +- Possible callers from another language family are dropped, and a `%%` + line counts them. sem 0.27 matches unresolved names across languages, and + a TS function cannot be called from Dart. + +Verified against sem 0.27.0 on a synthetic TS + Dart repo, with both +diagrams rendered by Mermaid 11.4.1. The script calls only `sem impact +--json` and `sem graph --json`. + +**Diff → text tree, not Mermaid.** For "what changed", a diff-shaped entity +tree read off `sem diff --json` (`changeType`, `entityType`, `entityName`, +`filePath`) beats a graph. It has no edges, so nothing in it can be +invented: + +```diff + ts/greeter.ts + class Greeter +~ method hello # signature: hello() → hello(greeting = "hi") + ts/app.ts ++ function typed +``` + +**Architecture → `sem certify --html`.** That is sem's own +self-contained page: no external resources, so it is safe to open locally. +Use it for module-level questions, and the flowchart for entity-level ones. + +Keep one diagram per question, placed next to the sentence it supports. +If the answer fits in two sentences, don't draw. + ## weave — semantic merge driver Line-based merge invents conflicts when two independent changes touch nearby @@ -355,6 +450,8 @@ with `GITHUB_TOKEN`, falling back to `gh`'s login. - "What did this change put at risk that it didn't touch?" → `inspect predict` - History questions ("when did this function change / who owns it") → `sem log` / `sem blame` -- **Dart/Flutter repo** → `sem diff` / `blame` / `log` / `context` and - `weave` are fine; `sem impact` / `callers` / `refs` return empty graphs, - and inspect doesn't parse Dart. See **Language coverage: Dart/Flutter**. +- "What should a human look at in this range?" → `sem certify ` +- "Show me" what a change reaches → `scripts/sem-mermaid.py` (see + **Drawing it**) +- **Dart/Flutter repo** → sem (≥ 0.26) and `weave` as usual; inspect + doesn't parse Dart. See **Language coverage: Dart/Flutter**. diff --git a/tools/agents/skills/entity-level-git/scripts/__pycache__/sem-mermaid.cpython-313.pyc b/tools/agents/skills/entity-level-git/scripts/__pycache__/sem-mermaid.cpython-313.pyc new file mode 100644 index 00000000..6e97f4cc Binary files /dev/null and b/tools/agents/skills/entity-level-git/scripts/__pycache__/sem-mermaid.cpython-313.pyc differ diff --git a/tools/agents/skills/entity-level-git/scripts/sem-mermaid.py b/tools/agents/skills/entity-level-git/scripts/sem-mermaid.py new file mode 100755 index 00000000..486fc0c1 --- /dev/null +++ b/tools/agents/skills/entity-level-git/scripts/sem-mermaid.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +"""Render `sem impact` as a Mermaid flowchart, drawing only edges sem reported. + + sem-mermaid.py [--max-nodes N] [-C DIR] -- + sem-mermaid.py -- Greeter --file src/greeter.ts --depth 1 + +Nodes come from `sem impact --json` (the entity, its dependencies, and every +transitively affected entity). Edges come from `sem graph --json`, filtered +to that node set: impact's JSON gives each entity's depth but not which +entity it was reached through, so drawing depth-2 arrows from impact alone +would mean guessing them. Possible callers that sem could not resolve are +drawn dotted, because an unresolved caller is still a caller to check. + +Refuses to draw (exit 2) when the graph has no edges - an empty graph +rendered as a diagram reads as "nothing depends on this" - and (exit 3) when +the node count exceeds --max-nodes, rather than silently dropping nodes. +Read-only: runs `sem impact --json` and `sem graph --json`, nothing else. +""" + +import argparse +import json +import subprocess +import sys + + +def sem(args, cwd): + out = subprocess.run(["sem", *args], cwd=cwd, capture_output=True, text=True) + if out.returncode != 0: + sys.exit(f"sem {' '.join(args)} failed ({out.returncode}):\n{out.stderr.strip()}") + return json.loads(out.stdout) + + +# Extensions that can call each other; anything else is its own family. +FAMILIES = [{"ts", "tsx", "mts", "cts", "js", "jsx", "mjs", "cjs"}, + {"c", "h", "cc", "cpp", "cxx", "hh", "hpp", "hxx"}] + + +def family(path): + ext = path.rsplit(".", 1)[-1] + return next((frozenset(f) for f in FAMILIES if ext in f), ext) + + +def label(e): + name = e.get("name") or e.get("entity") + line = (e.get("lines") or [e.get("start_line")])[0] + return f'{e.get("type", "?")} {name}
{e["file"]}:{line}'.replace('"', "#quot;") + + +def main(): + p = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + p.add_argument("--max-nodes", type=int, default=20) + p.add_argument("-C", dest="cwd", default=None, help="repository to run sem in") + p.add_argument("impact_args", nargs=argparse.REMAINDER) + a = p.parse_args() + impact_args = [x for x in a.impact_args if x != "--"] + if not impact_args: + p.error("pass the entity to sem impact after --") + + graph = sem(["graph", "--json"], a.cwd) + imp = sem(["impact", *impact_args, "--json"], a.cwd) + target = imp["entity"] + + # Entity ids are "::::". Count edges in the target's + # language, not the repo: a mixed repo can have edges in TS and none in + # the language you asked about, and that language's diagram would lie. + lang = family(target["file"]) + lang_edges = sum(1 for e in graph.get("edges", []) + if family(e["fromEntity"].split("::", 1)[0]) == lang) + if not lang_edges: + print(f"sem graph has 0 edges from {target['file']}'s language: no dependency " + "data, so a diagram would be a false 'nothing depends on this'. " + "Not drawing. Fall back to `rg` for callers.", file=sys.stderr) + sys.exit(2) + + nodes = {target["entityId"]: target} + for e in imp.get("dependencies", []) + imp.get("impact", {}).get("entities", []): + nodes.setdefault(e["entityId"], e) + # sem's possible-caller list matches by name across languages (verified on + # 0.27.0: a TS method lists a Dart caller). A call cannot cross languages, + # so keep those sharing the target's file extension and say what was cut. + every_possible = imp.get("possibleCallers", []) + possible = [pc for pc in every_possible if family(pc["file"]) == lang] + dropped = len(every_possible) - len(possible) + if len(nodes) + len(possible) > a.max_nodes: + print(f"{len(nodes)} entities + {len(possible)} possible callers exceed " + f"--max-nodes {a.max_nodes}; lower --depth or raise the cap.", file=sys.stderr) + sys.exit(3) + + ids = {eid: f"n{i}" for i, eid in enumerate(nodes)} + tests = {t["entityId"] for t in imp.get("tests", [])} + deps = {d["entityId"] for d in imp.get("dependencies", [])} + + out = ["flowchart LR", + " %% sem impact + sem graph; solid edges resolved, dotted unresolved"] + for eid, e in nodes.items(): + out.append(f' {ids[eid]}["{label(e)}"]') + for edge in graph["edges"]: + f, t = edge["fromEntity"], edge["toEntity"] + if f in ids and t in ids and f != t: + arrow = "-->" if edge.get("refType") == "calls" else f'-- {edge.get("refType")} -->' + out.append(f" {ids[f]} {arrow} {ids[t]}") + for i, pc in enumerate(possible): + pid = f"p{i}" + out.append(f' {pid}["{label(pc)}"]:::possible') + out.append(f" {pid} -. unresolved .-> {ids[target['entityId']]}") + + out.append(f" class {ids[target['entityId']]} target") + for group, cls in ((tests, "test"), (deps, "dep")): + members = [ids[x] for x in group if x in ids] + if members: + out.append(f" class {','.join(members)} {cls}") + out += [" classDef target stroke-width:3px", + " classDef test stroke-dasharray:2 2", + " classDef dep opacity:0.7", + " classDef possible stroke-dasharray:5 5,opacity:0.7"] + if not imp.get("callersComplete", True): + why = "; ".join(r["detail"] for r in imp.get("incompleteBecause", [])) + out.insert(2, f" %% INCOMPLETE caller set: {why}") + if dropped: + out.insert(2, f" %% {dropped} possible caller(s) in other languages omitted") + if imp.get("noTestReaches"): + out.insert(2, " %% no test reaches the target through resolved callers") + print("\n".join(out)) + + +if __name__ == "__main__": + main()