From ddc5a47742a969c137ab406808e251e619085732 Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Fri, 31 Jul 2026 21:06:10 +0200 Subject: [PATCH 1/2] feat(coding): quote the body's trigger paragraph in label findings (#331) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a check's edge label is empty or isn't a condition, the text that should be in the label is usually a few lines away in the node body. The finding now quotes it, so whoever fixes it has the source material without opening each node. On hadronmemory.com::hadron-server (hadron-server#845): review:entity-ref-args error label-is-condition edge label is the bare stem "Applies when" with no condition after it — the body states its scope, condense it into the label: "Adding or renaming a GraphQL argument (query OR mutation) that identifies an existing entity, or widening an existing arg to accept a URN. Skip for arguments…" Two body conventions are recognised, measured across all 90 review checks in the four memories that have them: 55 use `> **Scope.** …`, 2 a line-initial `**Applies when** …`, 33 neither. So a trigger is findable for ~63% of checks; the rest gain nothing rather than carrying a "none found" line, which would be noise on a third of every run. Quoted, NOT promoted into the label. Those paragraphs run a median of 238 characters (max 567) against a median healthy edge label of 84 (p90 142), so feeding one to --fix would mint a label 3x too long and, since the edge loc is slugified from the name, an enormous derived loc with it. --fix keeps reading only the description, which must already read as a trigger. The quote is truncated to 160 chars by default; --suggest prints it whole. Presentation only: a test asserts the hint changes neither which rules fire, their severities, nor the --fix plan. Content costs no extra round trip — the shared NodeBatch operation already selects it. Co-Authored-By: Claude Opus 5 --- docs/plans/coding-command-group.md | 24 ++++- internal/cmd/agentic/agentic-usage.md | 2 +- internal/cmd/coding/bodytrigger.go | 120 +++++++++++++++++++++ internal/cmd/coding/bodytrigger_test.go | 135 ++++++++++++++++++++++++ internal/cmd/coding/coding.go | 13 ++- internal/cmd/coding/review_lint.go | 15 ++- internal/cmd/coding/review_lint_test.go | 98 +++++++++++++++++ 7 files changed, 397 insertions(+), 10 deletions(-) create mode 100644 internal/cmd/coding/bodytrigger.go create mode 100644 internal/cmd/coding/bodytrigger_test.go diff --git a/docs/plans/coding-command-group.md b/docs/plans/coding-command-group.md index ea32c15..2f04425 100644 --- a/docs/plans/coding-command-group.md +++ b/docs/plans/coding-command-group.md @@ -200,7 +200,7 @@ subcommands take `-m/--memory` like every other group. | `label-present` | error | the empty label (Decision 2) | | `label-is-condition` | error | `child-of`, `applies-when`, `related`, bare `Applies when` | | `check-node-resolves` | warning | dangling / unreadable check node — the edge's `source` (Decision 3) | -| `description-has-trigger` | warning | second blind spot in `hadron_find_nodes` output | +| `description-present` | warning | second blind spot in `hadron_find_nodes` output (deviation 2) | | `duplicate-trigger` | warning | cloned check never re-pointed | | `seq-unique` | warning | non-deterministic sibling ordering | | `foreign-toolchain` | warning | the misfiled `format-sources` | @@ -391,6 +391,28 @@ possible and the visibility gap is real — but it is now a rule with no known live instance rather than one with a motivating example. Nothing in either memory currently trips it. +### 8. Label findings quote the body's trigger paragraph (#331) + +Added after `hadron coding review lint` found three bare-`Applies when` stems in +`hadronmemory.com::hadron-server` (hadron-server#845) and `--fix` could repair +none of them: the trigger text existed, but in the node body rather than the +description. + +Measuring all 90 checks across the four memories that have them: 55 state their +scope as `> **Scope.** …`, 2 as a line-initial `**Applies when** …`, and 33 +neither. So a body-derived trigger is findable for ~63% of checks. + +**It is quoted, not promoted.** Those paragraphs run a median of 238 characters +(max 567) against a median healthy edge label of 84 (p90 142), so feeding one to +`--fix` would mint a label 3x too long — and an enormous slugified edge loc with +it. `label-present` / `label-is-condition` findings therefore append the +paragraph, truncated to 160 characters, and `--fix` is untouched. `--suggest` +prints it in full. When a body states no scope the finding gains nothing, since +that is a third of checks and a "none found" line on each would be noise. + +Presentation only: a unit test asserts the hint changes neither which rules fire, +their severities, nor the `--fix` plan. + ## Verification (as built) `go test ./...` (16 packages) and `make lint` (0 issues) are green. Read-only diff --git a/internal/cmd/agentic/agentic-usage.md b/internal/cmd/agentic/agentic-usage.md index 13a0b87..598e679 100644 --- a/internal/cmd/agentic/agentic-usage.md +++ b/internal/cmd/agentic/agentic-usage.md @@ -75,7 +75,7 @@ hadron search [-m ]... [--mode hybrid|keyword|vector|regex] [--p hadron replace text --field (--node | -m ) [--prefix ] [--regex] [-i] [--dry-run] [--yes] [--max-nodes N] hadron edge list | add | update | rm hadron spec list [-m ] | get |--prefix | describe | use [] | register [--check] | find [--match-exactly] | grep [--regex] [-i] [--field content|abstract] [--prefix ] | replace [--regex] [--word-boundary=false] [--field content|abstract] [--dry-run] [--yes] [--max-specs N] | new ... | edit | extract --to-feature | link | lint [] | check-tools [--prefix ] | supersede | import spec-kit|code -hadron coding review lint -m [--root ] [--toolchain |-] [--strict] [--fix [--yes]] [--json] | preflight lint -m [--root ] [--strict] [--json] +hadron coding review lint -m [--root ] [--toolchain |-] [--strict] [--suggest] [--fix [--yes]] [--json] | preflight lint -m [--root ] [--strict] [--json] hadron app list --org | install (--org | --owner-me) --agent --name [--type ] [--urn ] [--description ] | uninstall | use hadron ai-config list [--app ] [--agent ] | create (--app|--agent|--org ) --name --provider

