fix: require a delimiter for bare TODO/FIXME markers

.abcd/development/specs/open/spc-12-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md

@@ -83,16 +83,22 @@ The primitive therefore adds no second read path to audit.
 
 - **Recognised markers:** `TODO`, `FIXME`, `XXX`, `HACK`, `BUG` — uppercase
   only, matched by
-  `(^|[^A-Za-z0-9_-])(TODO|FIXME|XXX|HACK|BUG)(:|\(|\s|$)`. The leading class
-  is the word boundary that stops `TODO` matching inside `TODOS` or
-  `todo_list`; the trailing class admits the two conventional spellings
-  (`TODO:` and `TODO(alice):`) plus a bare word. The hyphen is excluded from
-  the leading class so the common redaction placeholder shape (`XXX-XXX-XXX`)
-  is rejected at every one of its triples — with the hyphen admitted, the last
-  triple matches on its leading `-` and a support phone number becomes a
-  fabricated open question. `NOTE` and `OPTIMIZE` are *not* recognised: `NOTE`
-  marks explanation rather than unfinished work, and `OPTIMIZE` is rare enough
-  that its false-positive cost exceeds its value.
+  `(^|[^A-Za-z0-9_-])(?:(TODO|FIXME)(?::|\()|(XXX|HACK|BUG)(?::|\(|\s|$))`.
+  The leading class is the word boundary that stops a marker matching inside a
+  longer identifier (`TODOS`, `todo_list`). The trailing boundary splits the
+  markers into two classes: `TODO` and `FIXME` require a trailing `:` or `(`
+  (the conventional `TODO:` and `TODO(alice):` spellings), because these are the
+  markers a project documents by name and their bare-word form is
+  indistinguishable from prose that merely mentions the marker (iss-111); `XXX`,
+  `HACK`, and `BUG` additionally admit a bare word (trailing whitespace or
+  end-of-line), because they are rarely written as bare uppercase words in prose
+  and the bare spelling is how they are conventionally written. The hyphen is
+  excluded from the leading class so the common redaction placeholder shape
+  (`XXX-XXX-XXX`) is rejected at every one of its triples — with the hyphen
+  admitted, the last triple matches on its leading `-` and a support phone
+  number becomes a fabricated open question. `NOTE` and `OPTIMIZE` are *not*
+  recognised: `NOTE` marks explanation rather than unfinished work, and
+  `OPTIMIZE` is rare enough that its false-positive cost exceeds its value.
 - **Binary files are skipped** by a NUL byte in the first 8 KiB — the
   conventional heuristic, and dependency-free. No extension allow-list is
   maintained.
@@ -131,7 +137,7 @@ The primitive therefore adds no second read path to audit.
 
 | Question | Decision |
 |---|---|
