feat: filter the admin content list by byline

[MA2153]

What does this PR do?

Adds byline filtering to the admin content list, alongside the existing status, author, and date filters. Selecting one or more bylines matches entries credited to any of them (OR); a No byline assigned option matches entries with no credit at all.

Bylines are searched server-side in the picker rather than listed exhaustively, so the filter works across the whole byline directory rather than one page.

Inferred bylines are excluded by default. An entry with no explicit credit still renders the byline linked to its author (see hydrateBylinesMany), but filtering usually means "who is credited", not "whose name happens to show". A single Include inferred bylines switch opts into the wider behaviour, and it widens consistently: with it on, "No byline assigned" means nothing is rendered, so entries whose author resolves to a byline drop out too.

Filter values are translation_groups — what _emdash_content_bylines.byline_id has stored since migration 040 — so a selection matches a byline across every locale it exists in.

Proposed in Discussion #2347 (Ideas), now maintainer-approved.

Discussion: #2347

Credits resolve at the list's locale

Byline hydration is strict per locale: a credit renders only where its translation_group has a byline row at the locale the list is showing. Both branches of the filter originally ignored that, in the same way and with the same symptom — an entry the list renders as uncredited matching a filter on a byline, and matching No byline assigned at the same time. The two branches were fixed one review round apart:

• Inferred credits. The branch matched on author_id alone, against a set of users resolved from every locale row of the selected bylines. An entry whose author owns a byline with no row at the list's locale displays as uncredited but still matched a filter on it.
• Explicit credits (pullrequestreview-4890821647). The junction stores a group and copyContentBylines propagates junction rows to every translation of an entry, so a group that exists only in en still matched bylines=[group] on the fr list — where the credit renders nothing.

Both now resolve the credit the way the list renders it: the explicit branch repeats the _emdash_bylines join getContentBylinesMany makes, the inferred branch a correlated EXISTS on the same table, each scoped to the list's locale and falling back to each entry's own when the list spans locales. Byline translations start life with a null user_id, so a group translated into a locale but not re-linked correctly resolves to no credit.

One asymmetry is deliberate and load-bearing: whether a credit renders is locale-scoped, whether one exists is not. hydrateBylinesMany suppresses the author fallback on the presence of a junction row, not on one that resolves at this locale, so the inference gate keeps a separate locale-agnostic probe. Without it, an entry whose explicit credit fails to resolve would fall through to its author and match a filter on a byline the list doesn't show against it.

The inferred fix also removes a query: the handler no longer pre-resolves author ids for includeInferredBylines, so the opt-in costs no extra round-trip.

No migration required

The plan is now pinned by a committed test (content-list-byline-filter-plan.test.ts) rather than measured by hand, so it can't drift silently — the assertions run EXPLAIN QUERY PLAN over the SQL the repository actually emits, against a freshly-migrated stats-blind DB (no ANALYZE, matching D1). The UNIQUE(collection_slug, content_id, byline_id) constraint from migration 031 already creates an index of exactly the right shape, and the locale scoping lands on the existing byline uniques:

### include (EXISTS ... byline_id IN (...))
SEARCH ec_posts USING INDEX idx_ec_posts_loc_crt (deleted_at=? AND locale=?)
SEARCH cb USING INDEX sqlite_autoindex__emdash_content_bylines_2
    (collection_slug=? AND content_id=? AND byline_id=?)
SEARCH b  USING COVERING INDEX idx_bylines_group_locale_unique (translation_group=? AND locale=?)

### no-byline (NOT EXISTS)
SEARCH cb USING INDEX idx_content_bylines_content (collection_slug=? AND content_id=?)
SEARCH b  USING COVERING INDEX idx_bylines_group_locale_unique (translation_group=? AND locale=?)

### inferred opt-in (selected bylines)
SEARCH cb USING INDEX sqlite_autoindex__emdash_content_bylines_2 (...)
SEARCH b  USING COVERING INDEX idx_bylines_group_locale_unique (translation_group=? AND locale=?)
SEARCH cb USING INDEX idx_content_bylines_content (collection_slug=? AND content_id=?)
SEARCH b  USING INDEX idx_bylines_group_locale_unique (translation_group=? AND locale=?)

