diff --git a/webviews/src/agent-session/acpmux/chips/BareReferenceChip.tsx b/webviews/src/agent-session/acpmux/chips/BareReferenceChip.tsx index 37ac477a1876..7d203920a08e 100644 --- a/webviews/src/agent-session/acpmux/chips/BareReferenceChip.tsx +++ b/webviews/src/agent-session/acpmux/chips/BareReferenceChip.tsx @@ -16,21 +16,24 @@ import { usePullRequestsExist } from "./referenceExistence"; const address = (repository: string, number: number) => `https://github.com/${repository}/pull/${number}`; const NO_URLS: readonly string[] = []; -/// `href` is the folder's (or the transcript's) address for the number: rule d's. Without one, the -/// number is linked only when the transcript names its repository. +/// `href` is the folder's (or the transcript's) address for the number: rule d's. Without one, or +/// when the number is out of every candidate's range (rule 4), the number is linked only when the +/// transcript names its repository. export function BareReferenceChip({ number, href, text }: { number: number; href?: string; text: string }) { const scope = useContext(ReferenceScopeContext); const fallback = href && /^https:\/\/github\.com\/([^/]+\/[^/]+)\//.exec(href)?.[1]; const named = namedRepository(number, scope); - const candidates = !named && fallback ? candidateRepositories(fallback, scope) : []; - // Only a number with more than one candidate is asked about; one candidate is rule d's anyway. + const candidates = !named && fallback ? candidateRepositories(fallback, scope, number) : []; + // Rule d stands for the folder's repository alone, so one candidate is asked about only when the + // folder's is out of range (rule 4) and the cache must say the chat's repository has N. + const folderFirst = Boolean(fallback) && candidates[0] === fallback; const existing = usePullRequestsExist( - candidates.length > 1 ? candidates.map((repository) => address(repository, number)) : NO_URLS, + candidates.length > (folderFirst ? 1 : 0) ? candidates.map((repository) => address(repository, number)) : NO_URLS, ); const choice = named ? { repository: named, alternatives: [] } : candidates.length - ? chooseRepository(candidates, existing) + ? chooseRepository(candidates, existing, fallback || undefined) : undefined; const ambiguous = Boolean(choice?.alternatives.length); if (!choice) return text; diff --git a/webviews/src/agent-session/acpmux/conversation/Markdown.tsx b/webviews/src/agent-session/acpmux/conversation/Markdown.tsx index 1068ff110f65..be12d380055d 100644 --- a/webviews/src/agent-session/acpmux/conversation/Markdown.tsx +++ b/webviews/src/agent-session/acpmux/conversation/Markdown.tsx @@ -313,6 +313,10 @@ export type InlineOptions = { githubRepository?: string; /** Link GitHub references in prose; labels of an outer link disable this. */ linkGithubReferences?: boolean; + /** Link a bare `#N` too (default). Off in a quote: quoted words are someone else's, whose `#1` + * is rarely this repository's pull request (Leo 2026-10-29: "these auto render to the pr + * instead of just #1 #2..."). `owner/repo#N` and addresses still link there. */ + linkBareReferences?: boolean; /** The chats the open chat started, whose names draw as chips (started/startedSessions.ts). */ started?: StartedMentions; /** How a commit SHA draws: a GitHub chip (replies), or quiet monospace text that links to its @@ -432,6 +436,7 @@ function linkedGithubText( repository?: string, enabled = true, quietCommits = false, + bare = true, ): ReactNode[] { if (!enabled) return linkedText(text, key); // A pull request address, so the chip opens the pull request card (an issue redirects there); @@ -445,7 +450,7 @@ function linkedGithubText( const spans = commits.length || issues.some((issue) => issue.bare) ? textLinks(text) : []; const outside = (ref: { start: number; end: number }) => !spans.some((span) => ref.start < span.end && span.start < ref.end); - const refs = [...issues.filter((issue) => !issue.bare || outside(issue)), ...commits.filter(outside)] + const refs = [...issues.filter((issue) => !issue.bare || (bare && outside(issue))), ...commits.filter(outside)] .sort((a, b) => a.start - b.start) .filter((ref, index, all) => index === 0 || ref.start >= all[index - 1]!.end); if (!refs.length) return linkedText(text, key); @@ -525,6 +530,7 @@ function proseText(text: string, key: string, opts: InlineOptions): ReactNode[] opts.githubRepository, opts.linkGithubReferences !== false, opts.commits === "quiet", + opts.linkBareReferences !== false, ), current, ) @@ -826,7 +832,7 @@ function Block({ case "blockquote": return (
- +
); case "list": { @@ -894,6 +900,15 @@ function ItemThumbs({ text, opts, streaming }: { text: string; opts: InlineOptio /// Nested blocks (a quote's, a list item's). `plain` keeps the outer Markdown's plain fences /// (`plainCode`) inside them too: no diagram or render card at any depth. +/// A quote's options: the same object for the same `opts`, so its memoized blocks stay drawn. +const quotedOptions = new WeakMap(); +function quoted(opts: InlineOptions): InlineOptions { + if (opts.linkBareReferences === false) return opts; + let kept = quotedOptions.get(opts); + if (!kept) quotedOptions.set(opts, (kept = { ...opts, linkBareReferences: false })); + return kept; +} + function Blocks({ blocks, opts, diff --git a/webviews/src/agent-session/acpmux/conversation/bareReferences.ts b/webviews/src/agent-session/acpmux/conversation/bareReferences.ts index 659ca11e8287..740c5dab4377 100644 --- a/webviews/src/agent-session/acpmux/conversation/bareReferences.ts +++ b/webviews/src/agent-session/acpmux/conversation/bareReferences.ts @@ -14,6 +14,9 @@ // d. else the folder's repository (or the transcript's, transcriptRepositories.ts). // 3. A number that is a pull request in two or more candidates names its repository on the chip // (`leo-workspace#843`). +// 4. Rules c and d only take a candidate whose numbers the chat named are no longer than N: a cmux +// chat that named #27262 means no pull request by "#1 / #2" (Leo 2026-10-29: "these auto +// render to the pr instead of just #1 #2..."). With no candidate left, N is text. import { createContext } from "react"; import { githubReferences, isGithubOwner } from "../../../githubReferences"; import type { AcpmuxRow } from "../model"; @@ -26,6 +29,8 @@ export type ReferenceEvidence = { mentions: ReadonlyMap; /** Rule c's candidates after the folder's: the repositories the chat linked, latest first. */ linked: readonly string[]; + /** Rule 4: the smallest number the chat named in each repository, by its lowercase name. */ + smallest: ReadonlyMap; }; /** A turn's evidence: the transcript's, its own (rule a), and where it starts. */ @@ -88,6 +93,7 @@ function rowNamed(row: Pick): readonly Nam type Reading = { mentions: Map; repos: Map; + smallest: Map; turns: Map }>; turn: string; }; @@ -114,13 +120,25 @@ export const newReferenceScopeCache = (): ReferenceScopeCache => ({ scopes: new Map(), }); -const empty = (): Reading => ({ mentions: new Map(), repos: new Map(), turns: new Map(), turn: "" }); +const empty = (): Reading => ({ + mentions: new Map(), + repos: new Map(), + smallest: new Map(), + turns: new Map(), + turn: "", +}); const copy = (reading: Reading): Reading => { const turns = new Map(reading.turns); const open = turns.get(reading.turn); if (open) turns.set(reading.turn, { at: open.at, named: new Map(open.named) }); - return { mentions: new Map(reading.mentions), repos: new Map(reading.repos), turns, turn: reading.turn }; + return { + mentions: new Map(reading.mentions), + repos: new Map(reading.repos), + smallest: new Map(reading.smallest), + turns, + turn: reading.turn, + }; }; function read(reading: Reading, row: Pick, at: number) { @@ -135,6 +153,7 @@ function read(reading: Reading, row: Pick= (reading.smallest.get(key) ?? Infinity))) reading.smallest.set(key, number); reading.mentions.set(number, [...(reading.mentions.get(number) ?? []), { at, repo: spelled }]); if (!reading.turns.has(reading.turn)) reading.turns.set(reading.turn, { at: 0, named: new Map() }); reading.turns.get(reading.turn)!.named.set(number, spelled); @@ -175,7 +194,7 @@ export function referenceScopes( const evidenceKey = `${cache.generation}:${cache.prefixLength}:${tail}`; if (cache.evidence?.key !== evidenceKey) { const linked = [...whole.repos.values()].map(({ repo }) => repo).reverse(); - cache.evidence = { key: evidenceKey, value: { mentions: whole.mentions, linked } }; + cache.evidence = { key: evidenceKey, value: { mentions: whole.mentions, linked, smallest: whole.smallest } }; cache.scopes.clear(); } const evidence = cache.evidence.value; @@ -217,24 +236,33 @@ export function namedRepository(number: number, scope: ReferenceScope | null): s return before; } -/** Rule c's candidates: `fallback` (rule d's) first, then the chat's repositories, latest first. */ -export function candidateRepositories(fallback: string, scope: ReferenceScope | null): string[] { +/** Rule 4: whether `number` is as long as every number the chat named in `repo`; a repository + * the chat named no number in has no range, so any number is in it. */ +export function inNamedRange(number: number, repo: string, scope: ReferenceScope | null): boolean { + const smallest = scope?.evidence.smallest.get(repo.toLowerCase()); + return smallest === undefined || String(number).length >= String(smallest).length; +} + +/** Rule c's candidates: `fallback` (rule d's) first, then the chat's repositories, latest first, + * each only if `number` is in its range (rule 4). */ +export function candidateRepositories(fallback: string, scope: ReferenceScope | null, number: number): string[] { const out = [fallback]; for (const repo of scope?.evidence.linked ?? []) { if (out.length >= MAX_CANDIDATES) break; if (!out.some((kept) => kept.toLowerCase() === repo.toLowerCase())) out.push(repo); } - return out; + return out.filter((repo) => inNamedRange(number, repo, scope)); } /** Rules c and d: the first candidate with pull request N, and the others that have it too; - * `existing` is undefined (or has none) until the cache answers, and then rule d stands. */ + * `existing` is undefined (or has none) until the cache answers, and then rule d stands when + * `fallback` (the folder's repository) is still a candidate; without it, N is text. */ export function chooseRepository( candidates: readonly string[], existing: readonly boolean[] | undefined, -): { repository: string; alternatives: string[] } { + fallback = candidates[0], +): { repository: string; alternatives: string[] } | undefined { const found = existing ? candidates.filter((_, index) => existing[index]) : []; - return found.length - ? { repository: found[0]!, alternatives: found.slice(1) } - : { repository: candidates[0]!, alternatives: [] }; + if (found.length) return { repository: found[0]!, alternatives: found.slice(1) }; + return fallback !== undefined && candidates[0] === fallback ? { repository: fallback, alternatives: [] } : undefined; } diff --git a/webviews/src/agent-session/acpmux/conversation/transcript.gallery.ts b/webviews/src/agent-session/acpmux/conversation/transcript.gallery.ts index 43ea195b8af6..2691003f7843 100644 --- a/webviews/src/agent-session/acpmux/conversation/transcript.gallery.ts +++ b/webviews/src/agent-session/acpmux/conversation/transcript.gallery.ts @@ -1214,6 +1214,19 @@ export default agentPaneEntry({ ]), play: hoverReference(843, ".cv-link-preview.acpmux-pr-card"), }, + "github-references-quoted-numbers": { + note: 'Leo 2026-10-29: "these auto render to the pr instead of just #1 #2...". The reply quotes his "what is #1 / #2": in the quote and in prose they stay text (the chat named #27262, so a one-digit number is no cmux pull request); #27262 and manaflow-ai/cmux#27262 stay chips.', + ready: { githubRepository: "manaflow-ai/cmux" }, + native: { "git.githubRepository": { repository: "manaflow-ai/cmux" } }, + snapshot: chat([ + user("Is https://github.com/manaflow-ai/cmux/pull/27262 green? Also what is #1 / #2", 3), + assistant( + "> what is #1 / #2\n\nThose are the two options I listed: #1 keeps the dock, #2 drops it. #27262 (manaflow-ai/cmux#27262) is green.", + 2, + ), + summary(2, { status: "completed", durationMs: 9_000 }), + ]), + }, tasks: { note: "Checked, unchecked, nested and wrapping task-list items.", snapshot: chat([user("Review the release checklist", 4), assistant(TASKS_REPLY, 3)]), diff --git a/webviews/test/bare-ref-repo.test.tsx b/webviews/test/bare-ref-repo.test.tsx index 2649005fdffd..151272c69c4d 100644 --- a/webviews/test/bare-ref-repo.test.tsx +++ b/webviews/test/bare-ref-repo.test.tsx @@ -278,3 +278,45 @@ test("a streaming reply reads only its own row again; an older row that changed const again = referenceScopes([prompt, done, assistant("#9")], cache); expect(again(prompt.id).evidence.mentions.get(9)).toEqual([{ at: 1, repo: "x/y" }]); }); + +// Leo 2026-10-29: "these auto render to the pr instead of just #1 #2...". The agent quoted his +// "what is #1 / #2", and each bare number drew as a chip for cmux's (real, unrelated) pull +// requests #1 and #2. +test("Leo's #1 / #2: a quote's bare numbers stay text; explicit references stay chips", async () => { + const view = await render( + [ + user("what is #1 / #2"), + // The turn names cmux#2 itself (rule a), so only the quote keeps the quoted #2 text. + assistant(`> what is #1 / #2\n\nThe options I listed, not pull requests (${CMUX}#2 is unrelated).`), + ], + [pr(CMUX, 1), pr(CMUX, 2)], + { folder: CMUX }, + ); + expect(view.chips()).toEqual([[`${CMUX}#2`, pr(CMUX, 2)]]); + expect(view.element.querySelector(".cv-quote")?.textContent).toContain("what is #1 / #2"); +}); + +test("Leo's #1 / #2: a bare number far below the repository's named numbers stays text", async () => { + const view = await render( + [ + user(`Land ${pr(CMUX, 27262)}, then what is #1 / #2`), + assistant('You asked "what is #1 / #2": #1 is the sidebar fix, #2 the composer one. #27262 is green.'), + ], + [pr(CMUX, 1), pr(CMUX, 2), pr(CMUX, 27262)], + { folder: CMUX }, + ); + expect(view.chips().filter(([label]) => label?.startsWith("#"))).toEqual([["#27262", pr(CMUX, 27262)]]); +}); + +test("rule 4: with the folder's repository out of range, only the cache's answer links a number", async () => { + const view = await render( + [user(`Land ${pr(CMUX, 27262)} and look at ${pr("o/r", 3)}`), assistant("#5 is o/r's; #6 is nobody's.")], + [pr("o/r", 5)], + { folder: CMUX }, + ); + expect(view.chips().filter(([label]) => label?.startsWith("#"))).toEqual([["#5", pr("o/r", 5)]]); + const asked = inspected(view.calls); + expect(asked).toContain(pr("o/r", 6)); + // The folder's repository is out of range for both, so neither is asked about there. + expect(asked.filter((url) => url === pr(CMUX, 5) || url === pr(CMUX, 6))).toEqual([]); +});