8000 fix(mapper): read .reylo/ offline whatever the database url, or none by colinhacks · Pull Request #9 · colinhacks/reylo · GitHub
[go: up one dir, main page]

Skip to content

fix(mapper): read .reylo/ offline whatever the database url, or none - #9

Merged
colinhacks merged 1 commit into
mainfrom
cache-key-without-server
Oct 22, 2026
Merged

colinhacks merged 1 commit into
mainfrom
cache-key-without-server

Conversation

@colinhacks
@colinhacks colinhacks commented Oct 22, 2026 •
Copy lin 8000 k
Copy Markdown
Owner

A build that couldn't reach the database read .reylo/ only under the type check's exact URL. Each entry was found by the server it was read from: scheme, user, host, port and database, or for SQLite the database file's absolute path. So:

  • a container build had to set a database URL it couldn't use, and copying the real one in put a production host and user, or a password, into the build's environment;
  • a build with no URL failed before it read the cache;
  • a SQLite project's cache missed every entry once the project sat at another path, as it does in a container.

What changes

  • src/analyze.ts: an entry's file is named by its statement (the statement, the engine's fingerprint, the dialect), then by its server (never the password). Online, a session reads and writes its own server's entry, as before, so two databases keep an entry each. That matters on Postgres, whose schema fingerprint holds object ids, so a dev and a test database built by the same migrations still differ. A session that can't reach its database, or has no URL, reads its own server's entry where there is one, else the newest entry for the statement from any server. "Newest" means the write time each entry now records, since a copy of .reylo/ (cp -r, fs.cpSync, a CI artifact) may reset the files' times. With no URL and no entry, it's the existing "No database URL" error.
  • src/project.ts: with no URL, the dialect is the one engine package the project installed.
  • Docs: the In CI section and the esbuild and nub examples say a build without a database needs no URL.

Entries written before this change keep the old file names and become orphans, read by nothing, in a directory git ignores.

Verified

  • Tests: no URL, an unreachable URL, and a copy of the project at another path without its database all read the entry. Two databases with different schemas, used in turn, each read their own entry the second time. Each new test fails on the code before it, the second on this PR's first commit, which kept one entry per statement. An offline read picks the entry written last even when the copy's file times say otherwise.
  • The esbuild example, after a type check filled .reylo/ and with its database gone, type-checks and builds with no DATABASE_URL and with one it can't reach.
  • Local gates: the typecheck, loader tests (192, none skipped), the package, the site, the basic, bundler, SQLite, MySQL and Next.js labs, and the Node, nub, Vite, esbuild, Next.js and Cloudflare Workers examples pass. The first commit's run of the driver matrix differed from RESULTS.md only in the 24 S11 round-trip rows that main already fails.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 16:41
Copilot AI left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vercel
vercel Bot commented Oct 22, 2026 •
Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
reylo Ready Ready Preview Oct 22, 2026 5:31pm UTC

Request Review

@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues. One example README still documents the old keying, and there's one perf caveat for Postgres.

Reviewed changes

Initial review of the single commit, which takes the database server out of the .reylo/ cache key so that a build with another URL, no URL, or a moved project reads the entries a type check wrote.

  • Server-free entry key: Session.entryFile now hashes only the entry version, the engine fingerprint, the dialect and the statement. #server() is gone, and each entry's recorded schema fingerprint is now the only freshness check.
  • No-URL read path: with neither a URL nor schema, #analyze serves .reylo/ as-is, the same way it does when the database is unreachable. A miss, force, or a missing engine returns the existing missingUrl error (code 4).
  • Dialect without a URL: the new installedEngines takes the dialect from the single installed engine whether or not schema is set. The "which database?" error still fires only under schema.
  • Docs and tests: 03-inference.mdx now says a build needs no URL. There's a new offline test (no URL, another URL, a copied project) and a rewritten keying test.

ℹ️ examples/esbuild/README.md still tells builds to set the same DATABASE_URL

The "Build without a database" section wasn't updated. It still tells readers to build with the same DATABASE_URL "even though it cannot connect: Reylo keys each analysis by the scheme, user, host, port and database of the URL". It also says a build "with another URL, or none" fails. After this PR, both statements are false. The first one keeps pushing readers toward the problem this PR fixes: putting the production host and user into the build's environment.

Technical details
# Stale offline-build guidance after the cache key change