### no-byline + inferred opt-in
SEARCH cb USING INDEX idx_content_bylines_content (collection_slug=? AND content_id=?)
SEARCH b  USING COVERING INDEX idx_bylines_group_locale_unique (translation_group=? AND locale=?)
SEARCH cb USING INDEX idx_content_bylines_content (collection_slug=? AND content_id=?)
SEARCH b  USING INDEX idx_bylines_user_id_locale_unique (user_id=? AND locale=?)

Every probe is an indexed seek, and the outer query keeps its sort-ordered composite index in all four shapes, so LIMIT still short-circuits with no USE TEMP B-TREE FOR ORDER BY. Scoping the explicit branch to the locale added a join inside the EXISTS and no round-trips: the byline row is reached through the (translation_group, locale) unique, covering in three of the four shapes.

Two consequences are worth spelling out, since neither is visible from the query alone. The full reasoning lives here; the code comment states only the invariants a future reader needs:

• The correlated EXISTS shape matters. Driving from the pivot side (FROM _emdash_content_bylines JOIN ec_*) cannot use that index for the byline and adds USE TEMP B-TREE FOR ORDER BY. Written as an EXISTS from the content table, no new index is needed; written the other way, no index rescues it.
• "No byline" tests the junction, not primary_byline_id. The two agree — both junction write paths stamp the column in the same call — but not atomically (D1 has no transactions), so the junction stays authoritative, mirroring how migration 051 treats the denormalized taxonomy columns as advisory and re-checks on read.

Known limitation, deliberately not addressed: the EXISTS plan walks the collection's sort index and probes per row, so a byline matching very few entries in a very large collection reads a lot before filling LIMIT — the shape #1834/migration 051 fixed for taxonomies. Making that seek-optimal needs denormalization, i.e. a migration. This is the authenticated admin list rather than the logged-out hot path, so it didn't seem worth paying now. Happy to revisit.

No queries were added to any logged-out route.

Type of change

• Bug fix
• Feature (requires maintainer-approved Discussion)
• Refactor (no behavior change)
• Translation
• Documentation
• Performance improvement
• Tests
• Chore (dependencies, CI, tooling)

Checklist

• I have read CONTRIBUTING.md
• pnpm typecheck passes
• pnpm lint passes
• pnpm test passes (or targeted tests for my change)
• pnpm format has been run
• I have added/updated tests for my changes (if applicable)
• User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main.
• I have added a changeset (if this PR changes a published package)
• New features link to an approved Discussion — Filter the admin content list by byline #2347, now maintainer-approved.

AI-generated code disclosure

• This PR includes AI-generated code — model/tool: Claude Opus 5 (Claude Code)

Screenshots / test output

Rebased onto main (currently 215f36eb); the only conflict was an import block in ContentList.tsx that #2340 and this branch both added to, resolved by keeping both.

Verified in the browser against demos/simple: the filter renders in the existing filter bar as an "All bylines" dropdown containing a search box, the exclusive "No byline assigned" checkbox, the byline list, and the "Include inferred bylines" switch.

Also checked in Arabic per the RTL guidance — document.documentElement.dir is rtl, and the popover, checkboxes, switch, and caret all mirror correctly with no broken directionality. The selected-byline count goes through Lingui's plural rather than a bare interpolation, which only reads correctly in languages with one plural form. (New strings render in English until the extraction workflow picks them up on merge; no messages.po changes are included here.)

Screenshots can be attached on request — omitted here since I can't upload to GitHub's CDN from the CLI.

Tests

14 integration tests (content-list-byline-filter.test.ts), run against both dialects via describeEachDialect, covering single/multi-byline OR matching, the no-byline filter, inferred credits on and off, an explicit credit suppressing author inference, credits resolving at the locale the list is scoped to — explicit and inferred, composition with the status filter, and total reflecting the filter.

