Skip to content

docs(providers): cite factory.ts by its entry point, not a line - #629

Merged
cevheri merged 1 commit into
libredb:mainfrom
7487:docs/factory-citations-by-name
Sep 7, 2026
Merged

docs(providers): cite factory.ts by its entry point, not a line#629
cevheri merged 1 commit into
libredb:mainfrom
7487:docs/factory-citations-by-name

Conversation

@7487

@7487 7487 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Description

Both halves of #620.

Gap 2 — seven provider docs cited a line inside factory.ts, all stale. postgres.md (:62, end of a docblock), clickhouse.md (:87), couchbase.md (:93), druid.md (:95, the libsql import), libredb.md (:100, the oracle import), elasticsearch.md (:118) and opensearch.md (:123) now name createDatabaseProvider() and link the file without a coordinate, the way mssql.md, mysql.md, mongodb.md and redis.md already do. grep -rn 'factory\.ts:[0-9]' docs/providers/ returns nothing.

Gap 1 — the guard could go vacuous. The factory test filtered NAMED_CITATIONS, so renaming the phrase in every doc shed four assertions and stayed green. The population is now a sorted walk of docs/providers/*.md (the theme-token-usage idiom), selected on the entry point's name rather than on the link — oracle.md links factory.ts without naming the function and owes no such sentence — with a derived non-empty assertion and no pinned count. The same test also asserts no provider doc carries a factory.ts:<line>, so the seven fixes above cannot rot back.


Type of Change

  • Documentation update
  • Test addition or update

Related Issue

Fixes #620


Changes Made

  • docs/providers/{postgres,clickhouse,couchbase,druid,libredb,elasticsearch,opensearch}.md: one sentence each in Registration & lifecycle
  • tests/unit/provider-docs-monitoring-citations.test.ts: PROVIDER_DOCS from a sorted readdirSync, non-empty guard, factory.ts:\d ban across all provider docs, scope note updated

Testing

  • I have tested this locally
  • I have added/updated tests
  • All existing tests pass

Measured on this branch:

pass fail assertions
tests/unit/provider-docs-monitoring-citations.test.ts 27 0 237
same, with `createDatabaseProvider()` renamed in all 11 docs that cite it 26 1 (expect(docs.length).toBeGreaterThan(0)) 207

Before the test change, the second row was 27 / 0 / 207 — the four (now eleven) assertions vanished and nothing went red.

bun run format, bun run lint (0 errors), bun run typecheck and bun run knip are clean. bun test tests/unit passes except for the chart tests without helm on PATH (#570), sqlite-driver under Bun 1.3.11 (no node:sqlite; the repo pins 1.4.2) and packaging-standalone-zip without a standalone build — none of which touch these files.


Test Environment

  • LibreDB Studio Version: main @ b39381f
  • OS: macOS
  • Node.js/Bun Version: Bun 1.3.11

Checklist

  • My code follows the project's code style guidelines
  • I have performed a self-review of my code
  • I have updated the documentation accordingly
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Seven provider docs cited a line inside factory.ts, and all seven were
stale: postgres.md's `:62` is the end of a docblock, druid.md's `:95` is
the libsql import, libredb.md's `:100` the oracle import. They now name
`createDatabaseProvider()` and link the file, the way mssql.md, mysql.md,
mongodb.md and redis.md already do.

The factory test built its population by filtering NAMED_CITATIONS, so
renaming the phrase in every doc shed four assertions and stayed green.
The population now comes from a sorted walk of docs/providers/, selected
on the entry point's name (oracle.md links the file without naming it and
owes no such sentence), and asserts non-empty rather than pinning a
count. Every provider doc is also checked for a `factory.ts:<line>`.

Fixes libredb#620

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@cevheri
cevheri self-requested a review September 7, 2026 14:38
@cevheri

cevheri commented Sep 7, 2026

Copy link
Copy Markdown
Member

Welcome, and thank you. This is your first PR here, and it is a good one.

I did not review it by reading. I ran your own probe in a clean worktree at 942d7a5.

  ┌─────────────────────────────────────────────────┬──────┬──────┬────────────┐
  │                      Probe                      │ pass │ fail │ assertions │
  ├─────────────────────────────────────────────────┼──────┼──────┼────────────┤
  │ baseline                                        │ 27   │ 0    │ 237        │
  ├─────────────────────────────────────────────────┼──────┼──────┼────────────┤
  │ createDatabaseProvider() renamed in all 11 docs │ 26   │ 1    │ 207        │
  ├─────────────────────────────────────────────────┼──────┼──────┼────────────┤
  │ same rename, against the guard on main          │ 27   │ 0    │ 207        │
  ├─────────────────────────────────────────────────┼──────┼──────┼────────────┤
  │ factory.ts:78 put back into oracle.md           │ 25   │ 2    │ 232        │
  └─────────────────────────────────────────────────┴──────┴──────┴────────────┘

Row 2 is your table, reproduced. Row 3 is the one that matters. The old guard stays green under the same rename and quietly drops 30 assertions. So the gap was real, and your change is what closes it. Row 4 shows the factory.ts:\d ban reaching a doc outside the derived population, so the seven fixes cannot rot back.

grep -rn 'factory\.ts:[0-9]' docs/providers/ returns nothing. oracle.md stayed out of the population, which was the trap in the issue. You read past it.
The walk is sorted, the non-empty assertion is derived, no count is pinned. All three Done when conditions hold.

Two things worth knowing, and neither is a change request.

First, a house habit you already used, so this is just naming it. When a test derives its own population, prove the guard by breaking the thing it guards, and pair that with a control. Your row 2 alone says the new guard is red. Row 2 next to row 3 says the new guard is why. We ask for the control on every PR that touches a guard.

Second, my debt, not yours. Selecting on the entry point's name means any provider doc that merely mentions createDatabaseProvider() now owes the registration sentence.
I added a troubleshooting line to oracle.md and the suite went red for a doc that was correct. The issue told you to select on the name, so this is the instruction's limit. I will file it.

Optional, only if you want it. docs/AGENT_GUIDE.md:928 cites src/lib/db/factory.ts:649 for PROFILE_UNSUPPORTED_BY_PROVIDER. It is at 677 today, and 649 falls inside a comment block. Same defect class, outside this PR's scope. Happy to open it as a separate issue if you would rather take it in a fresh PR, or leave it and I will file it for someone else.

Approving. Your PR body was re-runnable line by line, which is rarer than it should be. Please pick up another one

@cevheri
cevheri merged commit c893dcf into libredb:main Sep 7, 2026
22 checks passed
cevheri pushed a commit that referenced this pull request Sep 8, 2026
* docs(clickhouse): cite code by name, not by line

clickhouse.md pinned 16 citations to line numbers in index.ts, transport.ts,
introspect.ts and connection-string-parser.ts (the 17th, factory.ts:87, went
with #629), and 13 of them were stale: prepareQuery() cited :562 is declared
at :596, resolveConnection() :402 at :420, getCapabilities() :461 at :503,
getLabels() :489 at :553, query() :593 at :674, runMaintenance() :920 at
:1013, and both index.ts:637 cites land on a closing brace today. Each now
names the declaration and links the file the way mssql.md, mongodb.md and
postgres.md already do; the two error-classification cites name
mapClickHouseError(), the verbatim-type one names readColumn().

The doc joins NAMED_CITATIONS in the guard, so it can carry no `.ts:<line>`
and every method it names must really be declared in index.ts. Reverting the
doc alone fails 2 of 31.

Fixes #588

* docs(clickhouse): drop getPlaceholder() row, widen the guard to every cited method

Section 2.3 listed getPlaceholder() as an inherited SQLBaseProvider member,
but #304 (02578ff) deleted it and nothing under src/ declares it. The row's
point survives as a sentence under the table: ClickHouse binds named
{name:Type} parameters only, so a positional `?` never applies and query()
throws rather than send one unbound.

The NAMED_CITATIONS entry named 6 of the 20 index.ts methods the doc cites.
It now names all 20, in declaration order, with a comment saying the list
tracks the doc rather than a hand-picked subset. Module-level functions
(resolveConnection) and inherited members carry no access modifier for
declarationLine() to match, so they stay unmeasured, as the PR body says.
cevheri pushed a commit that referenced this pull request Sep 8, 2026
docs/providers/couchbase.md cited code by line number in 29 places on
today's main (the 30th in #591, factory.ts:93, went with #629), plus one
ConnectionModal.tsx:139 that the `.ts:` grep cannot see. Of the 29, only
the six in introspect.ts and keyspace.ts still hold: the five that name
a method beside the line all miss (connect() :335 -> :399,
getCapabilities() :276 -> :299, getLabels() :293 -> :345, query() :408
-> :472, runMaintenance() :762 -> :869), and the unnamed ones land on
closing braces, section banners and a comment in use-query-execution.ts.

Every citation now names the declaration and links the file without a
coordinate, the shape mongodb.md, postgres.md and clickhouse.md already
use. No sentence changed meaning. The doc joins NAMED_CITATIONS with
index.ts as its source and the 24 class members it cites, and that
list's no-line-number test now reads `\.tsx?:\d` so the .tsx citation
cannot come back either.

Fixes #591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests: the factory-citation guard can go vacuous, and 7 provider docs still cite a line inside factory.ts

2 participants