oficina / memoria Public

The implementation review's findings: recognisers, perf, and four gate holes

Reviewed by Fable 5.1 (dispatched: fable; self-reported: claude-fable-5-1).
REQUEST CHANGES, eight must-fix. It re-ran all ten of the author's
mutations — every one red, as claimed — and then found six of its own
that stayed green, which is §1.9 working exactly as written.

**Three recognisers were wrong on ordinary document text**, verified live
before being fixed. "Take the N11 then exit at Bray" boxed "N11 then" and
"Form P45 then P60" boxed "P45 then", because the eircode identifier
alphabet is most of the consonants and the pattern was case-insensitive.
"The deadline is 01-02-2024 10:30" boxed "01-02-2024 10", walking past
the 9-or-10 bound written to keep dates out, because the pattern allowed
mixed separators. "Growth of +10 000 000 users" boxed "+10 000 000",
because E.164's floor of eight admits it. Eircodes are now matched
case-sensitively, a phone number must use one separator style throughout
and at most four groups, and the international floor is ten.

**And the IBAN kind was inert on its commonest written shape.** Groups of
two-to-four swallowed the following word, so "IE29 AIBK 9311 5212 3456 78
and our BIC is …" found nothing at all; only a full stop immediately
after the number worked. Groups of exactly four with one short tail.

**A label must not depend on which kinds the query named.** Scan located
spans only for wanted kinds, so the earlier-span rule could not skip what
it could not see: "Email paul@tito.io or phone: 01 547 7854" asked for
phone alone came back labelled "Email paul@tito.io or phone" — one fact's
value inside another's label, which is the failure that rule exists to
stop.

**Label derivation was quadratic in the facts a chunk holds**, and
chunk.Split bounds a chunk by words — so a 2 MiB upload with no
whitespace is one chunk, one line, and minutes of search latency bought
by one person's file. Measured 48.5s at 1 MB. Deduping before labelling,
binary-searching the span list and bounding the backward walk take it to
152 ms, and a timed test holds it there.

**Four gate holes, each now with the fixture that sees it.** silenced
counted before the ruling — no other gate-34 fixture had a NoBox
candidate at all. A citation cross-wired field by field — the two bodies
were identical, so only Title could be checked, and the mutation renders
an href to the withheld document under its own heading. A collision check
comparing only neighbours — every other collision fixture is a pair.
Rows sorted rather than first-seen — §10 said this gate asserted order
and it asserted a length. All four mutations re-run and red.

Worth recording: addFacts cannot see unruled data at all, because it
takes only *Page. Every leak mutation has to be made in Search, one level
up — a stronger statement of the design than the comment claiming it.