Plus 5 query-plan tests (content-list-byline-filter-plan.test.ts, SQLite-only) asserting the plan quoted above: an indexed seek for every probe in all four filter shapes, and no USE TEMP B-TREE FOR ORDER BY.

Four of them caught real bugs during development:

• The empty-selection guard used eb.val(false), which better-sqlite3 refuses to bind (SQLite3 can only bind numbers, strings, bigints, buffers, and null). It now emits a literal 1 = 0 predicate instead.
• The inferred-credit locale disagreement — an entry matching both a byline filter and "No byline assigned" at once.
• The same disagreement for explicit credits, from pullrequestreview-4890821647: an fr entry credited to an en-only byline group matched bylines=[group] while rendering uncredited.
• The follow-on: with the credit test locale-scoped, an entry whose explicit credit fails to resolve fell through to author inference, which hydration does not do. Written first as a failing test, and it did fail before the gate was split.

Behaviour verified end-to-end against demos/simple, filtering a 16-entry collection:

Query	total
no filter	16
bylines=<A>	4
bylines=<A>,<B>	6
bylines=none	10
bylines=<unknown-id>	0
bylines=<A>&status=published	4

6 + 10 = 16 — the include and no-byline filters partition the collection exactly.

Wire contract:

Input	Result
includeInferredBylines=1 / true / 0 / false	accepted, parsed strictly
includeInferredBylines=yes	400 VALIDATION_ERROR
26 byline ids	400 — "at most 25 bylines may be selected"
bylines= (empty)	400 VALIDATION_ERROR
duplicate ids	collapsed

The 25-id cap keeps the IN (...) clause clear of D1's bound-parameter ceiling once the rest of the list query's placeholders are counted.

Review responses

pullrequestreview-4867919957

Over-narrative doc comment on applyBylineFilter — fixed in 10f415d4. Agreed, and taken essentially as suggested. The comment now states only the two invariants: that the EXISTS correlates from the content table so the outer sort index survives, and that mode: "none" tests the junction rather than primary_byline_id because the two aren't written atomically. The rejected pivot-side shape, the EXPLAIN QUERY PLAN output, and the migration 051 precedent stay in this description, where they don't go stale in the source tree. The paragraph above that introduced them has been reworded, since it claimed those details were recorded in comments.

Discussion approval. Correct — #2347 is still unapproved and the Discussion checklist item is unticked, so this shouldn't merge until a maintainer weighs in on the direction. Earlier revisions of this description said the PR "stays a draft"; that was inaccurate, it is open for review rather than in draft state. Corrected above.

pullrequestreview-4868375437

Justification sentence in the ContentBylineFilter docstring — fixed in 763e417b. Taken as suggested; the docstring now ends at what includeInferred does, and the rationale for the default lives in this description.

A request on review process. This is the second review that confirms the approach, confirms the tests, confirms the query counts, and then holds approval for a single sentence in a comment. Both sentences were in the diff at the time of the first review — the second one wasn't introduced by the fix for the first. Reviewing one comment per round turns a two-line docs change into a multi-day round-trip for no added signal.

Please review the whole diff each time and give every objection in one go, ordered by severity, including the ones you consider minor. If a nit is genuinely not worth blocking on, raise it as a non-blocking note and approve rather than withholding approval for it. Absent a new finding of substance, this should be approved on the next pass.

Discussion approval stands as noted: #2347 is still unapproved, the checklist item stays unticked, and this shouldn't merge until a maintainer weighs in on the direction. That's a merge gate, not a review gate — it isn't a reason to withhold code-level approval.

pullrequestreview-4872763040

Rationale paragraph in the BylineFilter component docstring — fixed in 6d39c21a. Taken as suggested; the docstring now states only what the component does.

On the previous round's request, which wasn't followed. That comment has been in the diff unchanged since a3a13b52 on 31 July — the original commit of this branch. It was there for pullrequestreview-4867919957, for pullrequestreview-4868375437, and for this one. Same for the two comments raised before it: every one of the three was present in the first version reviewed, and none was introduced by a fix for an earlier round.

