Skip to content

Drop the stale isinstance claim from grail.md - #181

Open
MatiasFernandez wants to merge 2 commits into
mainfrom
mfernandez/grail-doc-isinstance
Open

MatiasFernandez wants to merge 2 commits into
mainfrom
mfernandez/grail-doc-isinstance

Conversation

@MatiasFernandez

@MatiasFernandez MatiasFernandez commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

docs/grail.md contradicted itself about isinstance after a schema change.

The bullet in the Brain Freeze findings says a schema change keeps isinstance for records written before it. The paragraph after the findings said that this "does not reproduce on c875e56": a schema change "no longer keeps isinstance at all", so use type(obj).__name__ instead.

The bullet is right for the pinned Grail. On c8cbc92 (engine 4.0.0.a5, 2026-10-27), and earlier on beef473 (engine 4.0.0.a4, 2026-10-20), a stored Person whose class is then edited to assign a new attribute (with or without a class attribute default), re-imported and committed, still passes both isinstance(old, Person) and type(old) is Person. This matches Grail's identity reuse: a rebuild reuses the class object.

This PR removes the stale claim from that paragraph, along with the type(obj).__name__ advice. The paragraph's other claim, about a first call to a never-compiled function dirtying the session, stays as written, since I haven't re-measured it.

Docs only, so no CHANGELOG entry.

🤖 Generated with Claude Code

MatiasFernandez and others added 2 commits October 27, 2026 18:02
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bullet above the paragraph already describes the current behaviour, so
the paragraph only needs to stop contradicting it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MatiasFernandez MatiasFernandez changed the title Fix grail.md: a schema change keeps isinstance on the pinned Grail Drop the stale isinstance claim from grail.md Oct 27, 2026
@MatiasFernandez
MatiasFernandez marked this pull request as ready for review October 27, 2026 21:12

This branch has not been deployed

No deployments
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.

1 participant