--model [--api-key -] [--file ] | update ... | rm hadron org list [--mine] | create --name --urn | get | public | update | rm | member list|add|set-role|rm --user [--role ] | invite create --org --role | invite accept | invite show diff --git a/internal/cmd/coding/bodytrigger.go b/internal/cmd/coding/bodytrigger.go new file mode 100644 index 0000000..b66f919 --- /dev/null +++ b/internal/cmd/coding/bodytrigger.go @@ -0,0 +1,120 @@ +package coding + +import ( + "regexp" + "strconv" + "strings" +) + +// Quoting the body's trigger paragraph in a label finding (#331). +// +// When a check's edge label is empty or isn't a condition, the text that +// *should* be in the label is usually sitting a few lines away in the node +// body. Measured across the 90 review checks in the four memories that have +// them, 57 state their scope in the body under one of two conventions: +// +// > **Scope.** Adding or renaming a GraphQL argument that identifies … +// **Applies when** a parent component reads a child's `bind:`-bound value … +// +// The finding quotes that text so whoever fixes it has the source material +// without opening each node. +// +// It is deliberately NOT fed to --fix. Those paragraphs run a median of 238 +// characters (max 567) against a median healthy edge label of 84, so promoting +// one verbatim would produce a label 3x too long — and, since the edge loc is +// slugified from the name, an enormous derived loc with it. Condensing a scope +// paragraph into a trigger is a judgement call the linter hands over rather +// than makes. +var ( + reScopeMarker = regexp.MustCompile(`(?i)^\s*>?\s*\*\*Scope\.?\*\*\s*(.*)$`) + reAppliesMarker = regexp.MustCompile(`(?i)^\s*\*\*Applies when\*\*\s*(.*)$`) +) + +// triggerQuoteLimit is how much of the paragraph a finding shows by default. +// A little above the p90 healthy label (142 chars), so what's displayed is +// roughly the size of the label being asked for. --suggest prints it whole. +const triggerQuoteLimit = 160 + +// bodyTrigger returns the scope/trigger paragraph a check's body states, and +// whether one was found. The paragraph is flattened to a single line. +// +// Both conventions wrap across lines: the blockquote form continues on +// following `>` lines, the inline form on following non-blank lines. Either +// ends at a blank line. +func bodyTrigger(content string) (string, bool) { + lines := strings.Split(content, "\n") + for i, ln := range lines { + if m := reScopeMarker.FindStringSubmatch(ln); m != nil { + return joinParagraph(m[1], lines[i+1:], true) + } + if m := reAppliesMarker.FindStringSubmatch(ln); m != nil { + // Keep the marker: "Applies when X" already reads as the trigger, + // whereas a Scope paragraph describes it. + text, ok := joinParagraph(m[1], lines[i+1:], false) + if !ok { + return "", false + } + return "Applies when " + text, true + } + } + return "", false +} + +// joinParagraph flattens a marker line's remainder plus its continuation lines +// into one whitespace-normalised string. quoted selects blockquote +// continuation (lines starting `>`) over plain continuation (any non-blank). +func joinParagraph(first string, rest []string, quoted bool) (string, bool) { + parts := []string{first} + for _, ln := range rest { + t := strings.TrimSpace(ln) + if t == "" { + break + } + if quoted { + if !strings.HasPrefix(t, ">") { + break + } + t = strings.TrimSpace(strings.TrimPrefix(t, ">")) + if t == "" { + break + } + } else if strings.HasPrefix(t, "#") || strings.HasPrefix(t, ">") { + break // a new heading or blockquote ends the paragraph + } + parts = append(parts, t) + } + out := strings.Join(strings.Fields(strings.Join(parts, " ")), " ") + if out == "" { + return "", false + } + return out, true +} + +// truncateRunes shortens s to at most n runes, cutting at a word boundary when +// one is near the limit so the quote doesn't end mid-word. +func truncateRunes(s string, n int) string { + r := []rune(s) + if len(r) <= n { + return s + } + cut := string(r[:n]) + if i := strings.LastIndexAny(cut, " \t"); i > n*3/4 { + cut = cut[:i] + } + return strings.TrimRight(cut, " \t,;:.") + "…" +} + +// triggerHint is the sentence appended to a label finding when the body states +// a scope. Returns "" when it doesn't, so a finding gains nothing rather than +// carrying a "couldn't find one" line — a third of checks have no such +// paragraph, and that noise would land on every one of them. +func triggerHint(content string, full bool) string { + text, ok := bodyTrigger(content) + if !ok { + return "" + } + if !full { + text = truncateRunes(text, triggerQuoteLimit) + } + return " — the body states its scope, condense it into the label: " + strconv.Quote(text) +} diff --git a/internal/cmd/coding/bodytrigger_test.go b/internal/cmd/coding/bodytrigger_test.go new file mode 100644 index 0000000..165803e --- /dev/null +++ b/internal/cmd/coding/bodytrigger_test.go @@ -0,0 +1,135 @@ +package coding + +import ( + "strings" + "testing" +) + +// The two conventions, in the shapes they actually appear in: hadron-server / +// hadron-cli / mmdata use the blockquote Scope form, hadron-portal the inline +// bold form. +func TestBodyTrigger(t *testing.T) { + cases := []struct { + name string + content string + want string + }{ + { + "blockquote scope, single line", + "# Review: whatever\n\n> **Scope.** Adding or renaming a GraphQL argument that identifies an entity.\n\nBody follows.", + "Adding or renaming a GraphQL argument that identifies an entity.", + }, + { + "blockquote scope, wrapped", + "> **Scope.** Run this when authorization compares two `organizationId`s,\n> or branches on org membership, for a user-owned entity.\n\nRest.", + "Run this when authorization compares two `organizationId`s, or branches on org membership, for a user-owned entity.", + }, + { + "inline applies-when keeps its marker", + "# Title\n\n**Applies when** a parent component reads a child's bound value.\n\nCheck 1 — ...", + "Applies when a parent component reads a child's bound value.", + }, + { + "inline applies-when, wrapped", + "**Applies when** a route form\nreturns a typed message.\n\nNext para.", + "Applies when a route form returns a typed message.", + }, + { + "scope without the trailing period in the marker", + "> **Scope** Something happens.\n", + "Something happens.", + }, + // A third of checks state no scope at all; those must yield nothing so + // the finding stays quiet rather than carrying a "none found" line. + {"no marker", "# Review: x\n\nJust prose about the rule.\n", ""}, + {"empty", "", ""}, + {"marker with no text", "> **Scope.**\n\nBody.", ""}, + {"marker with only whitespace", "> **Scope.** \n", ""}, + } + for _, tc := range cases { + got, ok := bodyTrigger(tc.content) + if tc.want == "" { + if ok || got != "" { + t.Errorf("%s: expected no trigger, got %q", tc.name, got) + } + continue + } + if !ok { + t.Errorf("%s: expected a trigger, found none", tc.name) + continue + } + if got != tc.want { + t.Errorf("%s:\n got %q\n want %q", tc.name, got, tc.want) + } + } +} + +// The blockquote form stops at the end of the blockquote, not at the end of the +// document — otherwise the whole body would be swallowed into the quote. +func TestBodyTriggerStopsAtParagraphEnd(t *testing.T) { + content := "> **Scope.** First para.\n> Still first.\n\n> A later blockquote that is not the scope.\n\n## Heading\n" + got, ok := bodyTrigger(content) + if !ok { + t.Fatal("expected a trigger") + } + if strings.Contains(got, "later blockquote") || strings.Contains(got, "Heading") { + t.Errorf("paragraph over-ran its end: %q", got) + } + if got != "First para. Still first." { + t.Errorf("got %q", got) + } +} + +func TestTruncateRunes(t *testing.T) { + // Multi-byte input must not be cut mid-rune. + s := strings.Repeat("é", 50) + got := truncateRunes(s, 10) + if len([]rune(got)) > 11 { // 10 + the ellipsis + t.Errorf("truncated to %d runes: %q", len([]rune(got)), got) + } + if !strings.HasSuffix(got, "…") { + t.Errorf("expected an ellipsis, got %q", got) + } + // Short input is returned untouched. + if got := truncateRunes("short", 160); got != "short" { + t.Errorf("short input altered: %q", got) + } + // Cuts on a word boundary when one is near the limit. + got = truncateRunes("alpha beta gamma delta epsilon", 20) + if strings.Contains(got, "delt…") { + t.Errorf("cut mid-word: %q", got) + } +} + +func TestTriggerHint(t *testing.T) { + long := "> **Scope.** " + strings.Repeat("word ", 100) + + // Default truncates to roughly label size. + h := triggerHint(long, false) + if h == "" { + t.Fatal("expected a hint") + } + if len([]rune(h)) > triggerQuoteLimit+80 { + t.Errorf("default hint is not truncated: %d runes", len([]rune(h))) + } + if !strings.Contains(h, "…") { + t.Error("expected the truncation ellipsis") + } + + // --suggest prints it whole. + full := triggerHint(long, true) + if len([]rune(full)) <= len([]rune(h)) { + t.Error("--suggest should produce a longer quote than the default") + } + if strings.Contains(full, "…") { + t.Error("--suggest should not truncate") + } + + // No scope paragraph → no hint at all, in either mode. + if got := triggerHint("just prose", false); got != "" { + t.Errorf("expected no hint, got %q", got) + } + if got := triggerHint("just prose", true); got != "" { + t.Errorf("expected no hint with --suggest either, got %q", got) + } +} diff --git a/internal/cmd/coding/coding.go b/internal/cmd/coding/coding.go index 6f37ed7..ded1e53 100644 --- a/internal/cmd/coding/coding.go +++ b/internal/cmd/coding/coding.go @@ -49,9 +49,13 @@ type checkNode struct { Loc string Name string Description string - Tags []string - Seq *int - IsRunnable bool + // Content is the node body. The shared NodeBatch operation already selects + // it, so carrying it costs no extra round trip — it is what lets a label + // finding quote the trigger paragraph the body already states (#331). + Content string + Tags []string + Seq *int + IsRunnable bool } // graphEdge is one edge incident to a lint root. Other* describe the far @@ -243,6 +247,9 @@ func fetchNodes(ctx context.Context, client graphql.Client, byID map[string]stri if n.Description != nil { cn.Description = *n.Description } + if n.Content != nil { + cn.Content = *n.Content + } if n.IsRunnable != nil { cn.IsRunnable = *n.IsRunnable } diff --git a/internal/cmd/coding/review_lint.go b/internal/cmd/coding/review_lint.go index 25da990..952aab2 100644 --- a/internal/cmd/coding/review_lint.go +++ b/internal/cmd/coding/review_lint.go @@ -30,11 +30,12 @@ type reviewInput struct { Edges map[string]graphEdge // loc → its edge to the review parent Unavailable []string // edge sources that could not be read Toolchain string // "" = infer; "-" = disabled + Suggest bool // quote the body's scope paragraph in full } func newCmdReviewLint(f *cmdutil.Factory) *cobra.Command { var memory, root, toolchain string - var strict bool + var strict, suggest bool var fix, yes bool cmd := &cobra.Command{ Use: "lint", @@ -125,7 +126,7 @@ Errors exit 5; --strict promotes warnings to errors too.`, } // An unreadable node can't be tested against the predicate, so its // membership is indeterminate — reported, never dropped. - in := reviewInput{Members: members, Edges: edgeByLoc, Unavailable: unavailable, Toolchain: toolchain} + in := reviewInput{Members: members, Edges: edgeByLoc, Unavailable: unavailable, Toolchain: toolchain, Suggest: suggest} findings := lintReview(in) if fix { @@ -184,6 +185,7 @@ Errors exit 5; --strict promotes warnings to errors too.`, cmd.Flags().StringVar(&root, "root", reviewRootLoc, "loc of the review parent node") cmd.Flags().StringVar(&toolchain, "toolchain", "", `the memory's toolchain for the foreign-toolchain check (e.g. "ts"; "-" disables; default: inferred)`) cmd.Flags().BoolVar(&strict, "strict", false, "treat warnings as errors") + cmd.Flags().BoolVar(&suggest, "suggest", false, "quote the body's scope paragraph in full rather than truncated") cmd.Flags().BoolVar(&fix, "fix", false, "promote a check's description into an empty/non-condition edge label where possible") cmd.Flags().BoolVar(&yes, "yes", false, "skip the confirmation prompt for --fix") return cmd @@ -211,19 +213,22 @@ func lintReview(in reviewInput) []findingDTO { continue } label := strings.TrimSpace(e.Label) + // The text the label should carry is usually already in the body; + // quote it so the fixer doesn't have to open the node (#331). + hint := triggerHint(n.Content, in.Suggest) switch { case label == "": msg := "edge label is empty — the check can never match a diff" if reRelLoc.MatchString(e.Loc) { msg += fmt.Sprintf(" (its derived loc %q confirms the name was never set)", e.Loc) } - out = append(out, findingDTO{loc, "label-present", sevError, msg}) + out = append(out, findingDTO{loc, "label-present", sevError, msg + hint}) case !strings.HasPrefix(strings.ToLower(label), triggerStem): out = append(out, findingDTO{loc, "label-is-condition", sevError, - fmt.Sprintf("edge label %q is not a condition — expected %q followed by the trigger", label, triggerStem)}) + fmt.Sprintf("edge label %q is not a condition — expected %q followed by the trigger", label, triggerStem) + hint}) case len(strings.TrimSpace(label[len(triggerStem):])) == 0: out = append(out, findingDTO{loc, "label-is-condition", sevError, - fmt.Sprintf("edge label is the bare stem %q with no condition after it", label)}) + fmt.Sprintf("edge label is the bare stem %q with no condition after it", label) + hint}) default: valid[loc] = strings.ToLower(strings.Join(strings.Fields(label), " ")) } diff --git a/internal/cmd/coding/review_lint_test.go b/internal/cmd/coding/review_lint_test.go index 90a884d..c14f48b 100644 --- a/internal/cmd/coding/review_lint_test.go +++ b/internal/cmd/coding/review_lint_test.go @@ -307,3 +307,101 @@ func TestFindingsAreDeterministic(t *testing.T) { } } } + +// #331: a label finding quotes the trigger the body already states, so the +// fixer has the source text without opening the node. +func TestLabelFindingQuotesBodyTrigger(t *testing.T) { + withScope := func(loc string) checkNode { + return checkNode{ + Loc: loc, Tags: []string{"review"}, Description: "Verifies a thing.", + Content: "# Review: x\n\n> **Scope.** Adding or renaming a GraphQL argument that identifies an entity.\n\nBody.", + } + } + for _, label := range []string{"", "child-of", "Applies when"} { + in := reviewInput{ + Members: map[string]checkNode{"review:x": withScope("review:x")}, + Edges: map[string]graphEdge{"review:x": edge("review:x", label)}, + Toolchain: "-", + } + var msg string + for _, f := range lintReview(in) { + if f.Rule == "label-present" || f.Rule == "label-is-condition" { + msg = f.Message + } + } + if msg == "" { + t.Fatalf("label %q: expected a label finding", label) + } + if !strings.Contains(msg, "Adding or renaming a GraphQL argument") { + t.Errorf("label %q: finding should quote the body scope, got %q", label, msg) + } + } +} + +// Two-thirds of checks state no scope, and a healthy label needs no hint — +// neither case should gain a line. +func TestTriggerQuoteStaysQuiet(t *testing.T) { + // Broken label, but no scope paragraph to quote. + noScope := checkNode{Loc: "review:x", Tags: []string{"review"}, Description: "d", Content: "# x\n\nJust prose.\n"} + in := reviewInput{ + Members: map[string]checkNode{"review:x": noScope}, + Edges: map[string]graphEdge{"review:x": edge("review:x", "child-of")}, + Toolchain: "-", + } + for _, f := range lintReview(in) { + if f.Rule == "label-is-condition" && strings.Contains(f.Message, "condense") { + t.Errorf("no scope in the body, so no hint should appear: %q", f.Message) + } + } + + // Healthy label: the body has a scope, but there is nothing to fix. + ok := checkNode{ + Loc: "review:y", Tags: []string{"review"}, Description: "d", + Content: "> **Scope.** Something.\n", + } + in2 := reviewInput{ + Members: map[string]checkNode{"review:y": ok}, + Edges: map[string]graphEdge{"review:y": edge("review:y", "Applies when y changes")}, + Toolchain: "-", + } + for _, f := range lintReview(in2) { + if strings.Contains(f.Message, "condense") { + t.Errorf("a healthy label must not carry a hint: %+v", f) + } + } +} + +// The hint is presentation only — it must not change which rules fire, their +// severities, or the --fix plan. +func TestTriggerQuoteDoesNotAffectRulesOrFix(t *testing.T) { + mk := func(content string) checkNode { + return checkNode{Loc: "review:x", Tags: []string{"review"}, Description: "Applies when a resolver changes.", Content: content} + } + base := reviewInput{ + Members: map[string]checkNode{"review:x": mk("")}, + Edges: map[string]graphEdge{"review:x": edge("review:x", "child-of")}, + Toolchain: "-", + } + withBody := reviewInput{ + Members: map[string]checkNode{"review:x": mk("> **Scope.** Adding an arg.\n")}, + Edges: map[string]graphEdge{"review:x": edge("review:x", "child-of")}, + Toolchain: "-", + } + a, b := lintReview(base), lintReview(withBody) + if len(a) != len(b) { + t.Fatalf("the hint changed the finding count: %d vs %d", len(a), len(b)) + } + for i := range a { + if a[i].Node != b[i].Node || a[i].Rule != b[i].Rule || a[i].Severity != b[i].Severity { + t.Errorf("the hint changed a finding's identity: %+v vs %+v", a[i], b[i]) + } + } + // --fix still reads the description, never the body scope. + pa, pb := planReviewFix(base, a), planReviewFix(withBody, b) + if len(pa) != len(pb) || len(pb) != 1 || pa[0].NewLabel != pb[0].NewLabel { + t.Errorf("the hint leaked into --fix: %+v vs %+v", pa, pb) + } + if strings.Contains(pb[0].NewLabel, "Adding an arg") { + t.Error("--fix must not promote the body scope paragraph") + } +} From 21f1b045dc2e7f795bcde5850018c8e8d1a30374 Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Sat, 1 Aug 2026 10:11:13 +0200 Subject: [PATCH 2/2] fix(coding): rune-consistent truncation, plain-Scope continuation, opt-in content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #332. - truncateRunes compared a BYTE offset against a rune count: LastIndexAny returns bytes while the 3/4 threshold is in runes, so multi-byte text was over-truncated. Verified — a 160-rune CJK trigger whose only space sits at rune 50 (byte ~150) came back as 51 runes. Both the search and the threshold are now in runes. - reScopeMarker makes the blockquote `>` optional, but bodyTrigger always asked joinParagraph for blockquote continuation, so a plain `**Scope.**` paragraph lost everything after its first line. Continuation now follows the line actually matched. Latent rather than observed: all 57 Scope markers in the live memories are blockquoted. - fetchNodes populated Content for every caller. Only `review lint` reads it; `preflight lint` was retaining a body per route target for nothing. Content is now opt-in per call, with a command test asserting review lint surfaces the body scope and preflight lint never does. Co-Authored-By: Claude Opus 5 --- internal/cmd/coding/bodytrigger.go | 28 ++++++++++++---- internal/cmd/coding/bodytrigger_test.go | 41 +++++++++++++++++++++++ internal/cmd/coding/coding.go | 7 ++-- internal/cmd/coding/preflight_lint.go | 2 +- internal/cmd/coding/review_lint.go | 2 +- internal/cmd/coding_cmd_test.go | 43 +++++++++++++++++++++++++ 6 files changed, 112 insertions(+), 11 deletions(-) diff --git a/internal/cmd/coding/bodytrigger.go b/internal/cmd/coding/bodytrigger.go index b66f919..0818d2a 100644 --- a/internal/cmd/coding/bodytrigger.go +++ b/internal/cmd/coding/bodytrigger.go @@ -45,7 +45,13 @@ func bodyTrigger(content string) (string, bool) { lines := strings.Split(content, "\n") for i, ln := range lines { if m := reScopeMarker.FindStringSubmatch(ln); m != nil { - return joinParagraph(m[1], lines[i+1:], true) + // The marker's `>` is optional, so how the paragraph continues has + // to follow the line actually matched — assuming blockquote would + // silently drop the continuation of a plain `**Scope.**` paragraph. + // Every Scope marker in the live memories is blockquoted today, so + // this is a latent case rather than an observed one. + quoted := strings.HasPrefix(strings.TrimSpace(ln), ">") + return joinParagraph(m[1], lines[i+1:], quoted) } if m := reAppliesMarker.FindStringSubmatch(ln); m != nil { // Keep the marker: "Applies when X" already reads as the trigger, @@ -90,18 +96,26 @@ func joinParagraph(first string, rest []string, quoted bool) (string, bool) { return out, true } -// truncateRunes shortens s to at most n runes, cutting at a word boundary when -// one is near the limit so the quote doesn't end mid-word. +// truncateRunes shortens s to at most n runes, backing off to a word boundary +// when one is near the limit so the quote doesn't end mid-word. +// +// Both the search and the threshold are in RUNES. strings.LastIndexAny would +// return a byte offset, which over-truncates multi-byte text: a CJK trigger +// whose only space sits at rune 50 has byte offset ~150, which passes a +// "3/4 of 160" test and cuts 160 runes down to 50. func truncateRunes(s string, n int) string { r := []rune(s) if len(r) <= n { return s } - cut := string(r[:n]) - if i := strings.LastIndexAny(cut, " \t"); i > n*3/4 { - cut = cut[:i] + cut := r[:n] + for i := len(cut) - 1; i > n*3/4; i-- { + if cut[i] == ' ' || cut[i] == '\t' { + cut = cut[:i] + break + } } - return strings.TrimRight(cut, " \t,;:.") + "…" + return strings.TrimRight(string(cut), " \t,;:.") + "…" } // triggerHint is the sentence appended to a label finding when the body states diff --git a/internal/cmd/coding/bodytrigger_test.go b/internal/cmd/coding/bodytrigger_test.go index 165803e..4122184 100644 --- a/internal/cmd/coding/bodytrigger_test.go +++ b/internal/cmd/coding/bodytrigger_test.go @@ -133,3 +133,44 @@ func TestTriggerHint(t *testing.T) { t.Errorf("expected no hint with --suggest either, got %q", got) } } + +// The Scope marker's `>` is optional, so continuation handling must follow the +// line actually matched. Assuming blockquote silently dropped the rest of a +// plain `**Scope.**` paragraph. No live memory uses this form today — all 57 +// Scope markers are blockquoted — so this guards a latent case. +func TestBodyTriggerPlainScopeWraps(t *testing.T) { + got, ok := bodyTrigger("**Scope.** Adding an argument that identifies\nan existing entity.\n\nRest.") + if !ok { + t.Fatal("expected a trigger") + } + want := "Adding an argument that identifies an existing entity." + if got != want { + t.Errorf("plain Scope paragraph lost its continuation:\n got %q\n want %q", got, want) + } + + // The blockquoted form still stops at the end of the blockquote. + got, _ = bodyTrigger("> **Scope.** First line.\nplain continuation must not be absorbed.\n") + if strings.Contains(got, "plain continuation") { + t.Errorf("blockquoted paragraph absorbed a non-quoted line: %q", got) + } +} + +// truncateRunes searches and thresholds in runes. strings.LastIndexAny returns +// a BYTE offset, which over-truncated multi-byte text: a CJK trigger whose only +// space sits at rune 50 has byte offset ~150, which passed a "3/4 of 160" test +// and cut 160 runes down to 50. +func TestTruncateRunesNonASCIIKeepsContext(t *testing.T) { + s := strings.Repeat("経", 50) + " " + strings.Repeat("路", 200) + got := []rune(truncateRunes(s, 160)) + if len(got) < 150 { + t.Errorf("over-truncated multi-byte text to %d runes; want ~160", len(got)) + } + // Still a valid string, still ends with the ellipsis. + if !strings.HasSuffix(string(got), "…") { + t.Errorf("expected an ellipsis, got %q", string(got)) + } + // ASCII behaviour is unchanged: back off to the word boundary. + if got := truncateRunes("alpha beta gamma delta epsilon zeta", 30); strings.Contains(got, "zet") { + t.Errorf("expected a word-boundary cut, got %q", got) + } +} diff --git a/internal/cmd/coding/coding.go b/internal/cmd/coding/coding.go index ded1e53..621fb52 100644 --- a/internal/cmd/coding/coding.go +++ b/internal/cmd/coding/coding.go @@ -215,7 +215,10 @@ func fetchRootEdges(ctx context.Context, client graphql.Client, mem codingMemory // an edge may legitimately cross into another memory, and rebuilding the ref // would then look up the wrong memory — reporting a live node as unresolvable, // or silently linting a same-loc node from the home memory instead. -func fetchNodes(ctx context.Context, client graphql.Client, byID map[string]string) (map[string]checkNode, []string, error) { +// wantContent retains node bodies. Only `review lint` reads them (to quote a +// check's scope paragraph, #331); `preflight lint` never does, so it opts out +// rather than holding every route target's body for the run. +func fetchNodes(ctx context.Context, client graphql.Client, byID map[string]string, wantContent bool) (map[string]checkNode, []string, error) { if len(byID) == 0 { return map[string]checkNode{}, nil, nil } @@ -247,7 +250,7 @@ func fetchNodes(ctx context.Context, client graphql.Client, byID map[string]stri if n.Description != nil { cn.Description = *n.Description } - if n.Content != nil { + if wantContent && n.Content != nil { cn.Content = *n.Content } if n.IsRunnable != nil { diff --git a/internal/cmd/coding/preflight_lint.go b/internal/cmd/coding/preflight_lint.go index ffc6572..8ed32d0 100644 --- a/internal/cmd/coding/preflight_lint.go +++ b/internal/cmd/coding/preflight_lint.go @@ -74,7 +74,7 @@ Errors exit 5; --strict promotes warnings to errors too.`, byID[r.OtherID] = r.Other } } - targets, unavailable, err := fetchNodes(ctx, client, byID) + targets, unavailable, err := fetchNodes(ctx, client, byID, false) if err != nil { return err } diff --git a/internal/cmd/coding/review_lint.go b/internal/cmd/coding/review_lint.go index 952aab2..b35558f 100644 --- a/internal/cmd/coding/review_lint.go +++ b/internal/cmd/coding/review_lint.go @@ -110,7 +110,7 @@ Errors exit 5; --strict promotes warnings to errors too.`, candidates[e.OtherID] = e.Other } - nodes, unavailable, err := fetchNodes(ctx, client, candidates) + nodes, unavailable, err := fetchNodes(ctx, client, candidates, true) if err != nil { return err } diff --git a/internal/cmd/coding_cmd_test.go b/internal/cmd/coding_cmd_test.go index 9eab275..4059efb 100644 --- a/internal/cmd/coding_cmd_test.go +++ b/internal/cmd/coding_cmd_test.go @@ -52,6 +52,16 @@ func codingBatchNode(loc, tags, description string) string { "outgoingEdges":[],"incomingEdges":[]}` } +// codingBatchWithContent is a one-node batch carrying a body. +func codingBatchWithContent(loc, tags, description, content string) string { + n := `{"id":"n_` + loc + `","memoryId":"mem1","loc":"` + loc + `","name":"` + loc + `", + "alias":null,"nodeType":"info","objectType":null,"isRunnable":false,"description":` + jsonStr(description) + `, + "abstract":null,"abstractOriginHash":null,"tags":[` + tags + `],"seq":null,"data":null,"properties":null, + "content":` + jsonStr(content) + `,"createdAt":"2026-07-30T00:00:00Z","updatedAt":"2026-07-30T00:00:00Z", + "outgoingEdges":[],"incomingEdges":[]}` + return codingBatch([]string{n}, "") +} + func codingBatch(nodes []string, unavailable string) string { return `{"data":{"nodeBatch":{"truncated":false,"omitted":[],"unavailable":[` + unavailable + `], "nodes":[` + strings.Join(nodes, ",") + `]}}}` @@ -341,3 +351,36 @@ func TestCodingLintRequiresMemory(t *testing.T) { } } } + +// review lint needs node bodies to quote a check's scope; preflight lint never +// reads them, so it must not retain a body per route target. +func TestCodingFetchNodesContentIsOptIn(t *testing.T) { + body := "> **Scope.** Adding an argument that identifies an entity." + + // review lint: the body reaches the finding. + gql := fakeGraphQL(t, map[string]string{ + "GetNode": codingRootJSON("review", inEdge("e1", "child-of", "review:x"), ""), + "FindNodes": `{"data":{"nodes":[` + codingListNode("review:x") + `]}}`, + "NodeBatch": codingBatchWithContent("review:x", `"review"`, "d", body), + }) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"coding", "review", "lint", "-m", codingMem, "--server", gql.URL}) + _ = root.Execute() + if !strings.Contains(out.String(), "Adding an argument that identifies") { + t.Errorf("review lint should quote the body scope, got %q", out.String()) + } + + // preflight lint: same payload, but nothing derived from the body. + gql2 := fakeGraphQL(t, map[string]string{ + "GetNode": codingRootJSON("preflight", "", outEdge("e2", "to do the thing", "findings:r")), + "NodeBatch": codingBatchWithContent("findings:r", "", "d", body), + }) + f2, out2 := testFactory(t) + root2 := NewRootCmd(f2) + root2.SetArgs([]string{"coding", "preflight", "lint", "-m", codingMem, "--server", gql2.URL}) + _ = root2.Execute() + if strings.Contains(out2.String(), "Adding an argument") { + t.Errorf("preflight lint must not surface node bodies, got %q", out2.String()) + } +}