So three review rounds have produced three separate single-nit findings of the same class, from the same unchanged files, each one held as the reason not to approve. The previous round asked explicitly for all objections in one pass ordered by severity. That request was acknowledged in neither the review body nor its scope.

Concretely, for the next pass: re-read every file in the diff, not just the ones touched since the last review, and list every remaining objection at once — including ones you'd rate minor. If nothing of substance remains, approve and attach any residual nits as non-blocking notes. Another round that surfaces one more pre-existing comment nit and withholds approval on it is not a useful review, and I'll ask a maintainer to take the code-level pass instead.

Discussion approval is unchanged: #2347 is still unapproved and this shouldn't merge until a maintainer weighs in on the direction. That remains a merge gate, not a code-review gate. (Superseded — see the round below.)

pullrequestreview-4890821647

The explicit-credit EXISTS wasn't locale-scoped — confirmed and fixed in 6c8fe031. The finding is correct as stated, including the mirrored mode: "none" gap. Reproduced first as a failing test: an fr entry credited to a byline group that exists only in the default locale renders uncredited via hydrateBylinesMany, matched bylines=[group], and was excluded from No byline assigned — the same entry under two mutually exclusive filters, which is exactly the disagreement the inferred branch was fixed for one round earlier. I'd scoped that branch and left this one, on the wrong assumption that the junction storing a group made it locale-independent; copyContentBylines propagating rows to every translation is what makes it not.

Fixed as suggested: the credit test now joins _emdash_bylines on b.translation_group = cb.byline_id with the same filter.locale / entry-locale fallback authorHasByline uses — the join getContentBylinesMany already makes, so the filter and the render path derive the credit identically.

One follow-on the suggestion doesn't cover. Locale-scoping the credit test alone introduces a second disagreement, in the inference gate. hydrateBylinesMany suppresses the author fallback on the presence of any junction row (via primaryBylineId), not on one that resolves at this locale — so an entry whose explicit credit renders nothing at fr stays uncredited rather than falling through to its author. With a single locale-scoped predicate serving both roles, that entry would have matched a filter on its author's byline and dropped out of "no byline". The two roles are now separate predicates: creditRenders (locale-scoped, decides what matches) and hasExplicitCredit (locale-agnostic, decides whether inference applies at all). Both directions are covered by the new test.

Plan re-measured, and now pinned. The added join costs no round-trip and no migration — the byline row is reached through the existing (translation_group, locale) unique, covering in three of the four shapes. The plan output above is refreshed, and since the shape changed under me twice I've committed it as assertions (content-list-byline-filter-plan.test.ts) rather than re-pasting hand-run EXPLAIN into this description: all four filter shapes, every probe an indexed seek, no USE TEMP B-TREE FOR ORDER BY.

On review process. This round found a real bug that the previous three didn't, in a file all of them had in scope — that's the review the earlier rounds were asking for, and the earlier request is answered. Withdrawn, with thanks.

Discussion approval — resolved. #2347 is now maintainer-approved; the checklist item is ticked and the merge gate is lifted.

pullrequestreview-4915346586

Issue reference in the router.tsx filter-state comment — fixed in 848c726e. Taken as suggested; the comment now states only that the filter state is part of the query key. Missed when the same reference was dropped from the FilterBar docstring.

#1288 also appears twice in content-list-filters.test.ts — in a header comment and in the describeEachDialect name. That file isn't in this diff, so removing them here would be a drive-by; left for whoever next touches it.

On pinning index names in the plan test — keeping them. The suggestion is that not.toContain("SCAN ...") and not.toContain("TEMP B-TREE") already catch the real regression, so the named-index assertions only add fragility. They catch a different regression, though. The failure this test exists for is the query silently moving to a different but still indexed path — the pivot-side shape described above seeks too, and the locale join changed the shape under me twice while this branch was open. Behaviour-only assertions pass through all of that; the whole point of committing the plan was that hand-run EXPLAIN output in a description goes stale without anything failing.

