This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit a320000c226d629a8156f306760098126c067b1f Author: Stephen Mallette <[email protected]> AuthorDate: Fri Oct 9 15:07:54 2026 -0400 Bring a PR's beads into tinker-review Reviews now find the beads recorded for a PR through its record beads and show the plan and decisions in a Project Memory section; beads (bd) is required. Reviews of already-merged PRs stop with a clear message. Assisted-by: Claude Code:claude-opus-5-5 --- .beads/PRIME.md | 4 +- .skills/tinker-review/DESIGN.md | 6 +- .skills/tinker-review/SKILL.md | 24 +- .skills/tinker-review/playbooks/general.md | 39 ++- .skills/tinker-review/references/enrichment-cli.md | 18 ++ .skills/tinker-review/references/interfaces.md | 13 +- .skills/tinker-review/references/schema.md | 36 ++- .skills/tinker-review/scripts/discovery/beads.js | 289 +++++++++++++++++++++ .skills/tinker-review/scripts/enrichment/api.js | 102 ++++++++ .skills/tinker-review/scripts/enrichment/cli.js | 12 +- .../scripts/extraction/tree-sitter.js | 3 +- .../tinker-review/scripts/graph/populate-beads.js | 133 ++++++++++ .skills/tinker-review/scripts/renderer/render.js | 199 +++++++++++++- .../tinker-review/scripts/renderer/template.html | 8 + .skills/tinker-review/scripts/review.js | 29 ++- .skills/tinker-review/test/beads.test.js | 281 ++++++++++++++++++++ 16 files changed, 1162 insertions(+), 34 deletions(-) diff --git a/.beads/PRIME.md b/.beads/PRIME.md index 0e35f6369b..68fd335cd5 100644 --- a/.beads/PRIME.md +++ b/.beads/PRIME.md @@ -102,7 +102,7 @@ Every session works under one **root bead**. Find it before writing code. ```bash bd list --status=all --json # filter client-side: no parent, open/in_progress -bd children <root> # recursive — the whole subtree +bd children <root> # recursive — the whole subtree (--json: direct children only) ``` - Show the operator open/`in_progress` beads with **no parent**, most recently updated @@ -335,7 +335,7 @@ release build ## Essential commands ```bash -bd children <root> # the subtree, recursive +bd children <root> # the subtree, recursive (--json: direct children only) bd ready --parent <root> # startable now; ALWAYS scope to your root bd blocked --parent <root> # what is waiting, and on what bd show <id> # one bead with dependencies diff --git a/.skills/tinker-review/DESIGN.md b/.skills/tinker-review/DESIGN.md index 130a3c655c..f6b2598a61 100644 --- a/.skills/tinker-review/DESIGN.md +++ b/.skills/tinker-review/DESIGN.md @@ -9,8 +9,9 @@ document is only the machinery and its extension points. The pipeline is two phases, split by what is mechanical and what needs judgment. **Phase 1 — deterministic** (`scripts/review.js`) builds the graph and writes -`evidence.json`: fetch PR → worktree → start Gremlin Server → tree-sitter -extract → populate → discover discussions → run the structural checks. It exits +`evidence.json`: beads gate and pull → fetch PR → worktree → start Gremlin +Server → tree-sitter extract → populate → discover discussions → discover beads +→ run the structural checks. It exits cleanly (a `PHASE1_COMPLETE` sentinel) so a caller can detect completion; the Gremlin Server container stays up. @@ -38,6 +39,7 @@ has already exited. |------|------| | `scripts/review.js` | Phase 1 orchestrator — `setup` / `phase1` / `teardown` | | `scripts/extraction/tree-sitter.js` | source → structural extraction | +| `scripts/discovery/{discussions,beads}.js` | Phase 1 context discovery — JIRA, dev list, proposals and PR comments over HTTP (`discussions.js`); the PR's beads through the local `bd`, read-only (`beads.js`) | | `scripts/graph/*.js` | populate the graph; `confidence.js` / `externals.js` / `references.js` hold the data-model vocabularies and shared edge helpers | | `scripts/patterns/*.js` | one structural check per file; each defines its own result `@typedef` | | `scripts/enrichment/{api,cli}.js` | Phase 2 read/write commands over the live graph | diff --git a/.skills/tinker-review/SKILL.md b/.skills/tinker-review/SKILL.md index 34564b25e5..80b0c41d93 100644 --- a/.skills/tinker-review/SKILL.md +++ b/.skills/tinker-review/SKILL.md @@ -6,7 +6,7 @@ description: > playbooks, runs structural analysis, and produces an HTML evidence package. Use when asked to review a TinkerPop PR by number. license: Apache-2.0 -compatibility: Requires Docker, Node.js 20+, git. Network access for fetching PR refs. +compatibility: Requires Docker, Node.js 20+, git and beads (bd). Network access for fetching PR refs. metadata: version: "0.1.0" project: Apache TinkerPop @@ -19,6 +19,9 @@ metadata: - Docker running (for Gremlin Server) - `upstream` remote pointing to `[email protected]:apache/tinkerpop.git` (fetch only) - Node.js 20+ with dependencies installed in `.skills/tinker-review/` +- Beads (`bd`) installed, with the repo's beads database available (`bin/agent-setup.sh --contributor`). + The review stops before fetching the PR when `bd` is missing. It runs `bd dolt pull` first and + falls back to local state if the pull fails; it never writes to beads. ## References (load on demand) @@ -46,9 +49,11 @@ npm install --prefix .skills/tinker-review # only needed once node .skills/tinker-review/scripts/review.js <pr-number> <repo-path> ``` -This performs: fetch PR → create worktree → start Gremlin Server → extract -structure via Tree-sitter → populate knowledge graph → discover discussions -(JIRA, dev list, proposals, PR comments) → run pattern checks (completeness, +This performs: check for beads and pull it → fetch PR → create worktree → start +Gremlin Server → extract structure via Tree-sitter → populate knowledge graph → +discover discussions (JIRA, dev list, proposals, PR comments) → discover the PR's +beads (records naming the PR or those discussions, their roots and subtrees) → +run pattern checks (completeness, coverage gaps, centrality, blast radius, cluster analysis) → write evidence JSON. **Output:** `/tmp/pr-review-<pr>/evidence.json` @@ -177,8 +182,8 @@ command does and how to drive it. - Relevant Gherkin test features (for expected behavior reference) - The Gremlin Server URL (the `url` from the command above) -The subagent does NOT get: source code, the knowledge graph, code review findings, -or access to the analysis worktree. Brief it as a **minimally experienced +The subagent does NOT get: source code, the knowledge graph, the PR's beads, code +review findings, or access to the analysis worktree. Brief it as a **minimally experienced TinkerPop user** who has only the docs — it tests blind, and its stumbles are signal about how usable the feature is. @@ -264,11 +269,14 @@ produce a complete evidence-with-narrative JSON file. Write it to symbol's lines highlighted. Two or three refs per question is plenty. - `refs` on the item — optional "Also see" chips for code that belongs to the whole area. + - A ref can cite a bead instead of code: `{ bead: "tp-abc.3", label? }` links to that bead + in the report's Project Memory. Use it where a question or finding rests on a recorded + decision or task; it must be a bead in `discussions.beads.beads`. `render.js` prints any ref it cannot resolve; fix the `file`/`symbol` and re-render. An item with only an HTML `body` still renders (older reports). -- `findings` — array of `{ title, snippet, body }` objects, ordered most-severe-first (Interpret grades each blocking / high / low) -- `openQuestions` — array of `{ title, body, meta }` objects +- `findings` — array of `{ title, snippet, body, refs? }` objects, ordered most-severe-first (Interpret grades each blocking / high / low); `refs` take the same code or bead refs as `guidedWalk` +- `openQuestions` — array of `{ title, body, meta, refs? }` objects - `functionalTest` — `{ plan, results: [{name, pass, output}], observations }` (if testing was done). `plan` is HTML and `observations` is an array of HTML strings (one per insight); both and surface **themes and insights** — what families of behavior were exercised and what was learned — not a per-scenario diff --git a/.skills/tinker-review/playbooks/general.md b/.skills/tinker-review/playbooks/general.md index ce4f197ea5..d044c93a67 100644 --- a/.skills/tinker-review/playbooks/general.md +++ b/.skills/tinker-review/playbooks/general.md @@ -17,6 +17,15 @@ inherits it. Run in order: - `auditConfidence` again — anything still `AMBIGUOUS` goes to `openQuestions`, never asserted as fact. +**Project memory** — the PR's beads, when a record named the PR or its +discussions (`discussions.beads.roots` is non-empty). Run these before the +confidence pass so the `governs` edges they write go through it: +- `listBeads` — read the roots, decisions (each chosen one with the alternatives + it beat) and tasks. The `primary` root is the PR's own work; `owner` and + `shared` roots are connected work that explains it. +- `linkBead` — link each decision whose design is about changed code to that + Function, Type or File, chosen and rejected alike. + ## Inspect **Style:** - Wildcard imports in Java (`import foo.*`) @@ -35,6 +44,19 @@ inherits it. Run in order: - Error/exception paths that aren't tested - Test helpers without guard clauses (missing else/throw for invalid input) +**Project memory** (only when beads were found; read decisions by `listBeads`): +- Code against chosen decisions — does the diff do something a decision's design + rules out? Judge against the newest decision in a `supersededBy` chain. +- Code against rejected alternatives — each one is a "why not X?" already + answered; don't raise it again. But if the diff *does* X, check whether the + reason X lost still holds in this code. +- `in_progress` tasks under the PR's root against what the PR says it delivers — + claimed work that isn't here, or is here half-done. +- Decisions that record a departure from the JIRA, proposal or dev@ thread — + does the PR description say so too? +- When several roots were loaded, which one does this PR deliver, and what do + the others show about the connected work it depends on or affects? + **Resource safety:** - Connections, channels, or streams opened without a clear cleanup path - Log levels: error for unexpected failures, info for expected lifecycle events @@ -65,7 +87,8 @@ mistakes a real user would make: boundary values, and the feature used against the grain of the docs. The blind subagent designs and runs this battery from the docs alone — see -SKILL.md step 4 for how it is briefed and isolated. +SKILL.md step 4 for how it is briefed and isolated. It never gets the beads: the +author's recorded reasoning is as much inside knowledge as the source code. ## Interpret - `checks.coverageGaps` / `checks.orphans` — missing tests on changed code; a @@ -79,6 +102,20 @@ SKILL.md step 4 for how it is briefed and isolated. - Style nits and unused variables — low; note them, don't let them dominate. - Formatting mixed with functional changes — high; it makes the PR harder to review and should ideally be separate commits. +- `discussions.beads` — the root is expected to be open: it closes at merge, so + an open root is never a finding, and neither is finding no beads or a failed + sync (the report says which). +- Diff does what a rejected alternative ruled out and the reason it lost still + holds — high. If that reason no longer holds, an open question: was this a + deliberate reconsideration? Diff contradicts a chosen decision with no newer + decision superseding it — high. +- An `in_progress` task under the PR's root that the PR doesn't finish — high + when the PR claims to be complete. An open, unclaimed task — an open question + only: an epic root spans several PRs, so it may belong to a later one. +- An open bead labelled `human` under a loaded root — an open question no one + has answered; carry it into `openQuestions`. +- The root's release label doesn't fit `meta.baseBranch` (a `3.7.x` root on a + PR to `master`) — an open question about where the work should land. - Deprecated API in new code — high. Deprecated API already present in modified code — low, unless the PR is specifically a migration away from it. diff --git a/.skills/tinker-review/references/enrichment-cli.md b/.skills/tinker-review/references/enrichment-cli.md index e42247c603..36cf4eaaf7 100644 --- a/.skills/tinker-review/references/enrichment-cli.md +++ b/.skills/tinker-review/references/enrichment-cli.md @@ -103,6 +103,16 @@ crucially, **flags any whose name matches a deleted symbol**: a changed file still calling a just-removed name is a dangling reference the graph catches on its own. Sorted so the dangerous ones surface first. +### listBeads +The beads Phase 1 loaded for this PR — the roots reached from records naming the +PR or its discussions, and everything under them — in reading order: the primary +root (the PR's own record) first, then owner and shared roots, each followed by +its decisions, tasks and records. A chosen decision lists the rejected +`alternatives` it beat and any decision it was `supersededBy`; `governs` shows +code already linked with `linkBead`. Filter with `--root`, `--type` or `--status` +(`--status in_progress` is the claimed, unfinished work). Empty when no record +matched — the review then simply proceeds without beads. + --- ## Agent-facing — write @@ -137,6 +147,14 @@ enrichment turns up prior context the Phase-1 discovery pass missed, so the report can cite where the change was debated. `--source` is `jira | devlist | proposal`. +### linkBead +Records that a bead — usually a decision — governs a piece of the changed code, +as a `governs` edge (Bead → Function, Type or File). Draw it when a decision's +design is about that code, so Inspect checks the code against what the decision +chose and ruled out, and the report can cite the decision beside it. Key a +Function/Type by `--name` (add `--file` when the name repeats) and a File by +`--file`. Defaults to `INFERRED`: the link is your judgment, not a bd fact. + ### linkDoc Records that a documentation file documents a graph entity, via a `documents` edge from a Doc vertex to the named entity (`--entity` label + `--name`). Use it diff --git a/.skills/tinker-review/references/interfaces.md b/.skills/tinker-review/references/interfaces.md index 60043ef934..c78eed686b 100644 --- a/.skills/tinker-review/references/interfaces.md +++ b/.skills/tinker-review/references/interfaces.md @@ -105,12 +105,14 @@ interface PopulationSummary { // ArchitectureResult scripts/patterns/architecture.js // CodeIndex scripts/extraction/code-index.js // CodeRef scripts/renderer/code-links.js +// BeadsDiscovery scripts/discovery/beads.js (with Bead, BeadRoot) // === Evidence (evidence.json — what Phase 1 writes; the fields Interpret cites) === interface Evidence { meta: { pr: number; + baseBranch: string; // the branch the PR targets (master, 3.8-dev, 3.7-dev) headSha: string | null; // PR head commit reviewed; pins report code links title: string; domains: string[]; // e.g. ["general", "glv", "driver-server"] @@ -130,7 +132,10 @@ interface Evidence { externals: ExternalsResult; orphans: OrphanResult; }; - discussions: DiscussionsResult; // jiras[], devList[], secondary[], proposals[], prComments{} + discussions: DiscussionsResult & { beads: BeadsDiscovery }; + // jiras[], devList[], secondary[], proposals[], prComments{}; beads = the roots + // reached from record beads naming the PR or those discussions, their subtrees, + // and whether bd dolt pull succeeded changedFiles: string[]; codeIndex: CodeIndex; // changed files → hunks + symbol line ranges (scripts/extraction/code-index.js) } @@ -154,8 +159,10 @@ interface ReportPackage extends Evidence { // CodeRef = { file; symbol?; lines?: [number, number]; label? } — see // scripts/renderer/code-links.js. The renderer resolves it against codeIndex; // the agent never writes line numbers or URLs. Legacy { body } items still render. - findings: { title; snippet; body }[]; - openQuestions: { title; body; meta }[]; + // A ref may instead be { bead; label? }: a chip linking to that bead in Project + // Memory; it must be one of discussions.beads.beads. + findings: { title; snippet; body; refs?: CodeRef[] }[]; + openQuestions: { title; body; meta; refs?: CodeRef[] }[]; functionalTest?: { plan; results: { name; pass; output }[]; observations: string[] }; // plan: HTML; observations: array of HTML strings; both theme-level. results rows are THEMES, each `name` // naming the scenario labels it spans — not one row per scenario. diff --git a/.skills/tinker-review/references/schema.md b/.skills/tinker-review/references/schema.md index f80dbef8b4..26fd9de4a1 100644 --- a/.skills/tinker-review/references/schema.md +++ b/.skills/tinker-review/references/schema.md @@ -1,6 +1,6 @@ # PR Knowledge Graph Schema -## Vertices (9 labels) +## Vertices (10 labels) ### Code structure @@ -109,7 +109,29 @@ The PR itself is a Discussion with `source: "pr"`. **Comment** `{ author, body, timestamp }` A comment on a Discussion. -## Edges (16 implemented + 1 planned) +### Project memory + +**Bead** `{ beadId, type, status, title, description, design, labels, root, rejected?, parent?, externalRef?, closedAt?, rootRole? }` +A bead from the reviewer's local beads database (`bd`), loaded because a record +bead's external ref names one of the PR's artifacts (see `discovery/beads.js`). +Every bead in the subtree of each root reached that way is loaded. `type` is +bd's issue type (`epic`, `feature`, `task`, `bug`, `decision`, `record`, …); +`root` is the id of the root the bead belongs to. `rejected` is set only on +decisions — `true` marks the road not taken, and `design` then says what the +option was, why it lost and what settled it. `externalRef` is set only on +records. `labels` is the comma-joined label list. + +`rootRole` is set only on roots: `primary` — the root of this PR's own record +(`apache/tinkerpop#N`); `owner` — the root a matched JIRA/dev@/proposal record +belongs to; `shared` — a root that links a matched record with `related` +(connected work). Query the PR's own work with `.has("rootRole", "primary")` +and a root's subtree with `.has("root", id)`. + +Status reads against the beads workflow: a root stays open until the work +merges, and decisions are closed as they are written, so at PR time status is +informative mainly on tasks (`in_progress` = claimed, unfinished). + +## Edges (19 implemented, plus bd's dependency types, + 1 planned) One edge is marked ⚠️ *planned* below (`depends_on`) — documented but intentionally not populated. @@ -182,3 +204,13 @@ renders this as the **Signal Confidence** panel. | `matched_in` | `reference`, `title`, `body` | How the proposal was linked: an explicit path reference in the PR (`EXTRACTED`), a keyword in the proposal's title/heading (`INFERRED`), or keywords in its body (`AMBIGUOUS`). | | `matched_keywords` | comma-separated terms | The keywords that matched, so the link is self-explanatory. | +### Project memory + +All `EXTRACTED` except `governs`: each is a fact in the beads database. + +| Edge | From | To | Meaning | +|------|------|----|---------| +| `child_of` | Bead | Bead | bd's parent-child tree: the bead belongs to its parent | +| `records` | Bead (record) | Discussion | The record's external ref names this Discussion (the PR, a JIRA, a dev@ thread, a proposal). Only drawn to Discussions discovery created. | +| `governs` | Bead | Function, Type or File | The bead — usually a decision — is about this code. Written in Phase 2 by `linkBead`, so it is `INFERRED` by default, unlike the rest of this table. | +| `blocks`, `caused_by`, `related`, `supersedes`, `discovered_from`, … | Bead | Bead | bd's other dependency types, carried over with `-` written as `_`. `related` pairs a decision with the alternative it was chosen over; `caused_by` runs from a task to the decision that shaped it; `supersedes` from a newer decision to the one it replaces. Drawn only between loaded beads. | diff --git a/.skills/tinker-review/scripts/discovery/beads.js b/.skills/tinker-review/scripts/discovery/beads.js new file mode 100644 index 0000000000..9896a1bd52 --- /dev/null +++ b/.skills/tinker-review/scripts/discovery/beads.js @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Beads discovery: find the bead roots recorded for the change under review and +// load their subtrees. The project's beads schema (.beads/PRIME.md section 6) +// puts every external artifact — JIRA, PR, dev@ thread, proposal — on a `record` +// bead that is a parent-child child of its root, and only records carry an +// external ref. A second root that needs the same artifact links to the existing +// record with a `related` edge. So a record whose ref names one of the PR's +// artifacts leads to the owning root and any sharing roots, and everything +// reached that way is EXTRACTED. +// +// The review reads beads and never writes them: every call except the sync +// passes bd's --readonly flag, which makes bd itself refuse a write. + +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +const exec = promisify(execFile); + +export const BEADS_REQUIRED_MESSAGE = + "tinker-review requires beads (bd). Install it and run `bin/agent-setup.sh --contributor`, then re-run the review."; + +/** + * @typedef {object} Bead + * A bead from the reviewer's local beads database. + * @property {string} id - Bead id (e.g. `tp-hv9.1`). Never parse it for structure; `parent` is the truth. + * @property {string} type - bd issue type: `epic`, `feature`, `task`, `bug`, `decision`, `record`, ... + * @property {string} status - `open`, `in_progress`, `closed`, `pinned`, ... At PR time the root is + * expected to be open (it closes at merge), and decisions are closed as they are written, so + * status means most on tasks: `in_progress` is claimed, unfinished work. + * @property {string} title + * @property {string} description + * @property {string} design - For a decision, the reasoning: why it was chosen, or what the rejected + * option was, why it lost and what settled it. + * @property {boolean|null} rejected - On a decision, true marks the road not taken; null on other types. + * @property {string[]} labels + * @property {string|null} parent - Parent bead id; null on a root. + * @property {string|null} externalRef - The artifact a record names (`TINKERPOP-NNNN`, + * `apache/tinkerpop#N`, `apache/tinkerpop@sha`, a dev@ URL); null on non-records. + * @property {string|null} closedAt + * @property {string} root - Id of the root this bead belongs to. + * @property {{ to: string, type: string }[]} edges - Outgoing bd dependencies other than + * parent-child (`blocks`, `caused-by`, `related`, `supersedes`, `discovered-from`, ...). + */ + +/** + * @typedef {object} BeadRoot + * A root reached from a matched record. + * @property {string} id - Root bead id. + * @property {"primary"|"owner"|"shared"} role - `primary`: the root of this PR's own record + * (`apache/tinkerpop#N`), the work the PR delivers. `owner`: the root a matched record belongs + * to. `shared`: a root that links a matched record with `related` — connected work elsewhere. + * @property {string[]} via - Ids of the matched records that reached this root. + */ + +/** + * @typedef {object} BeadsDiscovery + * What beads discovery found for the PR. Present in `evidence.discussions.beads`. + * @property {boolean} synced - Whether `bd dolt pull` succeeded. When false the review used the + * local database as it was, which may be missing beads pushed since the last sync. + * @property {string|null} syncError - Why the sync failed, when it did. + * @property {string[]} seeds - External refs searched: the PR, its JIRAs, explicitly linked dev@ + * threads and explicitly referenced proposals. + * @property {{ seed: string, record: string }[]} matches - Record beads whose external ref equals a seed. + * @property {BeadRoot[]} roots - Roots reached from the matches, primary first. Empty when no + * record matched; the review then proceeds without beads, and that is not a finding. + * @property {Bead[]} beads - Every bead in the reached roots' subtrees, roots included. + * @property {Object<string, {author: string, text: string, createdAt: string}[]>} comments - + * Comments by bead id, for roots and decisions (where facts and later context are recorded). + */ + +/** + * A function that runs bd with the given arguments from the repo checkout and + * returns its parsed JSON output. Read commands get --readonly and --json added. + * @callback BdRunner + * @param {string[]} args + * @returns {Promise<any>} + */ + +/** + * Build a {@link BdRunner} that runs the real bd from `repoPath`. + * @param {string} repoPath + * @returns {BdRunner} + */ +export function bdRunner(repoPath) { + return async (args) => { + const { stdout } = await exec("bd", [...args, "--readonly", "--json"], { + cwd: repoPath, + maxBuffer: 32 * 1024 * 1024, + }); + return stdout.trim() ? JSON.parse(stdout) : null; + }; +} + +/** + * Fail early when beads is unavailable: bd must be installed and must open the + * repo's database. Throws an Error carrying {@link BEADS_REQUIRED_MESSAGE}. + * @param {string} repoPath + * @param {BdRunner} [run] + */ +export async function requireBeads(repoPath, run = bdRunner(repoPath)) { + try { + await run(["list", "-n", "1"]); + } catch (err) { + const detail = err.code === "ENOENT" ? "bd was not found on the PATH" : (err.stderr || err.message || "").trim(); + throw new Error(`${BEADS_REQUIRED_MESSAGE}\n (${detail})`); + } +} + +/** + * Pull the shared beads database so the review sees the latest state. A failed + * pull is not fatal: the review continues on the local state and reports it. + * @param {string} repoPath + * @param {(args: string[]) => Promise<unknown>} [pull] - runs `bd <args>`; injectable for tests. + * @returns {Promise<{ synced: boolean, syncError: string|null }>} + */ +export async function syncBeads(repoPath, pull = (args) => exec("bd", args, { cwd: repoPath, timeout: 120000 })) { + try { + await pull(["dolt", "pull"]); + return { synced: true, syncError: null }; + } catch (err) { + return { synced: false, syncError: (err.stderr || err.message || "bd dolt pull failed").toString().trim() }; + } +} + +// Trailing punctuation a URL picks up from surrounding prose. +const stripTrailing = (s) => s.replace(/[.,)\]>]+$/, ""); + +/** + * The external refs a record could carry for this PR. Only artifacts the PR + * itself names are seeds — its own ref, its JIRAs, explicitly linked dev@ + * threads and explicitly referenced proposals. Keyword-searched dev@ threads and + * keyword-matched proposals are guesses, and an EXTRACTED match can't rest on one. + * @param {number} pr + * @param {object} discussions - Output of discoverDiscussions(). + * @returns {string[]} + */ +export function beadSeeds(pr, discussions = {}) { + const seeds = [`apache/tinkerpop#${pr}`]; + for (const j of discussions.jiras || []) seeds.push(j.id); + for (const d of discussions.devList || []) if (d.found_in === "pr") seeds.push(stripTrailing(d.url)); + for (const p of discussions.proposals || []) if (p.matchedIn === "reference") seeds.push(p.path); + return [...new Set(seeds.filter(Boolean))]; +} + +function toBead(raw) { + return { + id: raw.id, + type: raw.issue_type, + status: raw.status, + title: raw.title || "", + description: raw.description || "", + design: raw.design || "", + rejected: raw.issue_type === "decision" ? raw.metadata?.rejected === true : null, + labels: raw.labels || [], + parent: raw.parent || null, + externalRef: raw.external_ref || null, + closedAt: raw.closed_at || null, + root: null, + edges: (raw.dependencies || []) + .filter((d) => d.type !== "parent-child") + .map((d) => ({ to: d.depends_on_id, type: d.type })), + }; +} + +/** + * Find the bead roots recorded for a PR and load their subtrees. + * + * 1. For each seed, `bd search <prefix> --external-contains <seed> --type record`, + * keeping exact matches (the flag is a substring match). The positional query + * is also applied, as a title/ID match, so it is the project's ID prefix — which + * every bead matches — rather than the ref, which a record's title need not repeat. + * 2. Each matched record's top-most ancestor is its owner root; every bead that + * links the record with `related` leads to a shared root. A PR record's owner + * root is primary. Every root reached is loaded — connected work is context. + * 3. Load each root's subtree. `bd children --json` returns one level only (its + * text output is the tree), so the candidates come from + * `bd dep tree <root> --direction up`, which walks every + * dependent in one call — including beads outside the subtree that merely + * relate to one inside it. One `bd list --id` call then supplies each + * candidate's real `parent` and full fields, and only beads whose parent + * chain reaches the root are kept. Comments are read for the roots and + * decisions that have any. + * + * @param {object} params + * @param {string[]} params.seeds - From {@link beadSeeds}. + * @param {string} [params.prRef] - This PR's ref, `apache/tinkerpop#N`; marks its root primary. + * @param {BdRunner} params.run + * @returns {Promise<Omit<BeadsDiscovery, "synced"|"syncError">>} + */ +export async function discoverBeads({ seeds, prRef, run }) { + const result = { seeds, matches: [], roots: [], beads: [], comments: {} }; + + const [sample] = (await run(["list", "-n", "1", "--all", "--flat"])) || []; + if (!sample) return result; + const idPrefix = sample.id.slice(0, sample.id.indexOf("-") + 1); + + for (const seed of seeds) { + const hits = (await run(["search", idPrefix, "--external-contains", seed, "--type", "record", "--status", "all", "--limit", "100"])) || []; + for (const h of hits) { + if (h.external_ref === seed) result.matches.push({ seed, record: h.id }); + } + } + if (result.matches.length === 0) return result; + + const fetched = new Map(); + const fetch = async (ids) => { + const missing = ids.filter((id) => !fetched.has(id)); + if (missing.length === 0) return; + const rows = (await run(["list", "--id", missing.join(","), "--all", "-n", "0", "--flat"])) || []; + for (const r of rows) fetched.set(r.id, r); + }; + const topOf = async (id) => { + let cur = id; + const seen = new Set(); + for (;;) { + await fetch([cur]); + const parent = fetched.get(cur)?.parent; + if (!parent || seen.has(parent)) return cur; + seen.add(cur); + cur = parent; + } + }; + + const RANK = { primary: 0, owner: 1, shared: 2 }; + const roots = new Map(); + const reach = (rootId, role, record) => { + const r = roots.get(rootId) || { id: rootId, role, via: [] }; + if (RANK[role] < RANK[r.role]) r.role = role; + if (!r.via.includes(record)) r.via.push(record); + roots.set(rootId, r); + }; + + for (const { seed, record } of result.matches) { + reach(await topOf(record), seed === prRef ? "primary" : "owner", record); + const sharers = (await run(["dep", "list", record, "--direction", "up", "-t", "related"])) || []; + for (const s of sharers) reach(await topOf(s.id), "shared", record); + } + result.roots = [...roots.values()].sort((a, b) => RANK[a.role] - RANK[b.role]); + + for (const { id: rootId } of result.roots) { + if (result.beads.some((b) => b.root === rootId)) continue; + const tree = (await run(["dep", "tree", rootId, "--direction", "up"])) || []; + await fetch(tree.map((t) => t.id)); + const inSubtree = new Set([rootId]); + let grew = true; + while (grew) { + grew = false; + for (const { id } of tree) { + if (!inSubtree.has(id) && inSubtree.has(fetched.get(id)?.parent)) { + inSubtree.add(id); + grew = true; + } + } + } + for (const id of inSubtree) { + const raw = fetched.get(id); + if (raw) result.beads.push({ ...toBead(raw), root: rootId }); + } + } + + for (const b of result.beads) { + if (b.id !== b.root && b.type !== "decision") continue; + if (!fetched.get(b.id)?.comment_count) continue; + const comments = (await run(["comments", b.id])) || []; + if (comments.length > 0) { + result.comments[b.id] = comments.map((c) => ({ author: c.author, text: c.text, createdAt: c.created_at })); + } + } + + return result; +} diff --git a/.skills/tinker-review/scripts/enrichment/api.js b/.skills/tinker-review/scripts/enrichment/api.js index 3f21cdf042..2952ac251c 100644 --- a/.skills/tinker-review/scripts/enrichment/api.js +++ b/.skills/tinker-review/scripts/enrichment/api.js @@ -180,6 +180,73 @@ export async function listExternalRefs(g) { }); } +/** + * The beads Phase 1 loaded for this PR — the author's recorded plan, decisions + * and the alternatives they turned down — so the reviewer can read the change + * against them. Roots come first (primary, then owner, then shared), then each + * root's decisions, tasks and records. A decision lists the rejected + * alternatives it was chosen over (`related` to a rejected decision) and any + * decision that supersedes it; judge the code against the newest decision in a + * supersedes chain. `governs` lists code already linked with `linkBead`. + * Empty when no record named the PR or its discussions. + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @param {object} [filter] + * @param {string} [filter.root] - only this root's subtree + * @param {string} [filter.type] - bd type: decision | task | record | ... + * @param {string} [filter.status] - e.g. in_progress + * @returns {Promise<object[]>} + */ +export async function listBeads(g, filter = {}) { + let t = g.V().hasLabel("Bead"); + if (filter.root) t = t.has("root", filter.root); + if (filter.type) t = t.has("type", filter.type); + if (filter.status) t = t.has("status", filter.status); + const rows = await t.project("bead", "alternatives", "supersededBy", "governs") + .by(__.valueMap()) + .by(__.both("related").has("rejected", true).values("beadId").fold()) + .by(__.in_("supersedes").values("beadId").fold()) + .by(__.out("governs").coalesce(__.values("path"), __.values("name")).fold()) + .toList(); + + const one = (m, k) => { + const v = m.get(k); + return Array.isArray(v) ? v[0] : v; + }; + const beads = rows.map((r) => { + const m = r.get("bead"); + const bead = { + beadId: one(m, "beadId"), + type: one(m, "type"), + status: one(m, "status"), + title: one(m, "title"), + design: one(m, "design") || "", + root: one(m, "root"), + rootRole: one(m, "rootRole") ?? null, + rejected: one(m, "rejected") ?? null, + externalRef: one(m, "externalRef") ?? null, + labels: one(m, "labels") || "", + }; + if (bead.type === "decision") { + if (bead.rejected === false) bead.alternatives = r.get("alternatives"); + bead.supersededBy = r.get("supersededBy"); + } + const governs = r.get("governs"); + if (governs.length > 0) bead.governs = governs; + return bead; + }); + + const ROLE = { primary: 0, owner: 1, shared: 2 }; + const TYPE = { decision: 1, record: 3 }; + const roleOf = new Map(beads.filter((b) => b.rootRole).map((b) => [b.beadId, ROLE[b.rootRole]])); + const rank = (b) => [roleOf.get(b.root) ?? 3, b.root, b.rootRole ? 0 : (TYPE[b.type] ?? 2), b.beadId]; + return beads.sort((a, b) => { + const [x, y] = [rank(a), rank(b)]; + for (let k = 0; k < x.length; k++) if (x[k] !== y[k]) return x[k] < y[k] ? -1 : 1; + return 0; + }); +} + // === Write operations === /** @@ -327,6 +394,41 @@ export async function linkDiscussion(g, url, source, title, body, confidence) { return { linked: `${source}: ${title}`, confidence: conf }; } +/** + * Record that a bead — usually a decision — governs a piece of the changed + * code, as a `governs` edge (Bead → Function, Type or File). Draw it when a + * decision's design is about that code, so Inspect can check the code against + * what the decision chose and ruled out, and the report can cite the decision + * beside it. Key a Function or Type by `--name` (plus `--file` when the name + * repeats) and a File by `--file`. + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @param {string} beadId - The bead id (e.g. tp-abc.3) + * @param {string} entityLabel - Function | Type | File + * @param {string} [name] - Function/Type name + * @param {string} [filePath] - File path; for a Function/Type, disambiguates the name + * @param {string} [confidence] - default INFERRED (the agent's judgment) + * @returns {Promise<object>} + */ +export async function linkBead(g, beadId, entityLabel, name, filePath, confidence) { + const conf = normalizeConfidence(confidence, CONFIDENCE.INFERRED); + if (!(await g.V().hasLabel("Bead").has("beadId", beadId).hasNext())) { + return { linked: false, reason: `no Bead ${beadId} in the graph` }; + } + const match = (t) => { + if (entityLabel === "File") return t.has("path", filePath); + return filePath ? t.has("name", name).has("filePath", filePath) : t.has("name", name); + }; + const found = (await match(g.V().hasLabel(entityLabel)).count().next()).value; + if (found === 0) return { linked: false, reason: `no ${entityLabel} ${name || filePath} in the graph` }; + if (found > 1) return { linked: false, reason: `${found} ${entityLabel} vertices match ${name}; pass --file` }; + await g.V().hasLabel("Bead").has("beadId", beadId) + .addE("governs").property("confidence", conf) + .to(match(__.V().hasLabel(entityLabel))) + .next(); + return { linked: `${beadId} governs ${entityLabel}(${name || filePath})`, confidence: conf }; +} + /** * Set an arbitrary property on a vertex identified by label + name. The * general-purpose escape hatch for recording a fact the schema has no dedicated diff --git a/.skills/tinker-review/scripts/enrichment/cli.js b/.skills/tinker-review/scripts/enrichment/cli.js index cac84dcc07..f9fdfdfd92 100644 --- a/.skills/tinker-review/scripts/enrichment/cli.js +++ b/.skills/tinker-review/scripts/enrichment/cli.js @@ -23,7 +23,7 @@ import { pathToFileURL } from "node:url"; import gremlin from "gremlin"; import { listFunctions, listTypes, getCallsFrom, getCanonicalSteps, - listDeleted, listExternalRefs, addReference, + listDeleted, listExternalRefs, listBeads, addReference, linkBead, mapStep, setEdgeConfidence, linkDiscussion, linkDoc, addGrammarRule, annotate, linkRule, mapCoverage, createPrDiscussion, } from "./api.js"; @@ -47,11 +47,13 @@ const COMMANDS = { listInferred: { fn: listInferred, needsG: true, facing: "agent" }, listDeleted: { fn: listDeleted, needsG: true, facing: "agent" }, listExternalRefs: { fn: listExternalRefs, needsG: true, facing: "agent" }, + listBeads: { fn: listBeads, needsG: true, facing: "agent" }, // Agent-facing (Phase 2 enrichment) — write addReference: { fn: addReference, needsG: true, facing: "agent" }, mapStep: { fn: mapStep, needsG: true, facing: "agent" }, setEdgeConfidence: { fn: setEdgeConfidence, needsG: true, facing: "agent" }, linkDiscussion: { fn: linkDiscussion, needsG: true, facing: "agent" }, + linkBead: { fn: linkBead, needsG: true, facing: "agent" }, linkDoc: { fn: linkDoc, needsG: true, facing: "agent" }, addGrammarRule: { fn: addGrammarRule, needsG: true, facing: "agent" }, linkRule: { fn: linkRule, needsG: true, facing: "agent" }, @@ -100,10 +102,12 @@ async function main() { console.log(" listInferred [--relation implements_step|calls|...] [--limit 100]"); console.log(" listDeleted"); console.log(" listExternalRefs"); + console.log(" listBeads [--root <beadId>] [--type decision|task|record] [--status in_progress]"); console.log(" addReference --fromPath <path> --toPath <deletedPath> [--symbol <name>] [--location <L42>] [--confidence ...]"); console.log(" mapStep --function <name> --file <path> --step <canonicalName> [--confidence INFERRED|AMBIGUOUS|EXTRACTED]"); console.log(" setEdgeConfidence --relation <label> --fromName <name> [--fromFile <path>] [--toName <name>] --confidence <EXTRACTED|INFERRED|AMBIGUOUS>"); console.log(" linkDiscussion --url <url> --source <jira|devlist|proposal> --title <title> [--body <body>] [--confidence ...]"); + console.log(" linkBead --bead <beadId> --entity <Function|Type|File> [--name <name>] [--file <path>] [--confidence ...]"); console.log(" linkDoc --entity <label> --name <name> --doc <path> [--section <section>] [--confidence ...]"); console.log(" addGrammarRule --name <name> [--production <production>]"); console.log(" linkRule --step <canonicalName> --rule <ruleName> [--confidence ...]"); @@ -198,6 +202,12 @@ async function main() { confidence: args.confidence, }); break; + case "listBeads": + result = await fn(g, { root: args.root, type: args.type, status: args.status }); + break; + case "linkBead": + result = await fn(g, args.bead, args.entity, args.name, args.file, args.confidence); + break; case "linkDiscussion": result = await fn(g, args.url, args.source, args.title, args.body, args.confidence); break; diff --git a/.skills/tinker-review/scripts/extraction/tree-sitter.js b/.skills/tinker-review/scripts/extraction/tree-sitter.js index c8f7d4bfca..fddd85cc60 100644 --- a/.skills/tinker-review/scripts/extraction/tree-sitter.js +++ b/.skills/tinker-review/scripts/extraction/tree-sitter.js @@ -996,7 +996,8 @@ export async function extractMulti(directory, languages, options = {}) { const one = await extract(directory, language, options); merged.languages.push(language); for (const key of ["files", "functions", "types", "calls", "imports", "tests", "declares"]) { - merged[key].push(...one[key]); + // Not push(...one[key]): spreading a large array exceeds the call stack. + for (const item of one[key]) merged[key].push(item); } if (one.hierarchyNeighborhood) { merged.hierarchyNeighborhood.files += one.hierarchyNeighborhood.files; diff --git a/.skills/tinker-review/scripts/graph/populate-beads.js b/.skills/tinker-review/scripts/graph/populate-beads.js new file mode 100644 index 0000000000..a772c3b972 --- /dev/null +++ b/.skills/tinker-review/scripts/graph/populate-beads.js @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import gremlin from "gremlin"; +import { CONFIDENCE } from "./confidence.js"; + +const { process: { statics: __ } } = gremlin; + +const BATCH_SIZE = 50; + +async function submitBatch(batch) { + await Promise.allSettled(batch.map((t) => t.next())); +} + +/** + * The Discussion url a record's external ref names, matching the urls + * populateDiscussions() writes. Commit refs (`apache/tinkerpop@sha`) name no + * Discussion and return null. + * @param {string|null} ref + * @returns {string|null} + */ +export function discussionUrlForRef(ref) { + if (!ref) return null; + const pr = ref.match(/^apache\/tinkerpop#(\d+)$/); + if (pr) return `https://github.com/apache/tinkerpop/pull/${pr[1]}`; + if (/^TINKERPOP-\d+$/.test(ref)) return `https://issues.apache.org/jira/browse/${ref}`; + if (ref.includes("@")) return null; + return ref; +} + +// bd dependency types become edge labels with underscores: caused-by -> caused_by. +const edgeLabel = (type) => type.replace(/-/g, "_"); + +/** + * Load the beads found by discoverBeads() into the graph: a Bead vertex per + * bead, `child_of` for the parent-child tree, bd's other dependencies carried + * over as edges between loaded beads, and `records` from a record bead to the + * Discussion its external ref names. Every edge is EXTRACTED: each one is a + * fact in the beads database. + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @param {import("../discovery/beads.js").BeadsDiscovery} beads + * @returns {Promise<{vertices: number, edges: number, breakdown: object}>} + */ +export async function populateBeads(g, beads) { + const counts = { vertices: 0, edges: 0, breakdown: { beads: 0, childOf: 0, records: 0, dependencies: 0 } }; + if (!beads || !beads.beads || beads.beads.length === 0) return counts; + + const roleOf = new Map((beads.roots || []).map((r) => [r.id, r.role])); + const loaded = new Set(beads.beads.map((b) => b.id)); + + for (const b of beads.beads) { + let t = g.addV("Bead") + .property("beadId", b.id) + .property("type", b.type) + .property("status", b.status) + .property("title", b.title) + .property("description", (b.description || "").slice(0, 4000)) + .property("design", (b.design || "").slice(0, 4000)) + .property("labels", (b.labels || []).join(", ")) + .property("root", b.root); + if (b.rejected !== null) t = t.property("rejected", b.rejected); + if (b.parent) t = t.property("parent", b.parent); + if (b.externalRef) t = t.property("externalRef", b.externalRef); + if (b.closedAt) t = t.property("closedAt", b.closedAt); + if (roleOf.has(b.id)) t = t.property("rootRole", roleOf.get(b.id)); + await t.next(); + counts.vertices++; + counts.breakdown.beads++; + } + + const bead = (id) => __.V().hasLabel("Bead").has("beadId", id); + let batch = []; + const push = async (traversal, key) => { + batch.push(traversal); + counts.edges++; + counts.breakdown[key]++; + if (batch.length >= BATCH_SIZE) { await submitBatch(batch); batch = []; } + }; + + for (const b of beads.beads) { + if (b.parent && loaded.has(b.parent)) { + await push( + g.V().hasLabel("Bead").has("beadId", b.id) + .addE("child_of").property("confidence", CONFIDENCE.EXTRACTED) + .to(bead(b.parent)), + "childOf", + ); + } + for (const e of b.edges || []) { + if (!loaded.has(e.to)) continue; + await push( + g.V().hasLabel("Bead").has("beadId", b.id) + .addE(edgeLabel(e.type)).property("confidence", CONFIDENCE.EXTRACTED) + .to(bead(e.to)), + "dependencies", + ); + } + } + + // records: only to Discussions discovery already created, so a record for some + // other PR in a loaded subtree doesn't invent a Discussion vertex. + for (const b of beads.beads) { + if (b.type !== "record") continue; + const url = discussionUrlForRef(b.externalRef); + if (!url || !(await g.V().hasLabel("Discussion").has("url", url).hasNext())) continue; + await push( + g.V().hasLabel("Bead").has("beadId", b.id) + .addE("records").property("confidence", CONFIDENCE.EXTRACTED) + .to(__.V().hasLabel("Discussion").has("url", url)), + "records", + ); + } + + if (batch.length > 0) await submitBatch(batch); + return counts; +} diff --git a/.skills/tinker-review/scripts/renderer/render.js b/.skills/tinker-review/scripts/renderer/render.js index aeafc0ea27..aa5dc2440f 100644 --- a/.skills/tinker-review/scripts/renderer/render.js +++ b/.skills/tinker-review/scripts/renderer/render.js @@ -87,11 +87,12 @@ function stripCodeWrapper(str) { * guidedWalk: [{ title, badge: "attention|info|safe", badgeText, * intro: "HTML", questions: [{ text: "HTML", refs: [CodeRef] }], refs: [CodeRef] }] * (CodeRef = { file, symbol?, lines?, label? } — see code-links.js; the renderer resolves it - * against evidence.codeIndex into a link. A legacy item with only body: "HTML" still renders.) + * against evidence.codeIndex into a link. A ref may instead be { bead, label? }, which links + * to that bead in Project Memory. A legacy item with only body: "HTML" still renders.) * functionalTest: { plan: "HTML", results: [{name, pass, output}], observations: ["HTML"] } * (results rows are THEME-level, each naming the scenario labels it covers) - * findings: [{ title, snippet: "code", body: "HTML" }] - * openQuestions: [{ title, body: "HTML", meta: "string" }] + * findings: [{ title, snippet: "code", body: "HTML", refs?: [CodeRef] }] + * openQuestions: [{ title, body: "HTML", meta: "string", refs?: [CodeRef] }] * appendixFunctional: { environment: "HTML", testCode: "raw text", fullOutput: "raw text" } * (testCode = the COMPLETE labeled battery; renderer wraps it in <pre><code>, do NOT pre-wrap) */ @@ -104,6 +105,14 @@ export function render(evidence, options = {}) { const { meta, graphStats, checks, discussions, summary, clusters, communityAssessment, communityNames, guidedWalk, functionalTest, findings, openQuestions, appendixFunctional } = evidence; + const beads = discussions?.beads; + const refCtx = { + pr: meta.pr, + headSha: meta.headSha, + codeIndex: evidence.codeIndex, + beadIds: new Set((beads?.beads || []).map((b) => b.id)), + }; + const parts = []; parts.push(renderHeader(meta)); parts.push(renderNav()); @@ -111,11 +120,11 @@ export function render(evidence, options = {}) { parts.push(discussions ? renderContext(discussions) : notProvided("context", "Discovered Context")); parts.push(renderClusters(clusters, checks && checks.clusters, evidence.architecture)); parts.push(renderCommunitySection(checks && checks.communities, communityAssessment, communityNames)); - parts.push(guidedWalk && guidedWalk.length > 0 ? renderGuidedWalk(guidedWalk, { pr: meta.pr, headSha: meta.headSha, codeIndex: evidence.codeIndex }, warnings) : notProvided("guided-walk", "Guided Walk")); + parts.push(guidedWalk && guidedWalk.length > 0 ? renderGuidedWalk(guidedWalk, refCtx, warnings) : notProvided("guided-walk", "Guided Walk")); parts.push(functionalTest ? renderFunctionalTest(functionalTest) : notProvided("functional-test", "Functional Test")); - parts.push(findings && findings.length > 0 ? renderFindings(findings) : notProvided("findings", "Findings")); - parts.push(openQuestions && openQuestions.length > 0 ? renderOpenQuestions(openQuestions) : notProvided("open-questions", "Open Questions")); - parts.push(renderAppendixStructural(checks, graphStats, communityNames)); + parts.push(findings && findings.length > 0 ? renderFindings(findings, refCtx, warnings) : notProvided("findings", "Findings")); + parts.push(openQuestions && openQuestions.length > 0 ? renderOpenQuestions(openQuestions, refCtx, warnings) : notProvided("open-questions", "Open Questions")); + parts.push(renderAppendixStructural(checks, graphStats, communityNames, beads)); parts.push(appendixFunctional ? renderAppendixFunctional(appendixFunctional) : notProvided("appendix-functional", "Appendix: Functional Test Details")); parts.push(`<footer>Graph Review — Apache TinkerPop | PR #${esc(String(meta.pr))} | Generated ${esc(meta.timestamp)}</footer>`); @@ -193,9 +202,163 @@ function renderContext(disc) { cards.push(`<div class="card">\n <h3>Proposals</h3>\n <p>No matching proposal in <code>docs/src/dev/future/</code>.</p>\n </div>`); } + if (disc.beads) cards.push(renderProjectMemory(disc.beads)); + return `<section id="context">\n <h2>Discovered Context</h2>\n ${cards.join("\n ")}\n</section>`; } +// ---- Project Memory (beads) ------------------------------------------------- + +const ROLE_TEXT = { + primary: "This PR's work", + owner: "Owns a linked record", + shared: "Connected work", +}; + +// Where an external ref points: a PR, a JIRA, a commit, a URL; null for a path. +function externalRefUrl(ref) { + const pr = ref.match(/^apache\/tinkerpop#(\d+)$/); + if (pr) return `https://github.com/apache/tinkerpop/pull/${pr[1]}`; + if (/^TINKERPOP-\d+$/.test(ref)) return `https://issues.apache.org/jira/browse/${ref}`; + const sha = ref.match(/^apache\/tinkerpop@([0-9a-f]+)$/); + if (sha) return `https://github.com/apache/tinkerpop/commit/${sha[1]}`; + return /^https?:\/\//.test(ref) ? ref : null; +} + +function externalRefHtml(ref) { + const url = externalRefUrl(ref); + return url ? `<a href="${esc(url)}">${esc(ref)}</a>` : `<code>${esc(ref)}</code>`; +} + +function beadAnchor(b, text) { + return `<span id="bead-${esc(b.id)}"></span>${text}`; +} + +// A short status chip; in_progress is the one that matters at PR time. +function beadStatus(b) { + if (b.status === "in_progress") return `<span class="badge badge-attention">in progress</span>`; + return `<span class="bead-status">${esc(b.status)}</span>`; +} + +function beadText(text) { + return text ? `<div class="bead-design">${esc(text)}</div>` : ""; +} + +function renderDecisions(decisions, beadsById) { + if (decisions.length === 0) return ""; + const supersededBy = new Map(); + for (const d of decisions) { + for (const e of d.edges || []) if (e.type === "supersedes") supersededBy.set(e.to, d.id); + } + // A chosen decision and its rejected alternatives are joined by `related`, in either direction. + const rejectedOf = new Map(); + const paired = new Set(); + for (const d of decisions) { + for (const e of d.edges || []) { + if (e.type !== "related") continue; + const other = beadsById.get(e.to); + if (!other || other.type !== "decision") continue; + const [chosen, alt] = d.rejected === false && other.rejected === true ? [d, other] + : d.rejected === true && other.rejected === false ? [other, d] : [null, null]; + if (!chosen) continue; + if (!rejectedOf.has(chosen.id)) rejectedOf.set(chosen.id, []); + rejectedOf.get(chosen.id).push(alt); + paired.add(alt.id); + } + } + const superseded = (d) => supersededBy.has(d.id) + ? ` <span class="bead-superseded">superseded by <a href="#bead-${esc(supersededBy.get(d.id))}">${esc(supersededBy.get(d.id))}</a></span>` : ""; + + const items = decisions.filter((d) => d.rejected === false).map((d) => { + const alts = (rejectedOf.get(d.id) || []).map((a) => + `<li>${beadAnchor(a, `<span class="bead-rejected">Instead of</span> ${esc(a.title)} <code>${esc(a.id)}</code>${superseded(a)}`)}${beadText(a.design)}</li>`).join(""); + return `<li>${beadAnchor(d, `<strong>Chose</strong> ${esc(d.title)} <code>${esc(d.id)}</code>${superseded(d)}`)}${beadText(d.design)}${alts ? `<ul class="bead-alternatives">${alts}</ul>` : ""}</li>`; + }); + const unpaired = decisions.filter((d) => d.rejected === true && !paired.has(d.id)).map((d) => + `<li>${beadAnchor(d, `<span class="bead-rejected">Not taken</span> ${esc(d.title)} <code>${esc(d.id)}</code>${superseded(d)}`)}${beadText(d.design)}</li>`); + const chosen = decisions.filter((d) => d.rejected === false).length; + return `<details class="bead-decisions"> + <summary>Decisions: ${chosen} chosen, ${decisions.length - chosen} not taken</summary> + <ul class="bead-list">${[...items, ...unpaired].join("")}</ul> + </details>`; +} + +function renderBeadRoot(root, members, comments, beadsById, role, via) { + const records = members.filter((b) => b.type === "record" && b.externalRef); + const decisions = members.filter((b) => b.type === "decision"); + const work = members.filter((b) => b.id !== root.id && !["record", "decision"].includes(b.type)); + const active = work.filter((b) => b.status === "in_progress"); + const waiting = work.filter((b) => b.status === "open"); + const finished = work.filter((b) => b.status !== "in_progress" && b.status !== "open"); + const human = members.filter((b) => (b.labels || []).includes("human") && !["closed", "pinned"].includes(b.status)); + + const recordList = records.length > 0 + ? `<p><strong>Records:</strong> ${records.map((r) => beadAnchor(r, `${externalRefHtml(r.externalRef)}${via.includes(r.id) ? " <em>(matched)</em>" : ""}`)).join(", ")}</p>` : ""; + const commentList = (comments[root.id] || []).length > 0 + ? `<details><summary>Root comments (${comments[root.id].length})</summary><ul class="bead-list">${comments[root.id].map((c) => `<li>${beadText(c.text)}</li>`).join("")}</ul></details>` : ""; + const humanList = human.length > 0 + ? `<p><strong>Unanswered questions:</strong></p><ul class="bead-list">${human.map((b) => `<li>${beadAnchor(b, `${esc(b.title)} <code>${esc(b.id)}</code>`)}</li>`).join("")}</ul>` : ""; + const workItem = (b) => `<li>${beadAnchor(b, `${beadStatus(b)} ${esc(b.title)} <code>${esc(b.id)}</code>`)}</li>`; + const plan = work.length > 0 + ? `<p><strong>Plan:</strong> ${active.length} in progress, ${waiting.length} open, ${finished.length} done.</p>${active.length + waiting.length > 0 ? `<ul class="bead-list">${[...active, ...waiting].map(workItem).join("")}</ul>` : ""}${finished.length > 0 ? `<details><summary>Done (${finished.length})</summary><ul class="bead-list">${finished.map(workItem).join("")}</ul></details>` : ""}` : ""; + + return `<div class="card card-context"> + <h3>${beadAnchor(root, `<span class="badge badge-context">Beads</span> <span class="badge bead-role-${esc(role)}">${esc(ROLE_TEXT[role] || role)}</span> ${esc(root.title)} <code>${esc(root.id)}</code>`)}</h3> + <p class="discovery-meta" style="font-style: normal;">${esc(root.type)} · ${esc(root.status)}${(root.labels || []).length ? ` · ${root.labels.map((l) => `<code>${esc(l)}</code>`).join(" ")}` : ""}</p> + ${recordList} + ${humanList} + ${plan} + ${renderDecisions(decisions, beadsById)} + ${commentList} + </div>`; +} + +/** + * The Project Memory part of Discovered Context: the beads recorded for this + * change, one card per root (the PR's own root first), or a note that none were + * found. Says so when bd dolt pull failed and local state was used instead. + */ +function renderProjectMemory(beads) { + const syncNote = beads.synced === false + ? `<p class="discovery-meta">The latest beads data wasn't available (<code>bd dolt pull</code> failed${beads.syncError ? `: ${esc(beads.syncError)}` : ""}); this shows the local beads database as it was.</p>` : ""; + const roots = beads.roots || []; + if (roots.length === 0) { + const seeds = (beads.seeds || []).map((s) => `<code>${esc(s)}</code>`).join(", "); + return `<div class="card"> + <h3>Project Memory</h3> + <p>The beads data wasn't available for this change: no bead record names this PR or its discussions.</p> + ${seeds ? `<p class="discovery-meta">Searched record beads for: ${seeds}</p>` : ""}${syncNote} + </div>`; + } + + const all = beads.beads || []; + const beadsById = new Map(all.map((b) => [b.id, b])); + const intro = `<h3>Project Memory</h3> + <p class="section-intro">Beads recorded for this change, found through record beads that name the PR or its discussions. Each root is the plan and the decisions behind a body of work: the PR's own root first, then connected work. An open root is expected before merge.</p>${syncNote}`; + const cards = roots.map((r) => { + const root = beadsById.get(r.id); + if (!root) return ""; + return renderBeadRoot(root, all.filter((b) => b.root === r.id), beads.comments || {}, beadsById, r.role, r.via || []); + }); + return [intro, ...cards].join("\n "); +} + +function renderBeadsAppendix(beads) { + const all = beads?.beads || []; + if (all.length === 0) return ""; + const rows = all.map((b) => { + const decision = b.rejected === true ? "not taken" : b.rejected === false ? "chosen" : ""; + return `<tr><td class="fn-name"><a href="#bead-${esc(b.id)}">${esc(b.id)}</a></td><td>${esc(b.type)}${decision ? ` (${decision})` : ""}</td><td>${esc(b.status)}</td><td><code>${esc(b.root)}</code></td><td>${esc(b.title)}</td></tr>`; + }).join("\n "); + return ` + <h3>Beads</h3> + <p class="section-intro">Every bead loaded from the roots in <a href="#context">Project Memory</a>. Each is a <code>Bead</code> vertex in the graph; query a root's subtree with <code>.has("root", id)</code>.</p> + <table class="gap-table"> + <thead><tr><th>Bead</th><th>Type</th><th>Status</th><th>Root</th><th>Title</th></tr></thead> + <tbody>\n ${rows}\n </tbody> + </table>`; +} + function generateClusterSvg(clusterData, architecture) { if (!clusterData || !clusterData.clusters || clusterData.clusters.length === 0) return ""; @@ -420,6 +583,14 @@ function generateCommunitySvg(communityData, names) { function renderCodeRefs(refs, ctx, warnings, where, cls = "code-refs") { if (!Array.isArray(refs) || refs.length === 0) return ""; const chips = refs.map((ref) => { + if (ref.bead) { + const label = ref.label || ref.bead; + if (!ctx.beadIds || !ctx.beadIds.has(ref.bead)) { + warnings.push(`${where}: unresolved: no bead "${ref.bead}" in discussions.beads (${JSON.stringify(ref)})`); + return `<span class="code-ref code-ref-unresolved" title="unresolved: no bead ${esc(ref.bead)}">${esc(label)}</span>`; + } + return `<a class="code-ref bead-ref" href="#bead-${esc(ref.bead)}" title="bead ${esc(ref.bead)} in Project Memory">${esc(label)}</a>`; + } const r = resolveRef(ref, ctx); if (!r.url) { warnings.push(`${where}: ${r.title} (${JSON.stringify(ref)})`); @@ -484,21 +655,22 @@ function renderFunctionalTest(ft) { </section>`; } -function renderFindings(findings) { +function renderFindings(findings, ctx, warnings) { const cards = findings.map((f, i) => { const snippet = f.snippet ? `\n <pre><code>${esc(f.snippet)}</code></pre>` : ""; return `<div class="card finding"> <p class="finding-title">${i + 1}. ${esc(f.title)}</p>${snippet} - ${f.body} + ${f.body}${renderCodeRefs(f.refs, ctx, warnings, `findings[${i}].refs`)} </div>`; }).join("\n "); return `<section id="findings">\n <h2>Findings</h2>\n ${cards}\n</section>`; } -function renderOpenQuestions(questions) { - const cards = questions.map(q => { +function renderOpenQuestions(questions, ctx, warnings) { + const cards = questions.map((q, i) => { const meta = q.meta ? `\n <p class="discovery-meta">${esc(q.meta)}</p>` : ""; - return `<div class="card">\n <h3>${esc(q.title)}</h3>\n ${q.body}${meta}\n </div>`; + const refs = renderCodeRefs(q.refs, ctx, warnings, `openQuestions[${i}].refs`); + return `<div class="card">\n <h3>${esc(q.title)}</h3>\n ${q.body}${refs}${meta}\n </div>`; }).join("\n "); return `<section id="open-questions">\n <h2>Open Questions</h2>\n ${cards}\n</section>`; } @@ -544,7 +716,7 @@ function renderConfidence(confidence) { `; } -function renderAppendixStructural(checks, graphStats, communityNames) { +function renderAppendixStructural(checks, graphStats, communityNames, beads) { const hotspots = checks?.centrality?.hotspots || []; const blast = checks?.blastRadius?.functions || []; const stats = graphStats || {}; @@ -609,6 +781,7 @@ function renderAppendixStructural(checks, graphStats, communityNames) { </table> ${hierarchyHtml} ${communitiesHtml} +${renderBeadsAppendix(beads)} ${confidenceHtml} <h3>Graph Statistics</h3> <div class="stats-grid"> diff --git a/.skills/tinker-review/scripts/renderer/template.html b/.skills/tinker-review/scripts/renderer/template.html index 0e2a6da0e6..8cdfe37c75 100644 --- a/.skills/tinker-review/scripts/renderer/template.html +++ b/.skills/tinker-review/scripts/renderer/template.html @@ -57,6 +57,14 @@ pre code { background: none; padding: 0; } .code-ref { display: inline-block; padding: 0.1rem 0.5rem; border: 1px solid var(--primary); border-radius: 3px; font-family: ui-monospace, SFMono-Regular, Menlo, monospace; font-size: 0.75rem; color: var(--primary); text-decoration: none; } .code-ref::after { content: " \2197"; } .code-ref:hover { background: var(--primary); color: #fff; } .code-ref-unresolved { border-color: var(--border); color: var(--text-muted); } .code-ref-unresolved::after { content: " ?"; } +.bead-ref { border-color: #6f42c1; color: #6f42c1; } .bead-ref::after { content: " \2193"; } .bead-ref:hover { background: #6f42c1; color: #fff; } +.bead-list { margin: 0.4rem 0 0.4rem 1.2rem; padding: 0; font-size: 0.875rem; } .bead-list li { margin-bottom: 0.45rem; } +.bead-alternatives { margin: 0.3rem 0 0 1.2rem; padding: 0; list-style: none; border-left: 2px solid var(--border); padding-left: 0.6rem; } +.bead-design { font-size: 0.8rem; color: var(--text-muted); white-space: pre-wrap; margin-top: 0.15rem; } +.bead-rejected { color: var(--danger); font-weight: 600; } .bead-superseded { font-size: 0.75rem; color: var(--text-muted); font-style: italic; } +.bead-status { font-size: 0.7rem; text-transform: uppercase; color: var(--text-muted); } +.bead-decisions summary, .card details summary { cursor: pointer; font-weight: 600; font-size: 0.9rem; margin-top: 0.4rem; } +.bead-role-primary { background: var(--primary); color: #fff; } .bead-role-owner { background: #adb5bd; color: #000; } .bead-role-shared { background: var(--warning); color: #000; } .finding { border-left: 3px solid var(--warning); } .finding-title { font-weight: 600; margin-bottom: 0.25rem; } .test-pass { color: var(--success); font-weight: 600; } .test-fail { color: var(--danger); font-weight: 600; } .test-grid { display: grid; grid-template-columns: auto 1fr auto; gap: 0.25rem 0.75rem; font-size: 0.875rem; align-items: baseline; } diff --git a/.skills/tinker-review/scripts/review.js b/.skills/tinker-review/scripts/review.js index 4ac6f24c4c..fc56951997 100644 --- a/.skills/tinker-review/scripts/review.js +++ b/.skills/tinker-review/scripts/review.js @@ -42,6 +42,8 @@ import { orphans } from "./patterns/orphans.js"; import { buildCodeIndex } from "./extraction/code-index.js"; import { createPrDiscussion } from "./enrichment/api.js"; import { discoverDiscussions } from "./discovery/discussions.js"; +import { requireBeads, syncBeads, bdRunner, beadSeeds, discoverBeads } from "./discovery/beads.js"; +import { populateBeads } from "./graph/populate-beads.js"; const exec = promisify(execFile); @@ -278,6 +280,13 @@ async function detectBaseBranch(pr) { export async function setup(params) { const { pr, repoPath, options = {} } = params; + + // Beads is required: fail before fetching anything. Then pull the latest state; + // a failed pull leaves the local database as it was, and the report says so. + await requireBeads(repoPath); + const beadsSync = await syncBeads(repoPath); + log(beadsSync.synced ? "Beads synced (bd dolt pull)" : `Beads sync failed — using local state: ${beadsSync.syncError}`); + const remote = options.remote || await detectRemote(repoPath); const baseBranch = options.baseBranch || await detectBaseBranch(pr); const prBranch = `pr-review/${pr}`; @@ -293,6 +302,12 @@ export async function setup(params) { await exec("git", ["worktree", "add", worktreePath, prBranch], { cwd: repoPath }); const changedFiles = await getChangedFiles(repoPath, prBranch, remote, baseBranch); + // No diff against the base usually means the PR has already merged. Extraction + // treats an empty change list as "parse the whole repo", so stop here instead. + if (changedFiles.length === 0) { + await cleanupWorktree(repoPath, worktreePath, prBranch); + throw new Error(`PR #${pr} has no changes against ${remote}/${baseBranch}; it has probably already been merged. tinker-review reviews open PRs.`); + } const languages = detectLanguages(changedFiles); const language = languages[0]; const domains = classifyDomains(changedFiles); @@ -330,6 +345,7 @@ export async function setup(params) { language, languages, domains, + beadsSync, handle, connection, aConnection, @@ -406,6 +422,13 @@ export async function phase1(session) { log(`Loading discussions into graph...`); await populateDiscussions(g, discussions, { pr, prTitle: prTitle.trim(), changedFiles }); + log(`Discovering beads...`); + const prRef = `apache/tinkerpop#${pr}`; + const beads = await discoverBeads({ seeds: beadSeeds(pr, discussions), prRef, run: bdRunner(repoPath) }); + discussions.beads = { ...(session.beadsSync || { synced: false, syncError: null }), ...beads }; + log(` beads: ${beads.matches.length} records matched, ${beads.roots.length} roots (${beads.roots.map((r) => `${r.id}:${r.role}`).join(", ") || "none"}), ${beads.beads.length} beads`); + await populateBeads(g, beads); + log(`Classifying external callees...`); const externalsResult = await classifyExternals(g, worktreePath); log(` externals: ${externalsResult.library.length} library / ${externalsResult.project.length} project / ${externalsResult.unresolved.length} unresolved`); @@ -449,6 +472,7 @@ export async function phase1(session) { const evidence = { meta: { pr, + baseBranch, headSha: headShaOut.trim() || null, title: prTitle.trim(), domains, @@ -548,7 +572,10 @@ if (process.argv[1] && basename(process.argv[1]) === "review.js") { } const repoPath = process.argv[3] || process.cwd(); - const session = await setup({ pr, repoPath }); + const session = await setup({ pr, repoPath }).catch((err) => { + console.error(err.message); + process.exit(1); + }); try { const { jsonPath } = await phase1(session); log(`Server running on port ${session.handle.port} — ready for enrichment.`); diff --git a/.skills/tinker-review/test/beads.test.js b/.skills/tinker-review/test/beads.test.js new file mode 100644 index 0000000000..7e0dbae873 --- /dev/null +++ b/.skills/tinker-review/test/beads.test.js @@ -0,0 +1,281 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Beads discovery against a fake bd — record matching, root roles, subtree +// loading, the beads gate and the sync fallback — and the Project Memory +// section the renderer builds from it. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + BEADS_REQUIRED_MESSAGE, beadSeeds, discoverBeads, requireBeads, syncBeads, +} from "../scripts/discovery/beads.js"; +import { render } from "../scripts/renderer/render.js"; + +// A small beads database shaped like PRIME.md section 6. Each entry is the row +// bd list --json returns; `deps` are outgoing dependencies as [target, type]. +function bead(id, type, { parent = null, ref = null, rejected, deps = [], comments = 0, title = id } = {}) { + return { + id, issue_type: type, status: type === "decision" ? "closed" : "open", title, + description: "", design: `design of ${id}`, labels: ["gremlin-core"], parent, + external_ref: ref, closed_at: null, comment_count: comments, + metadata: rejected === undefined ? null : { rejected }, + dependencies: [ + ...(parent ? [{ depends_on_id: parent, type: "parent-child" }] : []), + ...deps.map(([to, t]) => ({ depends_on_id: to, type: t })), + ], + }; +} + +const DB = [ + bead("tp-a", "feature", { comments: 2 }), + bead("tp-a.1", "record", { parent: "tp-a", ref: "TINKERPOP-1234", title: "PR and JIRA" }), + bead("tp-a.2", "record", { parent: "tp-a", ref: "https://lists.apache.org/thread/abc";, title: "dev@ thread: topic" }), + bead("tp-a.3", "decision", { parent: "tp-a", rejected: false, deps: [["tp-a.4", "related"]], comments: 1 }), + bead("tp-a.4", "decision", { parent: "tp-a", rejected: true }), + bead("tp-a.5", "task", { parent: "tp-a", deps: [["tp-a.3", "caused-by"]] }), + bead("tp-a.5.1", "task", { parent: "tp-a.5" }), + // A second root that shares tp-a's JIRA record instead of duplicating it. + bead("tp-b", "epic", { deps: [["tp-a.1", "related"]] }), + bead("tp-b.1", "record", { parent: "tp-b", ref: "apache/tinkerpop#99" }), + bead("tp-b.2", "decision", { parent: "tp-b", rejected: false }), + // Unrelated work that relates to a bead inside tp-a: reachable by bd dep tree + // from tp-a, but not part of tp-a's subtree. + bead("tp-c", "task", { deps: [["tp-a.3", "related"]] }), + // Substring trap for --external-contains. + bead("tp-d", "feature"), + bead("tp-d.1", "record", { parent: "tp-d", ref: "TINKERPOP-12345" }), +]; + +const COMMENTS = { + "tp-a": [{ author: "a", text: "a fact", created_at: "2026-01-01" }, { author: "b", text: "another", created_at: "2026-01-02" }], + "tp-a.3": [{ author: "a", text: "later context", created_at: "2026-01-03" }], +}; + +const byId = new Map(DB.map((b) => [b.id, b])); +const dependentsOf = (id) => DB.filter((b) => b.dependencies.some((d) => d.depends_on_id === id)); + +// Emulates the bd commands discovery uses, including the quirks it relies on: +// search's positional query is a title/ID match, and --external-contains is a +// case-insensitive substring match. +function fakeBd(calls = []) { + return async (args) => { + calls.push(args); + const opt = (name) => args[args.indexOf(name) + 1]; + switch (args[0]) { + case "list": + if (args.includes("--id")) return opt("--id").split(",").map((id) => byId.get(id)).filter(Boolean); + return DB.slice(0, Number(opt("-n")) || DB.length); + case "search": { + const q = args[1].toLowerCase(); + const ext = opt("--external-contains").toLowerCase(); + return DB.filter((b) => b.issue_type === opt("--type") + && (b.id.toLowerCase().startsWith(q) || b.title.toLowerCase().includes(q)) + && (b.external_ref || "").toLowerCase().includes(ext)); + } + case "dep": + if (args[1] === "list") { + return dependentsOf(args[2]).filter((b) => b.dependencies.some((d) => d.depends_on_id === args[2] && d.type === opt("-t"))); + } + if (args[1] === "tree") { + const seen = new Set([args[2]]); + const out = [byId.get(args[2])]; + for (let i = 0; i < out.length; i++) { + for (const d of dependentsOf(out[i].id)) if (!seen.has(d.id)) { seen.add(d.id); out.push(d); } + } + return out.map(({ id, issue_type, title }) => ({ id, issue_type, title })); + } + break; + case "comments": + return COMMENTS[args[1]] || []; + } + throw new Error(`fake bd: unhandled ${args.join(" ")}`); + }; +} + +test("seeds are the PR plus only the artifacts the PR names explicitly", () => { + const seeds = beadSeeds(42, { + jiras: [{ id: "TINKERPOP-1234" }], + devList: [ + { url: "https://lists.apache.org/thread/abc).", found_in: "pr" }, + { url: "https://lists.apache.org/thread/searched";, found_in: "search" }, + ], + proposals: [ + { path: "docs/src/dev/future/proposal-x.asciidoc", matchedIn: "reference" }, + { path: "docs/src/dev/future/proposal-y.asciidoc", matchedIn: "title" }, + ], + }); + assert.deepEqual(seeds, [ + "apache/tinkerpop#42", + "TINKERPOP-1234", + "https://lists.apache.org/thread/abc";, + "docs/src/dev/future/proposal-x.asciidoc", + ]); +}); + +test("a record matches only on an exact external ref", async () => { + const r = await discoverBeads({ seeds: ["TINKERPOP-1234"], prRef: "apache/tinkerpop#1", run: fakeBd() }); + assert.deepEqual(r.matches, [{ seed: "TINKERPOP-1234", record: "tp-a.1" }], "TINKERPOP-12345 is not a match"); +}); + +test("a record whose title does not repeat its ref is still found", async () => { + const r = await discoverBeads({ seeds: ["https://lists.apache.org/thread/abc";], run: fakeBd() }); + assert.deepEqual(r.matches.map((m) => m.record), ["tp-a.2"]); +}); + +test("the owning root and every sharing root are loaded; the PR's own root is primary", async () => { + const r = await discoverBeads({ + seeds: ["apache/tinkerpop#99", "TINKERPOP-1234"], prRef: "apache/tinkerpop#99", run: fakeBd(), + }); + assert.deepEqual(r.roots, [ + { id: "tp-b", role: "primary", via: ["tp-b.1", "tp-a.1"] }, + { id: "tp-a", role: "owner", via: ["tp-a.1"] }, + ]); +}); + +test("a shared record leads to the sharing root as shared", async () => { + const r = await discoverBeads({ seeds: ["TINKERPOP-1234"], prRef: "apache/tinkerpop#7", run: fakeBd() }); + assert.deepEqual(r.roots.map(({ id, role }) => ({ id, role })), [ + { id: "tp-a", role: "owner" }, + { id: "tp-b", role: "shared" }, + ]); +}); + +test("the subtree includes grandchildren and excludes beads that only relate to it", async () => { + const r = await discoverBeads({ seeds: ["https://lists.apache.org/thread/abc";], run: fakeBd() }); + const ids = r.beads.map((b) => b.id).sort(); + assert.deepEqual(ids, ["tp-a", "tp-a.1", "tp-a.2", "tp-a.3", "tp-a.4", "tp-a.5", "tp-a.5.1"]); + assert.ok(r.beads.every((b) => b.root === "tp-a")); +}); + +test("beads carry the rejected flag, external ref and non-tree edges", async () => { + const r = await discoverBeads({ seeds: ["https://lists.apache.org/thread/abc";], run: fakeBd() }); + const get = (id) => r.beads.find((b) => b.id === id); + assert.equal(get("tp-a.3").rejected, false); + assert.equal(get("tp-a.4").rejected, true); + assert.equal(get("tp-a.5").rejected, null, "only decisions have a rejected flag"); + assert.equal(get("tp-a.1").externalRef, "TINKERPOP-1234"); + assert.deepEqual(get("tp-a.3").edges, [{ to: "tp-a.4", type: "related" }]); + assert.deepEqual(get("tp-a.5").edges, [{ to: "tp-a.3", type: "caused-by" }]); + assert.equal(get("tp-a.5.1").parent, "tp-a.5"); +}); + +test("comments are read only for roots and decisions that have them", async () => { + const calls = []; + const r = await discoverBeads({ seeds: ["https://lists.apache.org/thread/abc";], run: fakeBd(calls) }); + assert.deepEqual(Object.keys(r.comments).sort(), ["tp-a", "tp-a.3"]); + assert.equal(r.comments["tp-a.3"][0].text, "later context"); + assert.deepEqual(calls.filter((c) => c[0] === "comments").map((c) => c[1]).sort(), ["tp-a", "tp-a.3"]); +}); + +test("no matching record means no roots and no beads", async () => { + const r = await discoverBeads({ seeds: ["apache/tinkerpop#5", "TINKERPOP-9"], run: fakeBd() }); + assert.deepEqual([r.matches, r.roots, r.beads], [[], [], []]); +}); + +test("requireBeads fails early with the install message when bd is missing", async () => { + const missing = async () => { throw Object.assign(new Error("spawn bd ENOENT"), { code: "ENOENT" }); }; + await assert.rejects(requireBeads("/repo", missing), (err) => { + assert.ok(err.message.startsWith(BEADS_REQUIRED_MESSAGE)); + assert.match(err.message, /not found on the PATH/); + return true; + }); +}); + +test("requireBeads passes when bd answers", async () => { + await requireBeads("/repo", fakeBd()); +}); + +test("a failed pull falls back to local state instead of failing", async () => { + const offline = async () => { throw Object.assign(new Error("pull failed"), { stderr: "remote unreachable" }); }; + assert.deepEqual(await syncBeads("/repo", offline), { synced: false, syncError: "remote unreachable" }); + assert.deepEqual(await syncBeads("/repo", async () => {}), { synced: true, syncError: null }); +}); + +// ---- Project Memory rendering ---- + + +const meta = { pr: 99, headSha: "abc", title: "t", domains: [], timestamp: "now" }; + +function shaped(id, type, extra = {}) { + return { + id, type, status: "closed", title: `title ${id}`, description: "", design: `why ${id}`, + rejected: type === "decision" ? false : null, labels: [], parent: null, externalRef: null, + closedAt: null, root: "tp-a", edges: [], ...extra, + }; +} + +const found = { + synced: true, syncError: null, + seeds: ["apache/tinkerpop#99"], + matches: [{ seed: "apache/tinkerpop#99", record: "tp-a.1" }], + roots: [{ id: "tp-a", role: "primary", via: ["tp-a.1"] }], + beads: [ + shaped("tp-a", "feature", { status: "open", labels: ["gremlin-core", "3.7.7"] }), + shaped("tp-a.1", "record", { parent: "tp-a", externalRef: "apache/tinkerpop#99" }), + shaped("tp-a.2", "decision", { parent: "tp-a", edges: [{ to: "tp-a.3", type: "related" }] }), + shaped("tp-a.3", "decision", { parent: "tp-a", rejected: true }), + shaped("tp-a.4", "decision", { parent: "tp-a", edges: [{ to: "tp-a.2", type: "supersedes" }] }), + shaped("tp-a.5", "task", { parent: "tp-a", status: "in_progress" }), + shaped("tp-a.6", "task", { parent: "tp-a", status: "open" }), + shaped("tp-a.7", "task", { parent: "tp-a", status: "open", labels: ["human"] }), + ], + comments: { "tp-a": [{ author: "a", text: "a fact on the root", createdAt: "x" }] }, +}; + +test("Project Memory shows the root, its records, plan and decisions with alternatives nested", () => { + const html = render({ meta, discussions: { beads: found } }); + assert.match(html, /This PR's work/); + assert.match(html, /id="bead-tp-a"/); + assert.match(html, /href="https:\/\/github.com\/apache\/tinkerpop\/pull\/99">apache\/tinkerpop#99<\/a> <em>\(matched\)<\/em>/); + assert.match(html, /Plan:<\/strong> 1 in progress, 2 open, 0 done/); + assert.match(html, /Unanswered questions:[\s\S]*tp-a\.7/); + assert.match(html, /Decisions: 2 chosen, 1 not taken/); + assert.match(html, /Chose<\/strong> title tp-a\.2[\s\S]*superseded by[\s\S]*bead-alternatives[\s\S]*Instead of<\/span> title tp-a\.3/); + assert.match(html, /a fact on the root/); + assert.match(html, /<h3>Beads<\/h3>/, "the appendix lists the loaded beads"); + for (const b of found.beads) { + assert.match(html, new RegExp(`id="bead-${b.id.replace(/\./g, "\\.")}"`), `appendix link target for ${b.id}`); + } +}); + +test("Project Memory says the data wasn't available when nothing matched, and when the sync failed", () => { + const html = render({ + meta, + discussions: { beads: { synced: false, syncError: "remote unreachable", seeds: ["apache/tinkerpop#99"], matches: [], roots: [], beads: [], comments: {} } }, + }); + assert.match(html, /beads data wasn't available for this change/); + assert.match(html, /Searched record beads for: <code>apache\/tinkerpop#99<\/code>/); + assert.match(html, /latest beads data wasn't available \(<code>bd dolt pull<\/code> failed: remote unreachable\)/); + assert.doesNotMatch(html, /<h3>Beads<\/h3>/); +}); + +test("bead refs link into Project Memory and an unknown bead is reported", () => { + const warnings = []; + const html = render({ + meta, + discussions: { beads: found }, + findings: [{ title: "f", body: "<p>b</p>", refs: [{ bead: "tp-a.3", label: "rejected option" }, { bead: "tp-zzz" }] }], + }, { warnings }); + assert.match(html, /<a class="code-ref bead-ref" href="#bead-tp-a\.3"[^>]*>rejected option<\/a>/); + assert.match(html, /code-ref-unresolved[^>]*>tp-zzz</); + assert.deepEqual(warnings.length, 1); + assert.match(warnings[0], /findings\[0\]\.refs: unresolved: no bead "tp-zzz"/); +});