Also fixed in the composing direction the spec names: initials ("J.
Smith's line" yielded "Smith's line") and NBSP-glued abbreviations, which
Word and Notion produce.

Still failing, and pre-existing on main: TestCreditsMatchProductionDependencies
reads $GOROOT/LICENSE, which the Homebrew Go layout does not have.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 073c17c960bd36f0ad3b52c8aac4c7ca3c51e254 parent cdd003c
6 files changed, +511 −44
  • FACT-BOX-PLAN.md +41 −0
  • docs/superpowers/specs/2026-09-03-memoria-design.md +61 −1
  • internal/memoria/fact/fact.go +165 −25
  • internal/memoria/fact/fact_test.go +113 −0
  • internal/memoria/factbox_test.go +83 −18
  • internal/memoriatest/factbox_test.go +48 −0
diff --git a/FACT-BOX-PLAN.md b/FACT-BOX-PLAN.md
index 874195e..1fc2344 100644
--- a/FACT-BOX-PLAN.md
+++ b/FACT-BOX-PLAN.md
@@ -230,3 +230,44 @@ connectors, `Delete`, `Rebuild`.
That list is the strongest evidence for draft 3. Draft 2's design touched
five of them.
+
+---
+
+## 8. What building it changed (2026-09-15)
+
+Kept as the record of what the plan did not foresee.
+
+**The reachability limit is the ordinary case, not an edge.** Writing the
+gates showed that bare `phone number` does not match a document reading
+`Office: +353 (0)1 547 7854` — the words are not there, nothing ranks,
+and there is nothing to extract from. Every fixture had to be given a
+word the query could find. Pinned in both directions by
+`TestTheBoxOnlySeesWhatRetrievalReturned`, and §12 carries the fix.
+
+**Three recognisers were wrong on ordinary text**, all found by the
+implementation review and none by the fixtures:
+`Take the N11 then exit at Bray` boxed `N11 then`;
+`The deadline is 01-02-2024 10:30` boxed `01-02-2024 10`;
+`Growth of +10 000 000 users` boxed `+10 000 000`. And the grouped IBAN
+pattern made that kind inert on its commonest written shape —
+`IE29 AIBK … 78 and our BIC is …` found nothing at all. §8.8 now carries
+the three structural rules that fix them.
+
+**Label derivation was quadratic in the facts a chunk holds.** `chunk.Split`
+bounds a chunk by WORDS, so a 2 MiB upload with no whitespace is one
+chunk and one line: 48.5s to scan 1 MB, which at the upload cap is
+minutes of search latency bought by one person's file. Deduping before
+labelling, binary-searching the span list and bounding the backward walk
+took 1 MB to 152 ms.
+
+**Four gates had holes the author's own mutations could not reach**, which
+is §1.9's whole point. `silenced` counted before the ruling, a citation
+cross-wired field by field, a collision check comparing only neighbours,
+and rows sorted rather than first-seen — each passed every shipped
+fixture and each now has one written for it.
+
+**And `addFacts` turned out to be structurally unable to see unruled
+data**, because it takes only `*Page`. The leak mutations all have to be
+made in `Search`, one level up. That is a stronger statement of the
+design than the comment claiming it, and it is why the gate-34 comment
+names the mutation at the level where it is reachable.
diff --git a/docs/superpowers/specs/2026-09-03-memoria-design.md b/docs/superpowers/specs/2026-09-03-memoria-design.md
index 5a08ef6..788a48a 100644
--- a/docs/superpowers/specs/2026-09-03-memoria-design.md
+++ b/docs/superpowers/specs/2026-09-03-memoria-design.md
@@ -3094,6 +3094,33 @@ plausible Irish mobile. **The cost is stated too:** a document writing
`0871234567` with no separators is not boxed. That is the safe direction,
and it is the direction a refusal should fail in.
+**Three structural rules came out of building it, and each one is here
+because ordinary document text was boxed without it.** The implementation
+review found all three live, not as mutations:
+
+- **A phone number uses one separator style throughout.** Mixed
+ separators walk straight past the 9-or-10 bound above: *"The deadline
+ is 01-02-2024 10:30"* matched `01-02-2024 10`, ten digits, valid, and
+ boxed a date. Three styles, never mixed.
+- **Four digit groups, never five.** *"Numbers drawn: 01 05 12 23 34"*
+ canonicalises to ten digits and passes every other test. No phone
+ number anybody writes has five groups.
+- **The international floor is ten digits, not E.164's eight.** Eight
+ admits `+10 000 000` out of *"Growth of +10 000 000 users"*. A country
+ code plus a national number reaches ten everywhere this corpus dials,
+ and refusing a genuinely short international number is the safe
+ direction.
+
+**And eircodes are matched case-sensitively**, which is not fussiness.
+The identifier alphabet is most of the consonants, so case-insensitively
+it matches lowercase English after a road number or a form code: *"Take
+the N11 then exit at Bray"* boxed `N11 then`, and *"Form P45 then P60"*
+boxed `P45 then`. A cited fact that is really an English word reads
+exactly like a correct answer — §8.7's *"cites correctly while being
+wrong"*, arriving from a character class. Eircodes are written uppercase;
+a lowercase one going unrecognised is the cost and it is the right way
+round.
+
**`email` is kept and its weakness is stated rather than hidden.**
`local@domain` is closer to pattern-spotting than to validation. On a
corpus of uploads and Notion pages it is useful — one support address,
@@ -3182,6 +3209,14 @@ span's position in the chunk text:
`Phone: 01 547 7854 - 01 598 1234` the second number's run would
otherwise be `Phone: 01 547 7854 -`, labelling the second number with
the first one's value.
+
+ **The spans it skips are of every kind, not only the kinds the query
+ named**, and that is a rule rather than an implementation note: a
+ label that depends on what was asked for is not the document's label.
+ Asked for `phone` alone, *"Email paul@tito.io or phone: 01 547 7854"*
+ came back labelled `Email paul@tito.io or phone` — one fact's value
+ inside another's label, which is the exact failure this rule exists to
+ stop, reappearing because the rule could not see the span.
2. **If what remains ends in a separator** — `:`, `—`, `–`, `-`, `=`, tab
— **and is non-empty after removing it and trimming, and is 60
characters or fewer, that is the label.**
@@ -3976,9 +4011,21 @@ The gates:
unruled set it collides and the box vanishes, which is the same
leak as the cap case wearing the collision rule's coat.
+ - **A withheld over-long label does not silence the kind.** Rule 4 —
+ a document-authored label too long to show renders no box for that
+ kind — is a count like the other two, and has to happen after the
+ ruling for the same reason. **No other fixture here has a `NoBox`
+ candidate at all**, so a mutation computing that rule over the
+ unruled set passed all six of the others. Found by the
+ implementation review.
+
**Assert the struct, not the page** — the candidate tuple
`(span, title, trail, label, DocumentID, Ordinal)` and the notice
- slice, never rendered copy. Two dispositions rendering an identical
+ slice, never rendered copy. **Every field, and the fixtures must
+ differ in all of them**: with two identical bodies, taking the title
+ from the ruled result and the rest from an unruled one passes, and
+ renders an `href` to a document the reader may not open under that
+ document's own heading and label. Two dispositions rendering an identical
page is how `Outcome.Record` filed silent rulings undetected (§1.9).
35. **Candidates are canonical values, ambiguity is enumerated, and the
cap is three** (§8.8). One gate, because the rules share a definition
@@ -4002,9 +4049,22 @@ The gates:
A cap counted across kinds suppresses both, and this is the fixture
that sees it.
+ - **Rows are in first-seen order, asserted** — on a fixture where
+ sorting by value and seeing in document order DISAGREE. A box
+ sorted by canonical value is deterministic and passes every length
+ check, and the two-in-one-passage fixture happens to sort the way
+ it was seen.
+ - **A collision between NON-ADJACENT rows still suppresses.** Every
+ other collision fixture is a pair, so a check comparing only
+ neighbours passes all of them.
+ - **The same label under different HEADINGS still renders**, which is
+ the trail's half of the rule the title case covers.
+
Mutation: return `candidates[0]` when there is more than one and case
2 goes red. Count passages instead of canonical values and cases 4
and 5 go red in opposite directions, which is the pair worth having.
+ The last three above were each found green by the implementation
+ review and each goes red on exactly the fixture written for it.
36. **The displayed value is byte-identical to the document's** (§8.8).
The canonical form exists for matching and must never reach the page.
Fixtures, with the expected bytes written out in each: `+353 (0)1 547
diff --git a/internal/memoria/fact/fact.go b/internal/memoria/fact/fact.go
index a3c9cbc..e6d9beb 100644
--- a/internal/memoria/fact/fact.go
+++ b/internal/memoria/fact/fact.go
@@ -25,6 +25,7 @@ package fact
import (
"regexp"
+ "sort"
"strings"
"unicode"
"unicode/utf8"
@@ -176,6 +177,24 @@ func tokens(s string) []string {
// the failure is a box that silently never appears.
const sep = ` \x{00A0}`
+// phoneGroups builds the digit-group half of the phone pattern for one
+// separator style.
+//
+// **Three repetitions, not four.** Four admits a five-group run of
+// two-digit numbers — "Numbers drawn: 01 05 12 23 34" canonicalises to
+// ten digits and passes every other test. No phone number anybody writes
+// has five groups.
+//
+// **And at least one, which is what makes the three styles alternate.**
+// Written {0,3} the first branch matched zero groups and won, because
+// Go's regexp prefers the earliest alternative that matches AT ALL —
+// so "01-547-7854" came back as the two-digit span "01" and was refused,
+// with the dash branch never tried. A branch that cannot match zero
+// groups fails, and the next one gets its turn.
+func phoneGroups(sepClass string) string {
+ return `(?:` + sepClass + `[0-9]{2,5}){1,3}`
+}
+
type recogniser struct {
kind Kind
re *regexp.Regexp
@@ -195,7 +214,17 @@ var recognisers = []recogniser{
// identifier's alphabet excludes the letters that misread. A
// pattern written [A-Z][0-9][0-9] gets Dublin 6W wrong and lets
// B12 3456 through; both are fixtures.
- re: regexp.MustCompile(`(?i)\b[ACDEFHKNPRTVWXY][0-9][0-9W][` + sep + `]?[0-9ACDEFHKNPRTVWXY]{4}\b`),
+ //
+ // **Case-SENSITIVE, and it was written (?i) first.** The review
+ // found what that costs: the identifier alphabet is most of the
+ // consonants, so lowercase English after a road number or a form
+ // code matches it. "Take the N11 then exit at Bray" boxed
+ // "N11 then"; "Form P45 then P60" boxed "P45 then". A query for
+ // an eircode then gets a cited fact that is an English word —
+ // §8.7's "cites correctly while being wrong", from a character
+ // class. Eircodes are written uppercase, and a lowercase one
+ // going unrecognised is the safe direction.
+ re: regexp.MustCompile(`\b[ACDEFHKNPRTVWXY][0-9][0-9W][` + sep + `]?[0-9ACDEFHKNPRTVWXY]{4}\b`),
canon: func(s string) string { return strings.ToUpper(stripSep(s)) },
valid: func(_, _ string) bool { return true }, // the pattern IS the closed set
},
@@ -217,8 +246,16 @@ var recognisers = []recogniser{
// leading 0. Anchoring there is what keeps account numbers,
// company numbers and DUNS numbers out before any validation
// runs at all.
- re: regexp.MustCompile(`(?i)(?:\+[0-9]{1,3}[` + sep + `]*(?:\(0\))?|\b0)[` + sep + `]*` +
- `[0-9]{1,4}(?:[` + sep + `.-][0-9]{2,5}){0,4}` +
+ //
+ // **One separator style throughout, and phoneValid's 9-or-10
+ // bound is why it has to be.** That bound was written to keep
+ // dates out, and a mixed-separator pattern walked straight past
+ // it: "The deadline is 01-02-2024 10:30" matched
+ // "01-02-2024 10" — ten digits, valid, boxed. A person writing a
+ // phone number uses one separator for all of it. RE2 has no
+ // backreferences, so the three styles are three alternatives.
+ re: regexp.MustCompile(`(?:\+[0-9]{1,3}[` + sep + `]*(?:\(0\))?|\b0)[` + sep + `]*` +
+ `[0-9]{1,4}(?:` + phoneGroups(`[`+sep+`]`) + `|` + phoneGroups(`-`) + `|` + phoneGroups(`\.`) + `)?` +
// An extension is part of the span, because §8.8 says the span
// is what the document wrote, and a number whose extension has
// been dropped is a number that does not reach the person.
@@ -229,9 +266,19 @@ var recognisers = []recogniser{
{
kind: IBAN,
// Two spellings, because a document writes an IBAN either solid
- // or in groups of four and a single pattern for both swallows
- // the words after it.
- re: regexp.MustCompile(`(?i)\b[A-Z]{2}[0-9]{2}(?:[A-Z0-9]{11,30}|(?:[` + sep + `][A-Z0-9]{2,4}){2,8})\b`),
+ // or in groups of four.
+ //
+ // **The grouped alternative was {2,4}-sized groups and that made
+ // the kind inert on its commonest shape.** A trailing 2-4 letter
+ // word was swallowed into the last group, the canonical form came
+ // out the wrong length, and validation refused the whole thing:
+ // "Our IBAN is IE29 AIBK 9311 5212 3456 78 and our BIC is …"
+ // found nothing at all. Only a full stop immediately after the
+ // number worked. Groups of exactly four with one short tail stop
+ // at the space before "and", because "and" is not four
+ // characters — and case-sensitivity means it would not match
+ // even if it were.
+ re: regexp.MustCompile(`\b[A-Z]{2}[0-9]{2}(?:[A-Z0-9]{11,30}|(?:[` + sep + `][A-Z0-9]{4}){2,7}(?:[` + sep + `][A-Z0-9]{1,3})?)\b`),
canon: func(s string) string { return strings.ToUpper(stripSep(s)) },
valid: func(_, c string) bool { return ibanValid(c) },
},
@@ -332,8 +379,14 @@ func phoneValid(span, _ string) bool {
}
digits := strings.TrimPrefix(phoneCanon(base), "+")
if strings.HasPrefix(base, "+") {
- // E.164: a country code plus a subscriber number, 8 to 15 digits.
- return len(digits) >= 8 && len(digits) <= 15
+ // E.164 caps at fifteen. The floor here is TEN rather than the
+ // eight E.164 technically permits, and that is a deliberate
+ // narrowing: a country code plus a national number reaches ten
+ // for every country anybody in this corpus dials, while eight
+ // admits "+10 000 000" out of the sentence "Growth of
+ // +10 000 000 users". Refusing a genuinely short international
+ // number is the safe direction; boxing a growth figure is not.
+ return len(digits) >= 10 && len(digits) <= 15
}
// A national number, canonicalised to +353 above, so the national
// form was 0 plus what follows. Irish numbers are 9 or 10 digits
@@ -487,10 +540,28 @@ func isSeparator(r rune) bool {
return false
}
+// maxLookback bounds how far sentenceStart walks.
+//
+// Without it this is linear in the LINE, and chunk.Split bounds a chunk
+// by words rather than bytes — so a 2 MiB upload with no whitespace is
+// one word, one paragraph, one chunk, and one line. Measured before this
+// bound: 0.49s at 100 KB, 1.94s at 200 KB, 48.5s at 1 MB, which at the
+// upload cap is minutes of search latency bought by one person's file.
+//
+// It changes no outcome. The run has to END in a separator near the
+// value and be MaxLabel characters or fewer to become a label, so a walk
+// that gets this far has already found a run too long to use; capped or
+// not, it falls through the same way.
+const maxLookback = 400
+
// sentenceStart walks back from at to the start of the enclosing sentence
-// or line.
+// or line, bounded by maxLookback.
func sentenceStart(text string, at int) int {
- for i := at - 1; i > 0; i-- {
+ floor := at - maxLookback
+ if floor < 0 {
+ floor = 0
+ }
+ for i := at - 1; i > floor; i-- {
if text[i] == '\n' {
return i + 1
}
@@ -500,15 +571,40 @@ func sentenceStart(text string, at int) int {
// A full stop followed by a space. Whether it ends a sentence
// depends on the word it is attached to.
w := i
- for w > 0 && text[w-1] != ' ' && text[w-1] != '\n' {
+ for w > floor && !isWordBreak(text[w-1]) {
w--
}
- if abbreviations[strings.ToLower(text[w:i])] {
+ if isAbbreviation(text[w:i]) {
continue
}
return i + 1
}
- return 0
+ return floor
+}
+
+// isWordBreak ends the backward word walk. **NBSP counts**, and it did
+// not at first: Word and Notion glue "Contact\u00a0Dr." into one token,
+// which then matched no abbreviation, so the boundary was taken and the
+// label came back as "Smith's direct line" — the composing failure the
+// abbreviation list exists to prevent, arriving through a space nobody
+// can see.
+func isWordBreak(b byte) bool {
+ // The second byte of U+00A0 in UTF-8 is 0xA0; the first is 0xC2, and
+ // stopping on either is enough to end the token.
+ return b == ' ' || b == '\n' || b == '\t' || b == 0xa0 || b == 0xc2
+}
+
+// isAbbreviation reports whether a token ending in "." is one that does
+// not end a sentence.
+//
+// The fixed list, plus **any single letter**, which covers the initials
+// the list cannot enumerate: "J. Smith's line:" yielded "Smith's line"
+// before this, and there is no list of every initial.
+func isAbbreviation(tok string) bool {
+ if abbreviations[strings.ToLower(tok)] {
+ return true
+ }
+ return len(tok) == 2 && tok[1] == '.' && unicode.IsLetter(rune(tok[0]))
}
// labelFor implements §8.8's five rules. It returns the label and whether
@@ -522,10 +618,12 @@ func labelFor(text string, start int, spans [][2]int, heading, title string) (st
// label.** Without this, "Phone: 01 547 7854 - 01 598 1234" labels
// the SECOND number with the first one's value — a box whose left
// column is another phone number.
- for _, sp := range spans {
- if sp[1] <= start && sp[1] > b {
- b = sp[1]
- }
+ //
+ // A binary search over spans sorted by start, not a scan of all of
+ // them: scanning made this quadratic in the facts a chunk holds, and
+ // a chunk can hold tens of thousands of them.
+ if e := lastSpanEndBefore(spans, start); e > b {
+ b = e
}
run := strings.TrimRight(text[b:start], "  ")
if r, size := utf8.DecodeLastRuneInString(run); size > 0 && isSeparator(r) {
@@ -549,6 +647,24 @@ func labelFor(text string, start int, spans [][2]int, heading, title string) (st
return title, false
}
+// lastSpanEndBefore returns the end of the latest span finishing at or
+// before at. spans must be sorted by start.
+func lastSpanEndBefore(spans [][2]int, at int) int {
+ lo, hi, out := 0, len(spans)-1, 0
+ for lo <= hi {
+ mid := (lo + hi) / 2
+ if spans[mid][1] <= at {
+ if spans[mid][1] > out {
+ out = spans[mid][1]
+ }
+ lo = mid + 1
+ } else {
+ hi = mid - 1
+ }
+ }
+ return out
+}
+
// ---------------------------------------------------------------------
// Scan.
// ---------------------------------------------------------------------
@@ -570,9 +686,19 @@ func Scan(text, heading, title string, want []Kind) []Found {
wanted[k] = true
}
- // Every span of every wanted kind is located BEFORE any label is
- // derived, because labelFor needs to know about the spans to its
- // left and cannot if it is called as they are found.
+ // Every span of EVERY kind is located first, and every label is
+ // derived afterwards. Two separate reasons, both found by review:
+ //
+ // - labelFor needs to know about the spans to its left, and cannot
+ // if it is called as they are found.
+ // - **the spans it needs are of every kind, not just the wanted
+ // ones.** Locating only what was asked for made a fact's label
+ // depend on which kinds the query named: "Email paul@tito.io or
+ // phone: 01 547 7854" asked for `phone` alone came back labelled
+ // "Email paul@tito.io or phone", one fact's value inside
+ // another's label — which is exactly what the earlier-span rule
+ // exists to stop, reappearing because the rule could not see the
+ // span.
type hit struct {
kind Kind
at [2]int
@@ -582,20 +708,27 @@ func Scan(text, heading, title string, want []Kind) []Found {
var hits []hit
var spans [][2]int
for _, r := range recognisers {
- if !wanted[r.kind] {
- continue
- }
for _, m := range r.re.FindAllStringIndex(text, -1) {
span := text[m[0]:m[1]]
c := r.canon(span)
if !r.valid(span, c) {
continue
}
- hits = append(hits, hit{r.kind, [2]int{m[0], m[1]}, span, c})
spans = append(spans, [2]int{m[0], m[1]})
+ if wanted[r.kind] {
+ hits = append(hits, hit{r.kind, [2]int{m[0], m[1]}, span, c})
+ }
}
}
-
+ sort.Slice(spans, func(i, j int) bool { return spans[i][0] < spans[j][0] })
+ sort.SliceStable(hits, func(i, j int) bool { return hits[i].at[0] < hits[j].at[0] })
+
+ // **Deduped BEFORE labels are derived, not after.** A label costs a
+ // backward walk, and a chunk repeating one value ten thousand times
+ // needs exactly one. The caller dedupes across passages on the same
+ // key and keeps the first, so doing it here keeps the same
+ // occurrence and changes no outcome.
+ seen := map[Kind]map[string]bool{}
var out []Found
for _, k := range order {
if !wanted[k] {
@@ -605,6 +738,13 @@ func Scan(text, heading, title string, want []Kind) []Found {
if h.kind != k {
continue
}
+ if seen[k] == nil {
+ seen[k] = map[string]bool{}
+ }
+ if seen[k][h.canonical] {
+ continue
+ }
+ seen[k][h.canonical] = true
lbl, noBox := labelFor(text, h.at[0], spans, heading, title)
out = append(out, Found{
Kind: h.kind,
diff --git a/internal/memoria/fact/fact_test.go b/internal/memoria/fact/fact_test.go
index d3b3ebd..9d6bfdb 100644
--- a/internal/memoria/fact/fact_test.go
+++ b/internal/memoria/fact/fact_test.go
@@ -4,6 +4,7 @@ import (
"reflect"
"strings"
"testing"
+ "time"
)
// The corpus this package is judged against is index/rank_test.go's,
@@ -293,3 +294,115 @@ func TestLabelComesFromTheDocument(t *testing.T) {
})
}
}
+
+// Ordinary document text that earlier revisions of these recognisers
+// boxed. Every row here was a live false positive found by review, not
+// an invented one, and each names the class it stands for — because the
+// failure they share is that a cited fact which is really an English
+// word reads exactly like a correct answer (§8.7's "cites correctly
+// while being wrong").
+func TestOrdinaryTextIsNotAFact(t *testing.T) {
+ for _, c := range []struct {
+ kind Kind
+ text string
+ why string
+ }{
+ {Eircode, "Take the N11 then exit at Bray.", "a road number and a lowercase word"},
+ {Eircode, "Form P45 then P60.", "a form code and a lowercase word"},
+ {Eircode, "The A65 area near Sligo is quiet.", "a routing key and a lowercase word"},
+ {Phone, "The deadline is 01-02-2024 10:30 sharp.", "a date followed by a time"},
+ {Phone, "Due 01.02.2024 10:30.", "the same with full stops"},
+ {Phone, "Numbers drawn: 01 05 12 23 34", "five two-digit groups"},
+ {Phone, "Growth of +10 000 000 users this year.", "a plus and grouped thousands"},
+ {Phone, "Companies House 12345678 is the reference.", "a bare run of digits"},
+ } {
+ if got := Scan(c.text, "Contact", "Notes", []Kind{c.kind}); len(got) != 0 {
+ t.Errorf("%s: %q boxed %q as a %s", c.why, c.text, got[0].Span, c.kind)
+ }
+ }
+}
+
+// The shapes a document actually writes, which must all still be found.
+// Half of these are here because a fix for the table above broke them:
+// a recogniser tightened until nothing false matches is a recogniser
+// that finds nothing at all, and only having both tables makes that
+// visible.
+func TestTheShapesDocumentsActuallyWrite(t *testing.T) {
+ for _, c := range []struct {
+ kind Kind
+ text string
+ want string
+ }{
+ {Phone, "Office phone: +353 (0)1 547 7854", "+353 (0)1 547 7854"},
+ {Phone, "Ring 087 123 4567 today.", "087 123 4567"},
+ {Phone, "Call 01-547-7854 for support.", "01-547-7854"},
+ {Eircode, "Eircode: D02 X285", "D02 X285"},
+ {Eircode, "We are at A65 F4E2.", "A65 F4E2"},
+ // Grouped, and followed by a BIC — which is how bank details are
+ // written, and which found nothing at all until the grouped
+ // alternative was fixed to groups of four.
+ {IBAN, "Our IBAN is IE29 AIBK 9311 5212 3456 78 and our BIC is AIBKIE2D.", "IE29 AIBK 9311 5212 3456 78"},
+ {IBAN, "IBAN: IE29 AIBK 9311 5212 3456 78 BIC AIBKIE2D", "IE29 AIBK 9311 5212 3456 78"},
+ {IBAN, "IBAN IE29AIBK93115212345678.", "IE29AIBK93115212345678"},
+ {VAT, "Our VAT number is IE3384527RH.", "IE3384527RH"},
+ } {
+ got := Scan(c.text, "Contact", "Notes", []Kind{c.kind})
+ if len(got) != 1 {
+ t.Errorf("%q found %d, want 1", c.text, len(got))
+ continue
+ }
+ if got[0].Span != c.want {
+ t.Errorf("%q span = %q, want %q", c.text, got[0].Span, c.want)
+ }
+ }
+}
+
+// A label must not depend on which kinds the query named.
+//
+// Scan locates spans of EVERY kind and emits only the wanted ones,
+// because the earlier-span rule can only skip a span it can see. Asking
+// for `phone` alone used to yield "Email paul@tito.io or phone" — one
+// fact's value inside another fact's label, which is exactly what that
+// rule exists to prevent.
+func TestALabelDoesNotDependOnWhichKindsWereAskedFor(t *testing.T) {
+ const text = "Email paul@tito.io or phone: 01 547 7854"
+ alone := Scan(text, "Contact", "T", []Kind{Phone})
+ both := Scan(text, "Contact", "T", []Kind{Phone, Email})
+ if len(alone) != 1 {
+ t.Fatalf("phone alone found %d, want 1", len(alone))
+ }
+ if strings.Contains(alone[0].Label, "@") {
+ t.Errorf("label %q carries another fact's value", alone[0].Label)
+ }
+ var phone Found
+ for _, f := range both {
+ if f.Kind == Phone {
+ phone = f
+ }
+ }
+ if phone.Label != alone[0].Label {
+ t.Errorf("label changed with the query: %q alone, %q with email", alone[0].Label, phone.Label)
+ }
+}
+
+// Label derivation was quadratic in the facts a chunk holds, and
+// chunk.Split bounds a chunk by WORDS — so a 2 MiB upload with no
+// whitespace is one chunk, one line, and minutes of search latency
+// bought by one person's file. Measured before the bound: 48.5s at 1 MB.
+//
+// A wall-clock assertion, which is a blunt instrument, so the bound is
+// loose enough not to flake and tight enough that the quadratic version
+// cannot pass it by two orders of magnitude.
+func TestLabellingIsNotQuadraticInAChunksFacts(t *testing.T) {
+ text := strings.Repeat("D02X285.", 125000) // 1 MB, one line, no whitespace
+ done := make(chan int, 1)
+ go func() { done <- len(Scan(text, "H", "T", []Kind{Eircode})) }()
+ select {
+ case n := <-done:
+ if n != 1 {
+ t.Errorf("found %d candidates, want 1: they are all the same value", n)
+ }
+ case <-time.After(5 * time.Second):
+ t.Fatal("scanning 1 MB took over 5s; label derivation is quadratic again")
+ }
+}
diff --git a/internal/memoria/factbox_test.go b/internal/memoria/factbox_test.go
index 71c6641..2eff9a6 100644
--- a/internal/memoria/factbox_test.go
+++ b/internal/memoria/factbox_test.go
@@ -175,12 +175,18 @@ func TestGate34TheFactBoxNeverSpeaksAWithheldPassage(t *testing.T) {
}
})
- // 5. THE CITATION IS THE SURVIVOR'S. The SAME eircode in a visible
- // and a withheld document: one candidate, cited to the visible one.
- // Every other fixture uses distinct values per document, so "cite
- // from the hits rather than from the results" passes all of them.
- // This is the only case that can see it.
- t.Run("the citation is the visible passage's", func(t *testing.T) {
+ // 5. EVERY FIELD OF THE CITATION IS THE SURVIVOR'S. The SAME eircode
+ // in a visible and a withheld document: one candidate, and all five
+ // of its fields from the visible one.
+ //
+ // **Asserting Title alone was not enough**, and a review mutation
+ // proved it: taking Title from the ruled result and DocumentID,
+ // Ordinal, Label and Trail from the first UNRULED occurrence passed
+ // this case, and would have rendered an href to a document the
+ // reader may not open, under the withheld document's own heading and
+ // label text. The two bodies were identical, so nothing could tell
+ // the fields apart. They differ now, and §10 says assert the tuple.
+ t.Run("every field of the citation is the visible passage's", func(t *testing.T) {
cus := &perItemCustody{
apps: roster,
give: map[string]suite.Authorization{"hidden": suite.NOT_VISIBLE},
@@ -188,24 +194,83 @@ func TestGate34TheFactBoxNeverSpeaksAWithheldPassage(t *testing.T) {
s := storeForTest(t)
s.custody = cus
ctx := context.Background()
- // The withheld one is ingested FIRST so it ranks first among the
- // hits: if the citation came from the hits, it would be this one.
- for _, d := range [][2]string{{"hidden", "Hidden office"}, {"shown", "Visible office"}} {
+ // The withheld one is ingested first AND carries the query's
+ // words three times, so it ranks above the visible one: if any
+ // field came from the hits rather than the results, it would be
+ // this document's.
+ hidden, err := s.Ingest(ctx, IngestSpec{
+ Source: "Docs", Key: "hidden", Title: "Hidden office",
+ Body: "## Hidden trail\n\nHidden eircode eircode eircode label: D02 X285",
+ ItemID: "hidden", SiblingRevision: 1,
+ })
+ if err != nil {
+ t.Fatalf("ingest hidden: %v", err)
+ }
+ shown, err := s.Ingest(ctx, IngestSpec{
+ Source: "Docs", Key: "shown", Title: "Visible office",
+ Body: "## Contact\n\nEircode: D02 X285",
+ ItemID: "shown", SiblingRevision: 1,
+ })
+ if err != nil {
+ t.Fatalf("ingest shown: %v", err)
+ }
+ if hidden == shown {
+ t.Fatal("the two documents share an id; this fixture cannot be wrong about anything")
+ }
+ p := searchFacts(t, s, "eircode")
+ if len(p.Facts) != 1 {
+ t.Fatalf("facts = %+v, want one", p.Facts)
+ }
+ got := p.Facts[0]
+ for _, c := range []struct{ name, got, want string }{
+ {"title", got.Title, "Visible office"},
+ {"trail", got.Trail, "Contact"},
+ {"label", got.Label, "Eircode"},
+ } {
+ if c.got != c.want {
+ t.Errorf("%s = %q, want %q: the box took a field from the passage this reader may not read", c.name, c.got, c.want)
+ }
+ }
+ if got.DocumentID != shown {
+ t.Errorf("citation links to document %d, want %d: the href points at the withheld document", got.DocumentID, shown)
+ }
+ })
+
+ // 7. A WITHHELD PASSAGE CANNOT SILENCE A KIND. §8.8's rule 4 — a
+ // document-authored label too long to show renders no box for that
+ // kind — is a count like the cap and the collision check, and it has
+ // to happen after the ruling for the same reason. Computed over the
+ // unruled set, a document the reader may not see takes the box away
+ // from them, which reports that document's existence.
+ //
+ // **No other fixture here has a NoBox candidate at all**, so a
+ // mutation OR-ing `silenced` from the unruled set passed all six of
+ // them. Found by review, 2026-09-14.
+ t.Run("a withheld over-long label does not silence the kind", func(t *testing.T) {
+ cus := &perItemCustody{
+ apps: roster,
+ give: map[string]suite.Authorization{"hidden": suite.NOT_VISIBLE},
+ }
+ s := storeForTest(t)
+ s.custody = cus
+ ctx := context.Background()
+ for _, d := range [][3]string{
+ // Over sixty characters, and carrying the query's word so it
+ // is actually retrieved — the first version of this fixture
+ // was green because nothing ever reached the box.
+ {"hidden", "Hidden office", "The eircode you must never give to any customer or supplier is: T12 XY45"},
+ {"shown", "Visible office", "Eircode: D02 X285"},
+ } {
if _, err := s.Ingest(ctx, IngestSpec{
Source: "Docs", Key: d[0], Title: d[1],
- Body: "## Contact\n\nEircode: D02 X285",
- ItemID: d[0],
- SiblingRevision: 1,
+ Body: "## Contact\n\n" + d[2],
+ ItemID: d[0], SiblingRevision: 1,
}); err != nil {
t.Fatalf("ingest %s: %v", d[0], err)
}
}
- p := searchFacts(t, s, "eircode")
- if len(p.Facts) != 1 {
- t.Fatalf("facts = %+v, want one", p.Facts)
- }
- if p.Facts[0].Title != "Visible office" {
- t.Errorf("cited %q; the box cited the passage this reader may not read", p.Facts[0].Title)
+ if got := spans(searchFacts(t, s, "eircode")); len(got) != 1 || got[0] != "D02 X285" {
+ t.Fatalf("spans = %q, want the visible one boxed", got)
}
})
diff --git a/internal/memoriatest/factbox_test.go b/internal/memoriatest/factbox_test.go
index 6e78ada..d493342 100644
--- a/internal/memoriatest/factbox_test.go
+++ b/internal/memoriatest/factbox_test.go
@@ -174,6 +174,54 @@ func TestGate35CandidatesAmbiguityAndTheCap(t *testing.T) {
}
})
+ // §10 says this gate asserts first-seen order, and for a while it
+ // asserted only the count. A box sorted by canonical value is
+ // deterministic and passes every length check, and the two-values-in-
+ // one-passage fixture happens to sort the same way it was seen — so
+ // nothing here could see it. Found by review, 2026-09-14.
+ t.Run("rows are in first-seen order", func(t *testing.T) {
+ h := newApp(t)
+ // Sorted by canonical the office number comes FIRST; seen in
+ // document order it comes second. The two orders disagree, which
+ // is what makes this assertion able to fail.
+ store(t, h, "Office address", "## Office Address\n\nTurn the knob. "+
+ "The office phone number is: +353 (0)1 598 1234. "+
+ "We also have the following unused phone numbers: +353 (0)1 547 7854.")
+ got := factValues(search(t, h, "phone number"))
+ if len(got) != 2 {
+ t.Fatalf("values = %q, want two", got)
+ }
+ if got[0] != "+353 (0)1 598 1234" {
+ t.Errorf("first row = %q; the box is not in the order the document states them", got[0])
+ }
+ })
+
+ // Every collision fixture above is a PAIR, so a check comparing only
+ // neighbours passes all of them. Three rows whose first and third are
+ // identical is the shape that sees it.
+ t.Run("a collision between non-adjacent rows still suppresses", func(t *testing.T) {
+ h := newApp(t)
+ store(t, h, "Tito office", "## Contact\n\n"+
+ "Ring the office phone on 01 547 7854 any weekday. "+
+ "Support phone: 01 598 1234. "+
+ "Or dial the phone on 01 555 0101 instead.")
+ if body := search(t, h, "phone number"); hasFactBox(body) {
+ t.Errorf("rendered a box whose first and third rows are identical: %q / %q",
+ factLabels(body), factValues(body))
+ }
+ })
+
+ // The mirror of the title case below, for the trail. Same title, same
+ // label, different headings — the trail is what tells them apart, and
+ // a collision check ignoring it loses a real answer.
+ t.Run("the same label under different headings still renders", func(t *testing.T) {
+ h := newApp(t)
+ store(t, h, "Tito office", "## Office\n\nPhone: +353 (0)1 547 7854\n\n## Support\n\nPhone: +353 (0)1 598 1234")
+ if got := factValues(search(t, h, "phone number")); len(got) != 2 {
+ t.Fatalf("values = %q, want both: the headings tell them apart", got)
+ }
+ })
+
// A different title disambiguates perfectly, so this one MUST box.
// Keying collision on the label alone loses a real answer here.
t.Run("the same label under different titles still renders", func(t *testing.T) {