The fragility objection does land on one line: sqlite_autoindex__emdash_content_bylines_2 is positional on migration 031's UNIQUE(collection_slug, content_id, byline_id) rather than a name anyone chose, so an unrelated constraint added to that table renumbers it. That's a real trap and I'll take a fix for it if you have one that keeps the assertion meaningful. The two idx_* names are explicit in their migrations — renaming one is an intentional act, and a test that has to be updated alongside it is working as intended, not fragile.

Everything else — approach, EXISTS shape, 25-id cap, locale resolution, test coverage, no new logged-out queries — is confirmed as read.

[changeset-bot]

🦋 Changeset detected

Latest commit: 6c8fe03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages

Name	Type
emdash	Patch
@emdash-cms/admin	Patch
@emdash-cms/cloudflare	Patch
@emdash-cms/sandbox-workerd	Patch
@emdash-cms/plugin-mcp-smoke	Patch
@emdash-cms/fixture-perf-site	Patch
@emdash-cms/perf-demo-site	Patch
@emdash-cms/cache-demo-site	Patch
@emdash-cms/do-demo-site	Patch
@emdash-cms/do-solo-demo-site	Patch
@emdash-cms/auth	Patch
@emdash-cms/blocks	Patch
@emdash-cms/gutenberg-to-portable-text	Patch
@emdash-cms/x402	Patch
create-emdash	Patch
@emdash-cms/auth-atproto	Patch
@emdash-cms/plugin-embeds	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Scope check

This PR changes 674 lines across 12 files. Large PRs are harder to review and more likely to be closed without review.

If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs.

See CONTRIBUTING.md for contribution guidelines.

[pkg-pr-new]

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@2312

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@2312

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@2312

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@2312

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@2312

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@2312

emdash

npm i https://pkg.pr.new/emdash@2312

create-emdash

npm i https://pkg.pr.new/create-emdash@2312

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@2312

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@2312

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@2312

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@2312

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@2312

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@2312

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@2312

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@2312

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@2312

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@2312

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@2312

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@2312

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@2312

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@2312

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@2312

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@2312

commit: 848c726

[github-actions]

Overlapping PRs

This PR modifies files that are also changed by other open PRs:

• feat(content): filter indexed custom fields #2213 (8 shared files)
• fix(content): search searchable custom fields #2191 (4 shared files)
• feat(core): configurable titleField and dateField per collection #1973 (7 shared files)
• feat: reference field type (storage-less edges + admin UI) #1928 (7 shared files)
• [codex] Add visual sections and public Pages preview #1206 (4 shared files)
• feat(content): support indexed custom field sorting #2212 (5 shared files)
• feat(admin): allow trusted plugins to add content list columns #2195 (4 shared files)
• feat(admin): show configured fields in content lists #2194 (4 shared files)
• fix: skip edge cache invalidate on draft-only content saves #2279 (3 shared files)

This may cause merge conflicts or duplicated work. A maintainer will coordinate.

[emdashbot]

This PR adds a byline filter to the admin content list. The approach is sound: it filters on the same translation_group values stored in _emdash_content_bylines, uses a correlated EXISTS so the outer query keeps its sort-ordered index, and resolves inferred credits at the list locale so the filter agrees with what the list actually renders. The 12 integration tests cover the important cases (OR matching, no-byline, inferred opt-in/out, locale-scoped inference, status composition, the empty-selection guard, and total). No new queries are added to logged-out routes, and the changeset covers the affected packages.

The main blocker is policy, not code: AGENTS.md requires maintainer-approved Discussion for new features, and the PR itself notes that Discussion #2347 is still pending approval and the PR is a draft. That approval should land before merge.

There is one AGENTS.md convention issue: an over-narrative doc comment in the repository layer that justifies the SQL shape, describes a rejected alternative, and references migration numbers. Comments should state non-obvious invariants concisely; everything else belongs in the PR description or commit message. Fixing that is the only code-level change I'd ask for.

[emdashbot]

