amadan
Review nits from the shared-chrome and rastrillo-path reviews
Fixes for the non-blocking findings on the merged branches (see this branch's discussion). The coordinating session checked them, on Paul's standing instruction to roll in fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Squashed from chore/review-nits into main.
1 file changed,
+133
−35
internal/memoriatest/gates_test.go+133 −35
CI passed — run details
CI log
=== vet === go vet ./... go vet -tags browser ./internal/memoriatest/ === fmt === === test === go test ./... ? amadan.net/oficina/memoria/cmd/memoria [no test files] ok amadan.net/oficina/memoria/internal/memoria 4.336s ok amadan.net/oficina/memoria/internal/memoria/chunk (cached) ok amadan.net/oficina/memoria/internal/memoria/fact (cached) ? amadan.net/oficina/memoria/internal/memoria/icons [no test files] ok amadan.net/oficina/memoria/internal/memoria/index (cached) ok amadan.net/oficina/memoria/internal/memoria/notion (cached) ok amadan.net/oficina/memoria/internal/memoria/provider (cached) ok amadan.net/oficina/memoria/internal/memoria/query (cached) ok amadan.net/oficina/memoria/internal/memoria/seal (cached) ok amadan.net/oficina/memoria/internal/memoria/sibling (cached) ok amadan.net/oficina/memoria/internal/memoriatest 2.174s ? amadan.net/oficina/memoria/scripts/writeestilo [no test files] ? amadan.net/oficina/memoria/tools/bench [no test files] ? amadan.net/oficina/memoria/tools/drive [no test files] ? amadan.net/oficina/memoria/tools/seed [no test files] ? amadan.net/oficina/memoria/tools/vendorassets [no test files] === migration-check === go tool rastrillo migration check rastrillo migration check: models and migrations agree
diff --git a/internal/memoriatest/gates_test.go b/internal/memoriatest/gates_test.go| index 3519071..2d3c5c5 100644 |
| --- a/internal/memoriatest/gates_test.go |
| +++ b/internal/memoriatest/gates_test.go |
| @@ -828,6 +828,21 @@ func bytesContainsFold(hay []byte, needle string) bool { |
| return strings.Contains(strings.ToLower(string(hay)), strings.ToLower(needle)) |
| } |
| +// designCSS is every stylesheet the templates are styled by, as one |
| +// string. |
| +func designCSS(t *testing.T) string { |
| + t.Helper() |
| + css := "" |
| + for _, name := range []string{"tokens.css", "theme.css", "memoria.css"} { |
| + raw, err := os.ReadFile(filepath.Join("..", "memoria", "static", name)) |
| + if err != nil { |
| + t.Fatalf("read %s: %v", name, err) |
| + } |
| + css += string(raw) |
| + } |
| + return css |
| +} |
| + |
| // Every `rst-` attribute a template writes is one the stylesheets |
| // actually style. |
| // |
| @@ -856,14 +871,7 @@ func bytesContainsFold(hay []byte, needle string) bool { |
| // write rst-callout="danger" again -> red |
| // misspell rst-callout-body -> red |
| func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) { |
| - css := "" |
| - for _, name := range []string{"tokens.css", "theme.css", "memoria.css"} { |
| - raw, err := os.ReadFile(filepath.Join("..", "memoria", "static", name)) |
| - if err != nil { |
| - t.Fatalf("read %s: %v", name, err) |
| - } |
| - css += string(raw) |
| - } |
| + css := designCSS(t) |
| // **Hooks, not styles**, and the list is derived rather than typed: |
| // anything one of the vendored scripts reads is a hook by definition, |
| @@ -896,12 +904,7 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) { |
| // now memoria.css's [rst-muted]; the Sources page's rst-list-item and |
| // rst-list-title are [rst-sources] rows; rst-facts is styled; the |
| // header's rst-brand and the Delete button's rst-danger are gone. |
| - unstyled := map[string]bool{ |
| - "rst-lede": true, |
| - "rst-steps": true, |
| - "rst-inline": true, |
| - "rst-bg": true, |
| - } |
| + unstyled := templateUnstyled |
| // **Values this app writes that the design system does not define.** |
| // Same treatment and same reason as the list above: named, dated, and |
| @@ -922,17 +925,8 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) { |
| "rst-btn=quiet": true, |
| "rst-btn=compact": true, |
| } |
| - exempt := map[string]bool{ |
| - "rst-withheld": true, // a test hook for §6.5's notice block |
| - // A class-only utility, written class="rst-sr-only" (tokens.css's |
| - // .rst-sr-only; utilities keep the class spelling). This gate reads |
| - // rst- words in class values too, and select.js's mention of it |
| - // used to satisfy it; select.js went when the app moved off |
| - // rastrillo's frozen path (no page links it). |
| - "rst-sr-only": true, |
| - } |
| - |
| - attr := regexp.MustCompile(`rst-[a-z0-9-]+`) |
| + exempt := templateExempt |
| + attr := rstWord |
| // **Whole tokens, never substrings**, and this is not fussiness. The |
| // first version tested membership with strings.Contains, and |
| @@ -957,11 +951,7 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) { |
| // rst-mono, … rst-cell-mut"), memoria.css says `rst-cell-mut` written |
| // as an attribute selects nothing, and AGENTS.md says utilities stay |
| // classes. The gate said they were fine. |
| - selector := regexp.MustCompile(`\[(rst-[a-z0-9-]+)`) |
| - cssAttrs := map[string]bool{} |
| - for _, m := range selector.FindAllStringSubmatch(css, -1) { |
| - cssAttrs[m[1]] = true |
| - } |
| + cssAttrs := attrSelectors(css) |
| jsAttrs := tokens(js) |
| dir := filepath.Join("..", "memoria", "templates") |
| entries, err := os.ReadDir(dir) |
| @@ -977,11 +967,8 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - // Native details groups use name values, not styled attributes. |
| - // Mutation-checked 2026-09-11: rst-not-a-real-attribute still fails. |
| - markup := strings.ReplaceAll(string(raw), `name="rst-menus"`, `name="menu-group"`) |
| - for _, m := range attr.FindAllString(markup, -1) { |
| - if exempt[m] || unstyled[m] || seen[m] || jsAttrs[m] { |
| + for _, m := range attrNames(string(raw)) { |
| + if unstyled[m] || seen[m] || jsAttrs[m] { |
| continue |
| } |
| seen[m] = true |
| @@ -1099,3 +1086,114 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// rstWord is one rst- name, wherever it is written. |
| +var rstWord = regexp.MustCompile(`rst-[a-z0-9-]+`) |
| + |
| +// templateExempt names rst- words the design-system gate does not hold |
| +// to the stylesheets at all. |
| +var templateExempt = map[string]bool{ |
| + "rst-withheld": true, // a test hook for §6.5's notice block |
| +} |
| + |
| +// classOnlyUtilities are tokens.css utilities that keep the class |
| +// spelling and have no attribute twin ("Utilities keep the class |
| +// spelling and are not paired"). Inside a class="…" value they are |
| +// correct and the gate skips them; anywhere else they style nothing and |
| +// the gate reports them like any other unstyled name. This gate reads |
| +// rst- words in class values too, and select.js's mention of |
| +// rst-sr-only used to satisfy it; select.js went when the app moved off |
| +// rastrillo's frozen path (no page links it). |
| +var classOnlyUtilities = map[string]bool{ |
| + "rst-sr-only": true, |
| +} |
| + |
| +// classValue is one class attribute and its value, either quote. |
| +var classValue = regexp.MustCompile(`(\sclass=)("[^"]*"|'[^']*')`) |
| + |
| +// **Pre-existing attributes that style nothing and no script reads.** |
| +// See TestEveryTemplateAttributeIsInTheDesignSystem. |
| +var templateUnstyled = map[string]bool{ |
| + "rst-lede": true, |
| + "rst-steps": true, |
| + "rst-inline": true, |
| + "rst-bg": true, |
| +} |
| + |
| +// attrSelectors is the stylesheets' ATTRIBUTE vocabulary: the names |
| +// written as `[rst-…` selectors, never the class spelling. |
| +func attrSelectors(css string) map[string]bool { |
| + out := map[string]bool{} |
| + for _, m := range regexp.MustCompile(`\[(rst-[a-z0-9-]+)`).FindAllStringSubmatch(css, -1) { |
| + out[m[1]] = true |
| + } |
| + return out |
| +} |
| + |
| +// attrNames is every rst- name a template writes that the gate must |
| +// find in the stylesheets' attribute vocabulary, exemptions removed. |
| +func attrNames(markup string) []string { |
| + // Native details groups use name values, not styled attributes. |
| + // Mutation-checked 2026-09-11: rst-not-a-real-attribute still fails. |
| + markup = strings.ReplaceAll(markup, `name="rst-menus"`, `name="menu-group"`) |
| + // Class-only utilities leave the class values they belong in, and |
| + // only those: written as an attribute they are still scanned. |
| + markup = classValue.ReplaceAllStringFunc(markup, func(m string) string { |
| + return rstWord.ReplaceAllStringFunc(m, func(w string) string { |
| + if classOnlyUtilities[w] { |
| + return "" |
| + } |
| + return w |
| + }) |
| + }) |
| + var out []string |
| + for _, m := range rstWord.FindAllString(markup, -1) { |
| + if templateExempt[m] { |
| + continue |
| + } |
| + out = append(out, m) |
| + } |
| + return out |
| +} |
| + |
| +// A class-only utility is exempt as a CLASS and only as a class. |
| +// tokens.css styles .rst-sr-only and has no [rst-sr-only] twin, so |
| +// `<p rst-sr-only>` renders on screen — the very text the utility was |
| +// meant to hide. The exemption used to be a bare word, which waved that |
| +// through along with the class it was written for (Astra, memoria #24). |
| +func TestClassOnlyUtilityPassesOnlyAsAClass(t *testing.T) { |
| + cssAttrs := attrSelectors(designCSS(t)) |
| + if cssAttrs["rst-sr-only"] { |
| + t.Fatal("the stylesheets now select [rst-sr-only]; the attribute spelling is styled and this test's premise is gone") |
| + } |
| + unstyled := func(markup string) []string { |
| + var out []string |
| + for _, m := range attrNames(markup) { |
| + if !cssAttrs[m] && !templateUnstyled[m] { |
| + out = append(out, m) |
| + } |
| + } |
| + return out |
| + } |
| + for _, ok := range []string{ |
| + `<label for="q" class="rst-sr-only">Search</label>`, |
| + `<p class="rst-sr-only" role="status"></p>`, |
| + `<p class="hint rst-sr-only big">x</p>`, |
| + `<p class='rst-sr-only'>x</p>`, |
| + } { |
| + if got := unstyled(ok); len(got) != 0 { |
| + t.Errorf("%s: the gate rejects %v; a class-only utility written as a class is correct", ok, got) |
| + } |
| + } |
| + for _, bad := range []string{ |
| + `<p rst-sr-only>x</p>`, |
| + `<p rst-sr-only="">x</p>`, |
| + `<p rst-sr-only class="hint">x</p>`, |
| + `<p class="hint" rst-sr-only>x</p>`, |
| + `<p data-x="rst-sr-only">x</p>`, |
| + } { |
| + if got := unstyled(bad); len(got) != 1 || got[0] != "rst-sr-only" { |
| + t.Errorf("%s: the gate reports %v, want [rst-sr-only]: written as an attribute the utility styles nothing", bad, got) |
| + } |
| + } |
| +} |