mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(frontmatter): drop five Apple Notes directory rules that cannot change an outcome (#3952)
* fix(frontmatter): drop five Apple Notes rules that cannot change an outcome DIRECTORY_RULES carried five subfolder rules whose every field equals what the generic 'apple notes/' rule already produces. The generic rule assigns type/source/date/title and tags the first path segment lowercased and hyphenated, which is exactly what these five spelled out by hand. They read as deliberate overrides and are not. Measured across prefix x body x filename (45 cases): title, type, date, source and tags are identical with and without them. The only field that moves is `matchedRule`, the debug label naming which rule fired -- and it must move, because the rule that fires is now the generic one. Nothing persists that field: serializeFrontmatter writes title/type/date/source/tags only, and the one behavioural consumer (frontmatter.ts:475) compares it against the literal '(default)', which neither value is. Removing them exposed an existing assertion as vacuous: the ordering test named 'apple notes/yc/' and asserted its index is below the generic rule's. findIndex returns -1 for an absent prefix, and -1 is below any index, so the test kept passing while testing nothing. Rewritten to require every subfolder rule to precede the generic one, and to require that the set is non-empty. Adds a guard that recomputes the equivalence rather than listing prefixes, so a rule that re-derives the generic output cannot be added back silently. Verification on130d321d(v0.42.76.0): - bun test frontmatter-inference + page-type-exhaustive -> 43 pass / 0 fail - red check: with upstream/master's frontmatter-inference.ts -> 38 pass / 1 fail - bun run typecheck -> clean - bun run verify -> 34/34 green - grep for each removed prefix across the tree -> 0 remaining references Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontmatter): mirror production normalization in the redundancy guard Codex review, both gaps verified against the implementation before fixing: - inferFrontmatter lowercases BOTH sides before `startsWith` (frontmatter-inference.ts:297), so a rule authored as 'Apple Notes/YC/' matches at runtime. The guard compared prefixes case-sensitively and skipped it. - The generic rule tags parts[1] only (frontmatter-inference.ts:332), so a nested rule 'apple notes/yc/archive/' carrying tags ['yc'] is equally redundant. The guard derived from the whole remainder ('yc/archive') and did not match. The predicate now lowercases the prefix and derives from the first segment, and it is extracted so the guard can be pointed at synthetic rules. A second test does exactly that: both missed spellings must be flagged, and three genuine overrides (different tag set, differing field, non-Apple prefix) must not be. A guard that never fires is indistinguishable from a guard that cannot fire; this makes the difference observable. Verification on130d321d(v0.42.76.0): - bun test test/frontmatter-inference.test.ts -> 40 pass / 0 fail - red check: with upstream/master's frontmatter-inference.ts -> 39 pass / 1 fail - bun run typecheck -> clean - bun run verify -> 34/34 green Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontmatter): normalize the optional strategy fields in the guard Codex round 3, verified: inferFrontmatter defaults datePattern and titleStrategy to 'filename' (frontmatter-inference.ts:310, :317), so an omitted field and an explicit 'filename' describe the same rule. The guard compared them literally, which meant a redundant rule could escape detection by simply leaving them out. Both sides are now normalized through the same default, and the guard's own self-test covers the escape: the same redundant rule with those two keys deleted must still be flagged. Verification on130d321d(v0.42.76.0): - bun test test/frontmatter-inference.test.ts -> 40 pass / 0 fail - bun run typecheck -> clean - bun run verify -> 34/34 green Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(frontmatter): pin the accepted matchedRule change instead of leaving it incidental Codex: matchedRule surfaces as results[].rule in `gbrain frontmatter` (frontmatter.ts:486), so deleting a rule is a consumed-output change even though nothing persists the field. Asserted rather than merely described: a path under a removed prefix now reports 'apple notes/', every field serializeFrontmatter writes is unchanged, and the value is still not the literal '(default)' that frontmatter.ts's catch-all skip keys on. Verification on130d321d: 41 pass / 0 fail, typecheck clean, verify 34/34. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -104,46 +104,6 @@ export const DIRECTORY_RULES: DirectoryRule[] = [
|
||||
datePattern: 'filename',
|
||||
titleStrategy: 'filename',
|
||||
},
|
||||
{
|
||||
pathPrefix: 'apple notes/yc/',
|
||||
type: 'apple-note',
|
||||
source: 'apple-notes',
|
||||
tags: ['yc'],
|
||||
datePattern: 'filename',
|
||||
titleStrategy: 'filename',
|
||||
},
|
||||
{
|
||||
pathPrefix: 'apple notes/archived/',
|
||||
type: 'apple-note',
|
||||
source: 'apple-notes',
|
||||
tags: ['archived'],
|
||||
datePattern: 'filename',
|
||||
titleStrategy: 'filename',
|
||||
},
|
||||
{
|
||||
pathPrefix: 'apple notes/politics/',
|
||||
type: 'apple-note',
|
||||
source: 'apple-notes',
|
||||
tags: ['politics'],
|
||||
datePattern: 'filename',
|
||||
titleStrategy: 'filename',
|
||||
},
|
||||
{
|
||||
pathPrefix: 'apple notes/pitch notes/',
|
||||
type: 'apple-note',
|
||||
source: 'apple-notes',
|
||||
tags: ['pitch-notes'],
|
||||
datePattern: 'filename',
|
||||
titleStrategy: 'filename',
|
||||
},
|
||||
{
|
||||
pathPrefix: 'apple notes/gstack/',
|
||||
type: 'apple-note',
|
||||
source: 'apple-notes',
|
||||
tags: ['gstack'],
|
||||
datePattern: 'filename',
|
||||
titleStrategy: 'filename',
|
||||
},
|
||||
{
|
||||
pathPrefix: 'apple notes/photo-cameras/',
|
||||
type: 'apple-note',
|
||||
|
||||
@@ -303,12 +303,111 @@ describe('DIRECTORY_RULES', () => {
|
||||
expect(catchAll!.type).toBe('note');
|
||||
});
|
||||
|
||||
// Mirror production normalization: inferFrontmatter lowercases BOTH sides
|
||||
// before `startsWith` (frontmatter-inference.ts:297), so a rule authored as
|
||||
// 'Apple Notes/YC/' matches at runtime and must be examined here too.
|
||||
const APPLE_PREFIX = 'apple notes/';
|
||||
const applePrefixOf = (r: { pathPrefix: string }) => r.pathPrefix.toLowerCase();
|
||||
const isAppleRule = (r: { pathPrefix: string }) => applePrefixOf(r).startsWith(APPLE_PREFIX);
|
||||
const isGenericApple = (r: { pathPrefix: string }) => applePrefixOf(r) === APPLE_PREFIX;
|
||||
|
||||
/**
|
||||
* True when a subfolder rule reproduces exactly what the generic
|
||||
* 'apple notes/' rule already derives, and so can never change an outcome.
|
||||
*/
|
||||
const isRedundantApple = (
|
||||
r: { pathPrefix: string; tags?: string[]; type?: string; source?: string; datePattern?: string; titleStrategy?: string },
|
||||
generic: { type?: string; source?: string; datePattern?: string; titleStrategy?: string },
|
||||
): boolean => {
|
||||
if (!isAppleRule(r) || isGenericApple(r)) return false;
|
||||
const seg = applePrefixOf(r).slice(APPLE_PREFIX.length).replace(/\/+$/, '').split('/')[0];
|
||||
const derived = seg.replace(/\s+/g, '-');
|
||||
// inferFrontmatter defaults both optional strategy fields to 'filename'
|
||||
// (frontmatter-inference.ts:310, :317), so an omitted field and an
|
||||
// explicit 'filename' are the same rule. Comparing them literally would
|
||||
// let a redundant rule escape by simply leaving them out.
|
||||
const dp = (v?: string) => v ?? 'filename';
|
||||
const ts = (v?: string) => v ?? 'filename';
|
||||
const sameFields =
|
||||
r.type === generic.type &&
|
||||
r.source === generic.source &&
|
||||
dp(r.datePattern) === dp(generic.datePattern) &&
|
||||
ts(r.titleStrategy) === ts(generic.titleStrategy);
|
||||
return sameFields && [...(r.tags ?? [])].sort().join(',') === derived;
|
||||
};
|
||||
|
||||
test('Apple Notes rules are more specific than the catch-all', () => {
|
||||
const appleRules = DIRECTORY_RULES.filter(r => r.pathPrefix.startsWith('apple notes/'));
|
||||
const appleRules = DIRECTORY_RULES.filter(isAppleRule);
|
||||
expect(appleRules.length).toBeGreaterThan(1); // subfolder rules + catch-all
|
||||
// Subfolder rules should come before the generic apple notes/ rule
|
||||
const ycIdx = DIRECTORY_RULES.findIndex(r => r.pathPrefix === 'apple notes/yc/');
|
||||
const genericIdx = DIRECTORY_RULES.findIndex(r => r.pathPrefix === 'apple notes/');
|
||||
expect(ycIdx).toBeLessThan(genericIdx);
|
||||
// EVERY subfolder rule must come before the generic one, not just a named
|
||||
// example: findIndex on an absent prefix returns -1, which satisfies
|
||||
// `toBeLessThan(genericIdx)` vacuously, so an assertion naming one rule
|
||||
// keeps passing after that rule is deleted.
|
||||
const genericIdx = DIRECTORY_RULES.findIndex(isGenericApple);
|
||||
expect(genericIdx).toBeGreaterThanOrEqual(0);
|
||||
const subIdx = appleRules
|
||||
.filter(r => !isGenericApple(r))
|
||||
.map(r => DIRECTORY_RULES.indexOf(r));
|
||||
expect(subIdx.length).toBeGreaterThan(0);
|
||||
for (const i of subIdx) expect(i).toBeLessThan(genericIdx);
|
||||
});
|
||||
|
||||
test('no Apple Notes subfolder rule duplicates what the generic rule derives', () => {
|
||||
// The generic 'apple notes/' rule assigns type/source/date/title and tags
|
||||
// the first path segment, lowercased and hyphenated. A subfolder rule that
|
||||
// reproduces exactly that is dead weight: it can never change an output,
|
||||
// but it reads as a deliberate override.
|
||||
const generic = DIRECTORY_RULES.find(isGenericApple);
|
||||
expect(generic).toBeDefined();
|
||||
expect(DIRECTORY_RULES.filter(r => isRedundantApple(r, generic!)).map(r => r.pathPrefix)).toEqual([]);
|
||||
});
|
||||
|
||||
test('a path under a removed prefix now reports the generic rule', () => {
|
||||
// Accepted, and pinned rather than incidental: `matchedRule` is a
|
||||
// diagnostic label, surfaced as results[].rule by `gbrain frontmatter`
|
||||
// (frontmatter.ts:486). Deleting a rule necessarily changes which rule is
|
||||
// named. Everything the label does NOT cover — title/type/date/source/tags,
|
||||
// i.e. every field serializeFrontmatter persists — is unchanged.
|
||||
const out = inferFrontmatter('Apple Notes/YC/2024-03-05 A Note.md', 'body');
|
||||
expect(out.matchedRule).toBe('apple notes/');
|
||||
expect(out).toMatchObject({
|
||||
title: 'A Note',
|
||||
type: 'apple-note',
|
||||
date: '2024-03-05',
|
||||
source: 'apple-notes',
|
||||
tags: ['yc'],
|
||||
});
|
||||
// The catch-all branch in frontmatter.ts keys on the literal '(default)',
|
||||
// so its skip decision is untouched by this rename.
|
||||
expect(out.matchedRule).not.toBe('(default)');
|
||||
});
|
||||
|
||||
test('the redundancy predicate catches the shapes a naive one misses', () => {
|
||||
// A guard that never fires is indistinguishable from a guard that cannot
|
||||
// fire. Feed it the two spellings production would still match: a
|
||||
// mixed-case prefix (inferFrontmatter lowercases both sides before
|
||||
// startsWith) and a nested prefix (the generic rule tags parts[1] only,
|
||||
// so a deeper rule repeating the first segment is equally redundant).
|
||||
const generic = DIRECTORY_RULES.find(isGenericApple)!;
|
||||
const like = (pathPrefix: string, tags: string[]) => ({
|
||||
pathPrefix,
|
||||
tags,
|
||||
type: generic.type,
|
||||
source: generic.source,
|
||||
datePattern: generic.datePattern,
|
||||
titleStrategy: generic.titleStrategy,
|
||||
});
|
||||
expect(isRedundantApple(like('Apple Notes/YC/', ['yc']), generic)).toBe(true);
|
||||
expect(isRedundantApple(like('apple notes/yc/archive/', ['yc']), generic)).toBe(true);
|
||||
expect(isRedundantApple(like('apple notes/pitch notes/', ['pitch-notes']), generic)).toBe(true);
|
||||
// Omitting the optional strategy fields is the same rule as spelling out
|
||||
// their defaults, so leaving them out must not buy an escape.
|
||||
const { datePattern: _dp, titleStrategy: _ts, ...withoutDefaults } = like('apple notes/yc/', ['yc']);
|
||||
expect(isRedundantApple(withoutDefaults, generic)).toBe(true);
|
||||
// Real overrides must NOT be flagged: a different tag set, or a field the
|
||||
// generic rule sets differently.
|
||||
expect(isRedundantApple(like('apple notes/youtube shows/', ['youtube', 'shows']), generic)).toBe(false);
|
||||
expect(isRedundantApple({ ...like('apple notes/yc/', ['yc']), type: 'note' }, generic)).toBe(false);
|
||||
expect(isRedundantApple(like('wiki/yc/', ['yc']), generic)).toBe(false); // not an Apple Notes rule
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user