The byline-filter approach is sound: it extends the existing content-list filter set, uses the same translation_group values stored in _emdash_content_bylines, resolves inferred credits at the list locale so the filter matches what the list renders, and keeps the query shaped as a correlated EXISTS from the content table so the outer sort index survives. The 12 integration tests cover the important cases (OR matching, no-byline, inferred opt-in/out, locale-scoped inference, status composition, total, and the empty-selection guard), and no new queries are added to any logged-out route.

The over-narrative repository comment flagged in the prior review has been trimmed to just the two invariants a future reader needs, which fixes that AGENTS.md issue.

Remaining item: one minor AGENTS.md convention note below on a justification-style sentence in the new ContentBylineFilter docstring.

Reminder: the PR itself notes that Discussion #2347 is still awaiting maintainer approval, and the feature checklist item is intentionally unticked. That policy approval should land before merge; the code-level review does not override that.

[emdashbot]

The byline-filter approach is sound and sits well in EmDash's architecture: it extends the existing server-side content-list filters, uses the same translation_group values stored in _emdash_content_bylines, resolves inferred credits at the list locale so filtering matches what the list renders, and keeps the query shaped as a correlated EXISTS from the content table so the outer sort index survives. The 12 integration tests cover the important cases (OR matching, no-byline, inferred opt-in/out, locale-scoped inference, status composition, total, and the empty-selection guard), and no new queries are added to logged-out routes.

The two AGENTS.md comment-discipline notes from the prior review have been addressed: the repository comment on applyBylineFilter is now limited to the two invariants a reader needs, and the ContentBylineFilter type docstring no longer contains a justification sentence.

One minor comment-discipline item remains in the new BylineFilter component docstring, which still narrates the rejected exhaustive-list alternative rather than only stating what the component does. Otherwise the code is clean.

[emdashbot]

The byline-filter approach remains sound: it extends the existing server-side content-list filters, matches on translation_group values stored in _emdash_content_bylines, and resolves inferred credits at the list locale so filtering agrees with what the list renders. The query is shaped as a correlated EXISTS from the content table, the IN clause is bounded by schema validation, and the 12 integration tests cover OR matching, no-byline, inferred opt-in/out, locale-scoped inference, status composition, total, and the empty-selection guard. No new queries are added to logged-out routes.

The two comment-discipline items from the last review (applyBylineFilter repository comment and ContentBylineFilter type docstring) are now trimmed to behavior-only statements. One minor AGENTS.md comment-discipline item remains in a docstring the PR edits: a stale GitHub issue reference in FilterBar. That's the only code-level note remaining.

Per the PR description, Discussion #2347 is still unapproved, so merging remains gated on maintainer direction there. Absent that process gate, this is close to an LGTM.

[emdashbot]

This is the right change for the problem described in Discussion #2347 (still pending approval). It extends the admin content list with a server-side byline filter, matches on translation_group values stored in _emdash_content_bylines, and resolves inferred credits at the list locale so filtering agrees with what the list renders. The correlated-EXISTS shape preserves the outer sort index, the 25-id schema cap avoids D1 bind-parameter blow-up, inferred/explicit semantics are handled consistently, and the 12 integration tests cover OR matching, no-byline, inferred opt-in/out, locale-scoped inference, status composition, and totals. No new queries are added to logged-out routes.

I re-read the diff, the full changed files, and relevant call sites/siblings. The previous comment-discipline finding in FilterBar is fixed. One stale GitHub issue reference remains, now in router.tsx directly above the newly-extended filter-state block. That’s the only code-level note. Fix it and this is a code-level LGTM. Merge remains gated on maintainer direction in Discussion #2347, not on the code review.

---

Findings