-| Which markers? | `TODO`, `FIXME`, `XXX`, `HACK`, `BUG`; uppercase only; word-boundary anchored, trailing `:`/`(`/space/EOL. `NOTE`, `OPTIMIZE` excluded. |
+| Which markers? | `TODO`, `FIXME`, `XXX`, `HACK`, `BUG`; uppercase only; word-boundary anchored. `TODO`/`FIXME` require a trailing `:`/`(` (a bare word reads as prose that names the marker, iss-111); `XXX`/`HACK`/`BUG` also admit a bare word (trailing space/EOL). `NOTE`, `OPTIMIZE` excluded. |
 | Which tier — conventions or git? | **Conventions.** A working-tree file scan through the `SourceContext` file surface. The adapter never touches git, so it grounds a bare snapshot as readily as a working tree. |
 | Scan scope and the missing primitive | Option (a): add a bounded recursive-walk primitive, `WalkFiles`, to `SourceContext`. It is shared with itd-96, so the walk lands once. |
 | Which files to scan | Every regular file the walk yields, minus the skip set (`.git`, `node_modules`, `vendor`, `generated`), minus symlinks, minus binaries (NUL-byte heuristic), minus oversized files (`ReadFile`'s cap). |

.abcd/work/issues/open/iss-111-the-open-questions-marker-pattern-admits-a-bare-uppercase-wo.md → .abcd/work/issues/resolved/iss-111-the-open-questions-marker-pattern-admits-a-bare-uppercase-wo.md

@@ -7,6 +7,8 @@ category: "tech-debt"
 source: "impl-review"
 found_during: "itd-95 P1 review"
 found_at: "internal/core/lifeboat/sources_conventions.go"
+resolution: "Marker pattern now splits by trailing boundary: TODO/FIXME require ':' or '(', XXX/HACK/BUG keep the bare word (option 2 of the two recorded). On this repo the durable-record documentation FP corpus (scope: .abcd/development/) dropped from 14 to 3 (the 3 survivors are prose that literally quotes the colon form, irreducible for this mechanism); 100% of the bare-word 'mentions a marker' FP class killed. All colon-form fixture markers preserved; only the bare 'FIXME check this' and '# TODO' pinning spellings reclassified to reject, correctly, as indistinguishable from prose. spc-12 pattern description amended in the same change."
+impact: fix
 ---
 
 The open-questions marker pattern admits a bare uppercase word followed by whitespace, so prose that merely mentions a marker matches. Measured against this repository the adapter reports 42 markers across 11 files at medium confidence, and every one is documentation about markers rather than a work marker. The precision cost lands on repos that document their own conventions. Options: require the trailing colon or parenthesis, or drop the bare-word alternative for the ambiguous markers only. spc-12 fixes the current pattern, so this is a design revisit.

CHANGELOG.md

@@ -54,6 +54,19 @@ called out in a **Breaking** section.
 
 ### Fixed
 
+- **The open-questions marker scan no longer reads documentation about markers as
+  open questions** (iss-111). The pattern that grounds `evidence/open-questions`
+  admitted a bare uppercase `TODO`/`FIXME` followed by whitespace, so on a
+  repository that documents its own conventions every prose mention of a marker
+  was cited as a work marker — 14 such false positives across the durable record
+  (`.abcd/development/`) on this repository, all documentation, none a real
+  marker, down to 3 after the fix (each an irreducible prose quotation of the
+  literal `TODO:` form). `TODO` and `FIXME`
+  now require a trailing `:` or `(` (the conventional `TODO:` / `TODO(alice):`
+  spellings), which is how genuine markers are almost always written; `XXX`,
+  `HACK`, and `BUG` still admit their conventional bare form, because they are
+  rarely written as bare words in prose and carry no measured false-positive
+  cost.
 - **Concurrent runs can no longer drop a repo registration or delete a
   just-committed issue file** (iss-101, iss-102). Two `abcd ahoy install` runs
   from different worktrees shared one `~/.abcd/history/index.json`, and its

internal/core/lifeboat/sources_conventions.go

@@ -697,18 +697,54 @@ func (convInternalsSource) probeLimited(ctx *SourceContext, walkLimit int) Evide
 // convMarkerNames are the in-code work markers recognised as open questions,
 // uppercase only. NOTE and OPTIMIZE are deliberately absent: NOTE marks
 // explanation rather than unfinished work, and OPTIMIZE is rare enough that its
-// false-positive cost exceeds its value.
-var convMarkerNames = []string{"TODO", "FIXME", "XXX", "HACK", "BUG"}
+// false-positive cost exceeds its value. The set splits by how the pattern
+// anchors each marker's trailing boundary (see convMarkerRe).
+var convMarkerNames = append(append([]string{}, convDelimitedMarkers...), convBareMarkers...)
+
+// convDelimitedMarkers must carry a trailing ':' or '(' to be recognised. They
+// are the markers a project documents by name, so their bare-word form (the
+// marker followed by whitespace or end-of-line) is indistinguishable from prose
+// that merely mentions the marker — the dominant false-positive class on a
+// repository that documents its own conventions (iss-111). Requiring the
+// conventional trailing delimiter — a colon or an open-paren, as in an authored
+// marker naming its owner — keeps the genuine markers and drops the mentions.
+// (This comment states the markers as slice values below rather than inline, so
+// the scan does not cite its own prose.)
+var convDelimitedMarkers = []string{"TODO", "FIXME"}
+
+// convBareMarkers additionally match a bare word (a trailing whitespace or
+// end-of-line). These three are rarely written as bare uppercase words in running
+// prose, so their bare form carries no measured false-positive cost, and that
+// bare spelling is how they are conventionally written — requiring a delimiter
+// would lose real markers for no precision gain. (Named only as slice values
+// below, for the same reason as convDelimitedMarkers.)
+var convBareMarkers = []string{"XXX", "HACK", "BUG"}
 
 // convMarkerRe matches one recognised marker on a line. The leading class is the
-// word boundary that stops TODO matching inside TODOS or todo_list; the trailing
-// class admits the two conventional spellings (TODO: and TODO(alice):) plus a
-// bare word. The hyphen is excluded from the leading class so the redaction
-// placeholder shape (XXX-XXX-XXX) is rejected at every one of its triples —
-// without it the last triple matches on its leading hyphen and a support phone
-// number becomes a fabricated open question. Built from convMarkerNames so the
-// set and the pattern cannot drift, and compiled once.
-var convMarkerRe = regexp.MustCompile(`(^|[^A-Za-z0-9_-])(` + strings.Join(convMarkerNames, "|") + `)(:|\(|\s|$)`)
+// word boundary that stops a marker matching inside a longer identifier (TODOS,
+// todo_list); the hyphen is excluded from it so the redaction placeholder shape
+// (XXX-XXX-XXX) is rejected at every one of its triples — without that exclusion
+// the last triple matches on its leading hyphen and a support phone number
+// becomes a fabricated open question. The two marker classes differ only in their
+// trailing boundary: convDelimitedMarkers (captured in group 2) require ':' or
+// '('; convBareMarkers (group 3) also accept whitespace or end-of-line. Exactly
+// one of the two marker groups is non-empty on a match — convMarkerName reads
+// whichever fired. Built from the marker slices so the set and the pattern cannot
+// drift, and compiled once.
+var convMarkerRe = regexp.MustCompile(
+	`(^|[^A-Za-z0-9_-])` +
+		`(?:(` + strings.Join(convDelimitedMarkers, "|") + `)(?::|\()` +
+		`|(` + strings.Join(convBareMarkers, "|") + `)(?::|\(|\s|$))`)
+
+// convMarkerName returns the marker a convMarkerRe match captured. The pattern
+// puts a delimited marker in group 2 and a bare-allowed marker in group 3, and
+// exactly one is non-empty.
+func convMarkerName(m []string) string {
+	if m[2] != "" {
+		return m[2]
+	}
+	return m[3]
+}
 
 // maxMarkerCitations caps how many path:line citations the marker scan reports.
 // Beyond it the scan keeps counting — the headline stays truthful — but stops
@@ -805,7 +841,7 @@ func (convOpenQuestionsSource) probeLimited(ctx *SourceContext, budget int) Evid
 			hits++
 			markers++
 			if len(citations) < maxMarkerCitations {
-				citations = append(citations, fmt.Sprintf("%s:%d (%s)", p, i+1, m[2]))
+				citations = append(citations, fmt.Sprintf("%s:%d (%s)", p, i+1, convMarkerName(m)))
 			}
 		}
 		if hits > 0 {

internal/core/lifeboat/sources_conventions_test.go

@@ -434,16 +434,39 @@ func TestConvOpenQuestionsIgnoresRedactionPlaceholders(t *testing.T) {
 	}
 }
 
