mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(effective-date): rank content_created_at with its provenance — stop reindex flipping deliberate date fields
Review fixes for the #2754 takeover: 1. content_created_at was ranked ABOVE event_date/date/published/filename while both in-tree writers (import-file, the backfill script) derive it from frontmatter.created — the chain's explicitly WEAKEST content signal. Import computed source='date' for a created+date page, then the first 'gbrain reindex-frontmatter' (which reads the persisted column) silently flipped effective_date to the created value. The column now ranks just above the frontmatter.created rung (still repairs the incident class: created-only pages beat the row-timestamp fallback), and import passes contentCreatedAt through so import and reindex compute identical results. Regression test added. 2. ESCAPE '\\' (2-char escape string) made every --slug-prefix run die with "invalid escape string" — in the new backfill script and the pre-existing copy in backfill-effective-date.ts. Both now send a single-char escape. Verified against PGLite. 3. Scrubbed a personal-brain entity slug from test/effective-date.test.ts per the repo privacy rule (generic placeholder instead). typecheck clean; 412 tests across the touched suites pass; jsonb guards clean; end-to-end PGLite smoke (fresh bootstrap + v125 migration + script step 1 with --slug-prefix + effective_date re-walk) verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
85389d37db
commit
19a291e897
@@ -75,7 +75,10 @@ export async function backfillContentCreatedAt(
|
||||
const slugPrefix = opts.slugPrefix?.replace(/[\\%_]/g, (c) => '\\' + c) ?? null;
|
||||
|
||||
while (true) {
|
||||
const slugFilter = slugPrefix ? `AND slug LIKE $2 ESCAPE '\\\\'` : '';
|
||||
// NOTE: ESCAPE takes a single character. In this TS source that's `'\\'`
|
||||
// (one backslash at runtime) — the doubled `'\\\\'` form sends a 2-char
|
||||
// escape string and Postgres rejects it with "invalid escape string".
|
||||
const slugFilter = slugPrefix ? `AND slug LIKE $2 ESCAPE '\\'` : '';
|
||||
const params: unknown[] = [lastId];
|
||||
if (slugPrefix) params.push(slugPrefix + '%');
|
||||
|
||||
|
||||
@@ -152,8 +152,12 @@ export async function backfillEffectiveDate(
|
||||
|
||||
// Keyset pagination: WHERE id > last_id ORDER BY id LIMIT N. Single-direction
|
||||
// walk; safe under concurrent inserts (new rows show up at the tail).
|
||||
// NOTE: ESCAPE takes a single character. In this TS source that's `'\\'`
|
||||
// (one backslash at runtime) — the doubled `'\\\\'` form sent a 2-char
|
||||
// escape string and every --slug-prefix run died with "invalid escape
|
||||
// string" (fixed in the tasks-41o takeover; PR #3168 review).
|
||||
const slugFilter = slugPrefix
|
||||
? `AND slug LIKE $2 ESCAPE '\\\\'`
|
||||
? `AND slug LIKE $2 ESCAPE '\\'`
|
||||
: '';
|
||||
const params: unknown[] = [lastId];
|
||||
if (slugPrefix) params.push(slugPrefix + '%');
|
||||
@@ -198,7 +202,7 @@ export async function backfillEffectiveDate(
|
||||
// tasks-41o: once scripts/backfill-content-created-at.ts has
|
||||
// populated the column, this re-walk (`gbrain reindex-frontmatter`)
|
||||
// is what actually flips effective_date/effective_date_source for
|
||||
// existing rows — content_created_at is the top-priority candidate.
|
||||
// existing rows — the column ranks just above the 'created' rung.
|
||||
contentCreatedAt: r.content_created_at ? new Date(r.content_created_at) : null,
|
||||
});
|
||||
|
||||
|
||||
+26
-27
@@ -7,23 +7,23 @@
|
||||
* is the row insert time). It's the user's stated content date.
|
||||
*
|
||||
* Precedence chain (default order):
|
||||
* 0. content_created_at — explicit, authoritative content-date
|
||||
* override (backfilled or importer-set;
|
||||
* see PageInput.content_created_at).
|
||||
* Ranked above frontmatter.event_date/
|
||||
* date/published because it's the one
|
||||
* signal specifically designed to correct
|
||||
* a wrong row-timestamp fallback — it's
|
||||
* NULL for the vast majority of pages
|
||||
* (those keep their existing behavior
|
||||
* unchanged) and only populated when we
|
||||
* know for certain when the content was
|
||||
* created.
|
||||
* 1. frontmatter.event_date — meeting / event pages
|
||||
* 2. frontmatter.date — dated essays
|
||||
* 3. frontmatter.published — writing/
|
||||
* 4. filename-date — leading YYYY-MM-DD in basename
|
||||
* 5. frontmatter.created — generic "content created" signal (e.g.
|
||||
* 5. content_created_at — the pages.content_created_at column (see
|
||||
* PageInput.content_created_at). Both
|
||||
* in-tree writers derive it from
|
||||
* frontmatter.created, so it ranks WITH
|
||||
* that signal — just above it, because a
|
||||
* persisted column survives a later
|
||||
* frontmatter edit that drops the key.
|
||||
* Ranking it above event_date/date/
|
||||
* published would let a generic `created`
|
||||
* stamp silently flip effective_date on
|
||||
* every reindex of a page that also
|
||||
* carries a deliberate date field.
|
||||
* 6. frontmatter.created — generic "content created" signal (e.g.
|
||||
* wiki/entities/ pages with no
|
||||
* event_date/date/published/filename
|
||||
* date). Ranked below the dedicated
|
||||
@@ -32,12 +32,11 @@
|
||||
* signals; `created` is a broader,
|
||||
* weaker one used as a last resort before
|
||||
* falling back to row bookkeeping.
|
||||
* 6. updated_at — fallback
|
||||
* 7. created_at — last resort (only if updated_at NULL)
|
||||
* 7. updated_at — fallback
|
||||
* 8. created_at — last resort (only if updated_at NULL)
|
||||
*
|
||||
* Per-prefix override: for `daily/` and `meetings/` slug prefixes, the
|
||||
* filename-date jumps ahead of frontmatter.event_date/date/published (but
|
||||
* stays below content_created_at) — the filename is the user's primary
|
||||
* filename-date jumps to position 1 — the filename is the user's primary
|
||||
* signal there ("daily/2024-03-15.md" the FILE date matters more than any
|
||||
* frontmatter the user pasted).
|
||||
*
|
||||
@@ -68,9 +67,9 @@ export interface ComputeEffectiveDateOpts {
|
||||
createdAt: Date;
|
||||
/**
|
||||
* tasks-41o: the page's `content_created_at` column value (or an
|
||||
* importer-computed override before the row exists). Highest-priority
|
||||
* precedence candidate — see the module doc comment. Optional/undefined
|
||||
* for pre-migration callers; treated the same as null.
|
||||
* importer-computed value before the row exists). Ranks just above the
|
||||
* frontmatter.created rung — see the module doc comment. Optional/
|
||||
* undefined for pre-migration callers; treated the same as null.
|
||||
*/
|
||||
contentCreatedAt?: Date | null;
|
||||
}
|
||||
@@ -151,9 +150,10 @@ export function computeEffectiveDate(opts: ComputeEffectiveDateOpts): EffectiveD
|
||||
const { slug, frontmatter, filename, updatedAt, createdAt, contentCreatedAt } = opts;
|
||||
const filenameFirst = hasFilenameFirstPrefix(slug);
|
||||
|
||||
// tasks-41o: explicit, authoritative content-date override. Checked FIRST,
|
||||
// ahead of every frontmatter/filename candidate below — see the module doc
|
||||
// comment for why this is safe (NULL for the vast majority of pages).
|
||||
// tasks-41o: the persisted pages.content_created_at column. Derived from
|
||||
// frontmatter.created by every in-tree writer, so it ranks WITH that
|
||||
// signal (just above it — the column survives a frontmatter edit that
|
||||
// drops the key). See the module doc comment.
|
||||
const contentCreated = validateInRange(contentCreatedAt ?? null);
|
||||
|
||||
const fmEvent = validateInRange(parseDateLoose(frontmatter.event_date));
|
||||
@@ -166,23 +166,22 @@ export function computeEffectiveDate(opts: ComputeEffectiveDateOpts): EffectiveD
|
||||
const fmCreated = validateInRange(parseDateLoose(frontmatter.created));
|
||||
|
||||
// Build the ordered candidate list. For filename-first prefixes
|
||||
// (daily/, meetings/) the filename moves ahead of the frontmatter date
|
||||
// fields (but stays below content_created_at).
|
||||
// (daily/, meetings/) the filename moves to the head of the chain.
|
||||
const candidates: Array<{ date: Date | null; source: EffectiveDateSource }> = filenameFirst
|
||||
? [
|
||||
{ date: contentCreated, source: 'content_created_at' },
|
||||
{ date: filenameDate, source: 'filename' },
|
||||
{ date: fmEvent, source: 'event_date' },
|
||||
{ date: fmDate, source: 'date' },
|
||||
{ date: fmPublished, source: 'published' },
|
||||
{ date: contentCreated, source: 'content_created_at' },
|
||||
{ date: fmCreated, source: 'created' },
|
||||
]
|
||||
: [
|
||||
{ date: contentCreated, source: 'content_created_at' },
|
||||
{ date: fmEvent, source: 'event_date' },
|
||||
{ date: fmDate, source: 'date' },
|
||||
{ date: fmPublished, source: 'published' },
|
||||
{ date: filenameDate, source: 'filename' },
|
||||
{ date: contentCreated, source: 'content_created_at' },
|
||||
{ date: fmCreated, source: 'created' },
|
||||
];
|
||||
|
||||
|
||||
+9
-14
@@ -754,20 +754,14 @@ export async function importFromContent(
|
||||
const nowDate = new Date();
|
||||
// tasks-41o — thread frontmatter.created forward so it PERSISTS on the
|
||||
// row as content_created_at (distinct from created_at, the row-insert
|
||||
// time; this NEVER touches created_at). Deliberately NOT passed as
|
||||
// computeEffectiveDate's `contentCreatedAt` opt here — that opt is the
|
||||
// top-priority explicit-override slot (reserved for an already-known,
|
||||
// authoritative content date, e.g. a prior backfill), whereas
|
||||
// frontmatter.created on a fresh import is the same generic, lower-
|
||||
// priority signal computeEffectiveDate already reads via the full
|
||||
// `frontmatter` object passed below (see effective-date.ts's 'created'
|
||||
// rung). Passing it as BOTH would wrongly promote it above
|
||||
// event_date/date/published for every import that happens to set
|
||||
// `created`. `existing` doesn't carry content_created_at
|
||||
// (engine.getPage's SELECT doesn't project it, same as effective_date),
|
||||
// so there's nothing to preserve at this layer — putPage's
|
||||
// COALESCE-preserve UPDATE keeps any prior column value when this
|
||||
// frontmatter has no `created` key.
|
||||
// time; this NEVER touches created_at). Passed as contentCreatedAt too
|
||||
// so import and the reindex re-walk (backfill-effective-date.ts, which
|
||||
// reads the column) compute the same result — the opt ranks just above
|
||||
// the 'created' rung, never above event_date/date/published/filename.
|
||||
// `existing` doesn't carry content_created_at (engine.getPage's SELECT
|
||||
// doesn't project it, same as effective_date); putPage's COALESCE-
|
||||
// preserve UPDATE keeps any prior column value when this frontmatter
|
||||
// has no `created` key.
|
||||
const contentCreatedAt = parseDateLoose(parsed.frontmatter.created);
|
||||
const { date: effectiveDate, source: effectiveDateSource } = computeEffectiveDate({
|
||||
slug,
|
||||
@@ -775,6 +769,7 @@ export async function importFromContent(
|
||||
filename: filenameForChain,
|
||||
updatedAt: existing?.updated_at ?? nowDate,
|
||||
createdAt: existing?.created_at ?? nowDate,
|
||||
contentCreatedAt,
|
||||
});
|
||||
|
||||
await tx.putPage(slug, {
|
||||
|
||||
+2
-2
@@ -5688,8 +5688,8 @@ export const MIGRATIONS: Migration[] = [
|
||||
// backfill inside the migration itself (see scripts/backfill-content-
|
||||
// created-at.ts — a separate, re-runnable, idempotent pass so a bad
|
||||
// backfill can be re-run without a schema round-trip). Consulted by
|
||||
// computeEffectiveDate (src/core/effective-date.ts) as the
|
||||
// highest-priority precedence candidate, and settable via
|
||||
// computeEffectiveDate (src/core/effective-date.ts) just above the
|
||||
// frontmatter.created rung, and settable via
|
||||
// PageInput.content_created_at on putPage. Mirrored in src/schema.sql,
|
||||
// src/core/pglite-schema.ts, and the generated src/core/schema-embedded.ts
|
||||
// so fresh installs carry the same column.
|
||||
|
||||
@@ -97,8 +97,8 @@ CREATE TABLE IF NOT EXISTS pages (
|
||||
import_filename TEXT,
|
||||
salience_touched_at TIMESTAMPTZ,
|
||||
-- tasks-41o (migration v125): the content's own creation date -- distinct
|
||||
-- from created_at (the row-insert time; never overwritten). Top-priority
|
||||
-- candidate in computeEffectiveDate's precedence chain. Mirrors src/schema.sql.
|
||||
-- from created_at (the row-insert time; never overwritten). Ranks just above
|
||||
-- the frontmatter.created rung in computeEffectiveDate. Mirrors src/schema.sql.
|
||||
content_created_at TIMESTAMPTZ,
|
||||
-- v0.37.0 (migration v79): real stale-page signal for gbrain lsd
|
||||
-- (mirrors src/schema.sql). NULL = never retrieved.
|
||||
|
||||
@@ -126,8 +126,8 @@ CREATE TABLE IF NOT EXISTS pages (
|
||||
-- tasks-41o (migration v125): the content's own creation date, distinct
|
||||
-- from created_at (row-insert time, never overwritten). NULL unless
|
||||
-- explicitly supplied (frontmatter \`created\`, backfilled from history, or
|
||||
-- caller override via PageInput.content_created_at). Highest-priority
|
||||
-- candidate in computeEffectiveDate's precedence chain.
|
||||
-- caller override via PageInput.content_created_at). Ranks just above the
|
||||
-- frontmatter.created rung in computeEffectiveDate's precedence chain.
|
||||
content_created_at TIMESTAMPTZ,
|
||||
-- v0.37.0 (migration v79): real stale-page signal for \`gbrain lsd\`. Bumped
|
||||
-- by op-layer write-back inside \`search\`/\`query\`/\`get_page\` op handlers
|
||||
|
||||
+5
-5
@@ -127,9 +127,9 @@ export interface Page {
|
||||
* tasks-41o (migration v125): the content's own creation date — distinct
|
||||
* from `created_at` (row-insert time, never overwritten by this field).
|
||||
* NULL unless explicitly supplied: frontmatter `created`, a backfill from
|
||||
* git/mtime history, or an importer/caller override. Highest-priority
|
||||
* candidate in `computeEffectiveDate`'s precedence chain — see
|
||||
* `src/core/effective-date.ts`.
|
||||
* git/mtime history, or an importer/caller override. Ranks just above the
|
||||
* frontmatter.created rung in `computeEffectiveDate`'s precedence chain —
|
||||
* see `src/core/effective-date.ts`.
|
||||
*/
|
||||
content_created_at?: Date | null;
|
||||
/**
|
||||
@@ -250,8 +250,8 @@ export interface PageInput {
|
||||
* `created_at` and never lets this field overwrite `created_at`). When
|
||||
* omitted, putPage leaves the column unchanged on conflict (preserves any
|
||||
* existing value, same COALESCE-preserve shape as effective_date); on
|
||||
* insert the column is NULL. Consulted by `computeEffectiveDate` as the
|
||||
* highest-priority precedence candidate.
|
||||
* insert the column is NULL. Consulted by `computeEffectiveDate` just
|
||||
* above the frontmatter.created rung.
|
||||
*/
|
||||
content_created_at?: Date | null;
|
||||
/**
|
||||
|
||||
+2
-2
@@ -122,8 +122,8 @@ CREATE TABLE IF NOT EXISTS pages (
|
||||
-- tasks-41o (migration v125): the content's own creation date, distinct
|
||||
-- from created_at (row-insert time, never overwritten). NULL unless
|
||||
-- explicitly supplied (frontmatter `created`, backfilled from history, or
|
||||
-- caller override via PageInput.content_created_at). Highest-priority
|
||||
-- candidate in computeEffectiveDate's precedence chain.
|
||||
-- caller override via PageInput.content_created_at). Ranks just above the
|
||||
-- frontmatter.created rung in computeEffectiveDate's precedence chain.
|
||||
content_created_at TIMESTAMPTZ,
|
||||
-- v0.37.0 (migration v79): real stale-page signal for `gbrain lsd`. Bumped
|
||||
-- by op-layer write-back inside `search`/`query`/`get_page` op handlers
|
||||
|
||||
@@ -176,27 +176,51 @@ describe('computeEffectiveDate range validation [1990, NOW + 1y]', () => {
|
||||
// real content date. content_created_at (the explicit override) and
|
||||
// frontmatter.created (the generic signal) close it.
|
||||
describe('computeEffectiveDate content_created_at / frontmatter.created (tasks-41o)', () => {
|
||||
test('content_created_at wins over everything, including event_date', () => {
|
||||
test('content_created_at loses to deliberate date fields (event_date) — created-derived, must not flip them on reindex', () => {
|
||||
const r = run({
|
||||
fm: { event_date: '2024-04-01' },
|
||||
contentCreatedAt: new Date('2026-05-14T00:00:00Z'),
|
||||
});
|
||||
expect(r.source).toBe('event_date');
|
||||
expect(r.date?.toISOString().startsWith('2024-04-01')).toBe(true);
|
||||
});
|
||||
|
||||
test('content_created_at loses to daily/meetings filename-first override', () => {
|
||||
const r = run({
|
||||
slug: 'meetings/2024-06-15-acme-call',
|
||||
filename: '2024-06-15-acme-call',
|
||||
contentCreatedAt: new Date('2026-05-14T00:00:00Z'),
|
||||
});
|
||||
expect(r.source).toBe('filename');
|
||||
});
|
||||
|
||||
test('content_created_at wins over frontmatter.created and the updated_at/created_at fallback', () => {
|
||||
const r = run({
|
||||
fm: { created: '2024-04-01' },
|
||||
contentCreatedAt: new Date('2026-05-14T00:00:00Z'),
|
||||
});
|
||||
expect(r.source).toBe('content_created_at');
|
||||
expect(r.date?.toISOString().startsWith('2026-05-14')).toBe(true);
|
||||
});
|
||||
|
||||
test('content_created_at wins over daily/meetings filename-first override', () => {
|
||||
const r = run({
|
||||
slug: 'meetings/2024-06-15-acme-call',
|
||||
filename: '2024-06-15-acme-call',
|
||||
contentCreatedAt: new Date('2026-05-14T00:00:00Z'),
|
||||
test('import and reindex agree: created + date page keeps source=date whether or not the column is populated', () => {
|
||||
// Regression for the reindex-flip bug: import computed 'date' while the
|
||||
// re-walk (which reads the persisted column) computed content_created_at
|
||||
// as top priority, silently flipping effective_date on the first
|
||||
// `gbrain reindex-frontmatter` run.
|
||||
const withoutColumn = run({ fm: { created: '2026-01-01', date: '2024-04-01' } });
|
||||
const withColumn = run({
|
||||
fm: { created: '2026-01-01', date: '2024-04-01' },
|
||||
contentCreatedAt: new Date('2026-01-01T00:00:00Z'),
|
||||
});
|
||||
expect(r.source).toBe('content_created_at');
|
||||
expect(withoutColumn.source).toBe('date');
|
||||
expect(withColumn.source).toBe('date');
|
||||
expect(withColumn.date?.getTime()).toBe(withoutColumn.date?.getTime());
|
||||
});
|
||||
|
||||
test('frontmatter.created wins when no event_date/date/published/filename present (couples-counseling case)', () => {
|
||||
test('frontmatter.created wins when no event_date/date/published/filename present (entity-page case)', () => {
|
||||
const r = run({
|
||||
slug: 'wiki/entities/couples-counseling-vishnu-and-silas',
|
||||
slug: 'wiki/entities/alice-and-charlie-example',
|
||||
fm: { created: '2026-05-14' },
|
||||
});
|
||||
expect(r.source).toBe('created');
|
||||
|
||||
Reference in New Issue
Block a user