• [needs fixing] packages/admin/src/router.tsx:352-353

  Comment still references a GitHub issue (#1288) in the filter-state block the PR extends. AGENTS.md says comments are for future readers, not PR/issue references. The FilterBar docstring already dropped this reference; remove it here too.

  	// Filter state. All are part of the query key so changing any of
  	// them restarts the cursor chain from a filtered first page.
  

[MA2153]

Ok I am done with the bot nitpicks.

[ascorbic]

Thanks. Nearly there: just one correctness issue in the sql

fixed

[emdashbot]

This is the right change for Discussion #2347: it adds a server-side byline filter to the admin content list, matches on translation_group values, and correctly resolves both explicit and inferred credits at the list locale so filtering agrees with rendering. The correlated-EXISTS shape preserves the outer sort index, the 25-id cap avoids D1 bind-parameter blow-up, and the 14 integration tests plus 5 plan tests cover the important edge cases (OR matching, no-byline, inferred opt-in/out, locale-scoped credits, status composition, totals). No new queries are added to logged-out routes, and route-level authorization is unchanged.

I re-read the diff, the full changed files, the route handlers, the API schema, the repository SQL, and the tests. One prior code-level note persists: the #1288 GitHub issue reference in router.tsx above the filter-state block is still there, despite the FilterBar docstring having dropped the same reference. Fixing that makes this a code-level LGTM. I also have one mild suggestion about the query-plan test pinning specific index names, which trades some fragility for the author's stated goal of preventing silent plan drift.

---

Findings

• [needs fixing] packages/admin/src/router.tsx:352-353

  The comment still references a GitHub issue (#1288) in the filter-state block this PR extends. AGENTS.md says comments are for future readers of the code, not issue/PR references, and the sibling FilterBar docstring already dropped this reference in an earlier round. Remove the issue number.

  	// Filter state. All are part of the query key so changing any of
  	// them restarts the cursor chain from a filtered first page.
  

• [suggestion] packages/core/tests/integration/content/content-list-byline-filter-plan.test.ts:90-91

  The plan tests assert specific SQLite index names such as sqlite_autoindex__emdash_content_bylines_2. Those names are derived from the physical schema, so an intentional migration rename or index replacement that keeps the same query shape would still fail the test. The behavior checks (not.toContain("SCAN ..."), not.toContain("TEMP B-TREE")) already guard against the real regression (full scans / temp sorts). Consider keeping only the behavior assertions so the test fails on plan regressions without pinning schema-derived identifiers.

[emdashbot]

This is the right change for Discussion #2347. The PR adds a server-side byline filter to the admin content list that matches on translation_group, supports OR selection plus a "no byline" mode, and correctly separates inferred from explicit credits. The correlated (NOT) EXISTS shape preserves the outer sort index, locale scoping keeps filtering consistent with how entries are rendered, and the 25-id cap avoids D1 bind-parameter blow-up. The implementation follows the existing filter plumbing, is gated behind unchanged admin permissions (content:read, bylines:read), and adds no queries to logged-out routes.

I re-read the diff, the full changed files, the route handlers, the API schema, the repository SQL, the byline repository, and the tests. The only prior code-level finding is fixed: the #1288 GitHub issue reference in router.tsx is gone. The locale-scoped explicit/inferred credit tests reproduce the subtle disagreements described in the PR and pass against the current logic.

One non-blocking follow-up persists: the plan test pins the auto-generated SQLite index name sqlite_autoindex__emdash_content_bylines_2, which the author already flagged as fragile because adding another unique constraint to the table can renumber it. I’ve left a suggestion to replace that assertion with a regex on the indexed column tuple, which keeps the meaningful invariant without coupling the test to SQLite’s autoindex numbering. The explicit idx_* assertions are intentionally named in migrations and are fine to keep.

With the documented limitation noted (full-scan cost on very large collections for sparse byline matches, accepted as an authenticated-admin trade-off), this is a code-level LGTM.

[emdashbot]

[suggestion] This assertion pins the auto-generated SQLite index name for the UNIQUE(collection_slug, content_id, byline_id) constraint. That name is positional: if an unrelated migration adds another unique constraint to _emdash_content_bylines, sqlite_autoindex__emdash_content_bylines_2 can become _3 and this test will fail even though the plan shape is still correct. Assert the seek columns instead so the test stays invariant under autoindex renumbering.

Suggested change

	expect(plan).toContain("sqlite_autoindex__emdash_content_bylines_2");
	expect(plan).toMatch(
	/SEARCH cb USING INDEX \S+ \(collection_slug=\? AND content_id=\? AND byline_id=\?\)/,
	);