+// TestConvOpenQuestionsIgnoresProseMentioningMarkers holds the iss-111 contract:
+// a repository that documents its own conventions mentions TODO and FIXME by name
+// in prose, and those mentions are not the team's open questions. A bare TODO or
+// FIXME followed by whitespace reads exactly like such a mention, so the scan must
+// come back blank rather than fabricate open questions out of the documentation.
+func TestConvOpenQuestionsIgnoresProseMentioningMarkers(t *testing.T) {
+	dir := t.TempDir()
+	writeTree(t, dir, map[string]string{
+		"go.mod": "module example.com/documented\n\ngo 1.22\n",
+		"CONTRIBUTING.md": "# Conventions\n\n" +
+			"Leave a TODO for unfinished work and a FIXME where a bug hides.\n" +
+			"We grep for TODO and FIXME markers before every release.\n",
+	})
+	ctx, err := newSourceContext(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ctx.Close()
+
+	ev := convSourceForSection(t, "evidence/open-questions").Probe(ctx)
+	if ev.Status != StatusBlank {
+		t.Fatalf("prose that merely names markers gives status %s, want blank; fabricated evidence %v", ev.Status, ev.Sources)
+	}
+}
+
 // TestConvMarkerRePinsTheRecognisedSpellings pins the marker pattern against the
 // spellings it must accept and the near-misses it must reject, so the word
 // boundary cannot be loosened or tightened without a test saying so.
 func TestConvMarkerRePinsTheRecognisedSpellings(t *testing.T) {
 	match := []string{
 		"// TODO: handle the retry case",
 		"//TODO: no space after the slashes",
-		"# TODO",
 		"- TODO: a list item",
-		"* FIXME check this",
 		"FIXME(alice): leaks a connection",
 		"-- BUG --",
 		"// HACK around the driver",
@@ -455,6 +478,15 @@ func TestConvMarkerRePinsTheRecognisedSpellings(t *testing.T) {
 		"XXX-XXX-XXX",
 		"TODOS are not markers",
 		"todo_list := nil",
+		// iss-111: TODO and FIXME require a trailing ':' or '('. A bare word is
+		// indistinguishable from prose that merely names the marker — the
+		// false-positive class that swamped a repo documenting its own
+		// conventions — so these documentation mentions must not match.
+		"# TODO",
+		"* FIXME check this",
+		"names TODO and FIXME markers as a read",
+		"a codebase dense with TODO markers",
+		"the TODO and FIXME conventions this project follows",
 	}
 	for _, line := range match {
 		if convMarkerRe.FindStringSubmatch(line) == nil {
@@ -463,7 +495,7 @@ func TestConvMarkerRePinsTheRecognisedSpellings(t *testing.T) {
 	}
 	for _, line := range reject {
 		if m := convMarkerRe.FindStringSubmatch(line); m != nil {
-			t.Errorf("convMarkerRe matches %q as a %s marker", line, m[2])
+			t.Errorf("convMarkerRe matches %q as a %s marker", line, convMarkerName(m))
 		}
 	}
 }