## Affected sites
- examples/esbuild/README.md:62 — says to build with the same `DATABASE_URL` because entries are keyed by the URL's scheme/user/host/port/database
- examples/esbuild/README.md:64-66 — the example build command sets `DATABASE_URL=postgres://db.internal:5432/app`, which is no longer needed
- examples/esbuild/README.md:68 — "So does a build with another URL, or none" (fails at that file); another URL or no URL now reads `.reylo/`
- examples/nub/README.md:48 (softer) — "`nub src/main.ts` needs `DATABASE_URL` ... as the type check does"; now true only for a statement `.reylo/` lacks

## Required outcome
- The example docs match `site/content/sections/03-inference.mdx:82`: a build that can't reach the database reads `.reylo/` and needs no database URL.
- A statement `.reylo/` lacks still fails. With no URL, the message is the `No database URL: ...` one (`missingUrl`), not the `.reylo has no analysis of this statement` one.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread src/analyze.ts Resolved
Comment thread src/analyze.ts Resolved
@colinhacks
colinhacks force-pushed the cache-key-without-server branch from 4b820c8 to 95bab96 Compare October 22, 2026 16:51
@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The earlier feedback is addressed. One minor note inline about how the offline fallback picks an entry.

Reviewed changes

Since 4b820c8, 95bab96 gives each server its own entry again and adds a fallback that reads any server's entry when the database can't be reached.

  • Entry per statement and server: entryFile is now <statementKey>-<server hash>.json. Two Postgres databases with different OIDs no longer overwrite each other's entries.
  • Offline fallback: #offlineEntry reads this session's own entry first. If there is none, it reads the newest valid entry for the statement from any server. This covers both the unreachable‑URL path and the no‑URL path.
  • Online path tightened: a reachable database now reads only its own entry, and only when that entry's fingerprint matches.
  • Docs and tests: the esbuild and nub READMEs and 03-inference.mdx now say a build needs no URL. A new test switches between two databases and checks that each one reuses its own entry.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread src/analyze.ts Resolved
@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The mtime concern is fixed. Two small cleanups are noted inline.

Reviewed changes

Since 95bab96, 261c734 makes the offline fallback order entries by the write time each entry records, not by the file's mtime.

  • Recorded write time: Entry gained an optional at, and #analyze sets it with Date.now() when it writes an entry.
  • Fallback ordering: #offlineEntry now does one pass and keeps the entry with the largest at. It uses statSync().mtimeMs only for an entry that has no at.
  • Test: the two-database test now takes both databases away and swaps the files' mtimes. It then checks that a URL with no entry of its own reads the entry with the later at.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread src/analyze.ts Resolved
Comment thread src/analyze.ts Resolved
@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. Both cleanups from the last review are addressed.

Reviewed changes

Since 261c734, 87b08b7 simplifies how the offline fallback orders entries.

  • Required at on Entry: at is now a required field, and its JSDoc sits on the field. Entry keeps its own description on hover.
  • Recorded time only: #offlineEntry drops the statSync mtime fallback and its import. A missing at counts as -Infinity, and the function returns the { file, entry } pair directly.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@colinhacks
colinhacks force-pushed the cache-key-without-server branch from 87b08b7 to 751a969 Compare October 22, 2026 17:14
@pullfrog pullfrog Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. The rebase onto main resolves cleanly against the removal of schema in #6.

Reviewed changes

Since 87b08b7, the branch was squashed into 751a969 and rebased onto main, which had removed the schema-files feature. The only net change is that the PR's schema handling is gone.

  • schema branches dropped in src/analyze.ts: the no-URL guards are now just !this.project.url. #server() no longer returns "schema". schema.fatal no longer tags its outcome with schema: true. The offline and online entry logic is otherwise unchanged.
  • schema branches dropped in src/project.ts: the "schema" names a schema of which database? and MySQL schema errors are gone. With no URL, installedEngines(root) picks the dialect only when exactly one engine is installed, and otherwise it stays postgres.
  • Comments updated: the file headers no longer mention the schema files. No references to schema-files.ts remain in src/ or tests/.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

An entry was found by the server it was read from, so a build that
could not connect had to set the type check's exact url, a SQLite
project its database file at the same absolute path, and a build with
no url failed before it read the cache. Each database keeps its own
entry, and a session that cannot reach its database, or has no url,
reads the statement's newest entry from any, by the time it records.
@colinhacks
colinhacks force-pushed the cache-key-without-server branch from 751a969 to 70c02fa Compare October 22, 2026 17:30
@colinhacks
colinhacks merged commit 9100d86 into main Oct 22, 2026
2 of 4 checks passed
@colinhacks
colinhacks deleted the cache-key-without-server branch October 22, 2026 17:31
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.

2 participants

0