Release v1.1.0 — stop emitting G3 as a runtime warning - #6
Merged
Conversation
G3 warned whenever an rls_condition used NOT IN without OR <col> IS NULL. The trap it names is real: SQL three-valued logic makes NOT IN UNKNOWN against a NULL, WHERE keeps only TRUE, and the row disappears. What made it the wrong shape for a warning is that it fired on every deny-list, in every run, for the life of a config — and it named only one of the two valid mitigations. A config that deliberately asserts the column is never NULL had no way to be clean, so its author learned to read past the warnings. Permanent warnings hide the occasional real one, which is the opposite of what a guardrail is for. The knowledge moves to docs/architecture.md, where G3 now describes the trap and both mitigations as guidance rather than being emitted per row. RLS.null_safety_warning() goes with it. Minor rather than patch: the warnings a caller sees change, and a documented helper leaves the public API.
The release notes moved to 1.1.0 but every doc still opened with "OLAF v1.0.0 is an independent community Preview", and the framework_version examples still showed what 1.0.0 stamped. Left alone deliberately, because they are true as written and changing them would make them false: CHANGELOG [1.0.0] the 1.0.0 release, dated roadmap.md what the first public candidate added test_mock_pipeline.py the 1.0.0 hash algorithm, kept for back-compat test_unit_apply.py what 1.0.0 solved olaf.ipynb hash comments a 1.0.0-stamped plan row still opens the gate CONTRIBUTING.md an example of the runtime-to-tag rule test.yml / test_public_release.py archive prefixes, mechanical .superpowers/ the 1.0.0 release record
The cookbook opened with two guards that do the same thing. The short one came with the 1.0.0 release; the longer one was added by 301c24c without noticing it, so both shipped. Kept the longer one. It says what would actually happen on a Run All, names the paths that trigger it (Run All, a Data Factory pipeline, notebookutils.notebook.run), and tells the reader how to use the notebook instead. Deleting the short one also makes its own claim true: it is now the first code cell, so "this guard runs FIRST" describes where it is rather than where it wishes it were.
Every doc opened with "OLAF v1.x.x is an independent community Preview", so a release meant editing eighteen files to say a number that __version__ and the CHANGELOG already carry. That is how they came to disagree in the first place. The prose now says "OLAF". Two places still carry a number on purpose: the CHANGELOG, which is versioned by definition, and __version__ itself. The three sample values (data-model.md, Audit.md) stay concrete because an example needs to show a real shape, and nobody reads sample output as a claim about today. Statements about WHEN something changed keep their number — "a runtime warning until 1.1.0" is history, and dropping the version would delete the fact.
Removing the G3 tests left RLS imported and unused in test_unit_lib.py. Tests do not fail on an unused import, so the local run stayed green and CI caught it. The lesson is the run, not the import: the repo lints notebooks and Python together through scripts/lint.sh, and I pushed twice without running it.
Review found the change stopped at the code and the internal design doc. Three
places still spoke for the deleted check, and one sentence this branch wrote was
false.
configs/onelake_security.xlsx the starter workbook's own note said "Every NOT
IN needs OR <col> IS NULL" -- the single-mitigation
framing this branch removed as wrong, stated more
strongly, on the artifact users copy from
configs/README.md said the workbook is "versioned for this OLAF
release" while the workbook stamped v1.0.0 in
three cells. Both now name no version
docs/api/functions.md listed RLS among namespaces hosting callable
helpers; its only row was the one removed
The CHANGELOG now says the thing that actually justifies the deletion, which the
first draft did not. The check was a substring test on the raw condition rather
than the literal-stripping lexer C9/C11/C13 share, so it warned on
"status = 'CANNOT INVOICE'" -- no NOT IN operator anywhere -- and stayed silent
on "region NOT IN ('a') AND type IS NULL", muted by a different column. Wrong in
both directions, and unfixable without the lexer it never used.
It also states the direction of failure, which CONTRIBUTING requires of anything
touching validation and the first draft omitted: NOT IN against a NULL
over-rejects. It removes rows a role should see and cannot widen access.
The workbook is patched as a zip, one member at a time. openpyxl's save()
rebuilds the package and re-adds docProps, and the release gate rejects that as
person metadata -- caught by test_candidate_tree_passes_the_release_gate, not by
me.
kengio
changed the title
…entity Round two of review. The removal was complete in code and correct in the design doc, and still left three ways to get hurt. THE ROUTE, NOT THE KNOWLEDGE. Following docs/README.md's own "Start here" trail, a user authoring their first NOT IN reads config-examples.md and runbook.md section 2 -- and meets the trap in neither. It survives only in architecture.md's Key invariants, inside the group that same index calls "reference material you'll come back to." Before this branch there was a runtime backstop: generate printed the warning at the moment of authoring, and no navigation was involved. Deleting it without touching the authoring path traded a discovery mechanism for a footnote. Both docs now carry it, and both say the direction: over-rejection, a role sees fewer rows and never more. THE TAG. v1.0.0 was signed with a sanitized maintainer identity. This repo's git config -- local and global -- is a personal address, and check_public_release.py lists that domain under PERSONAL_EMAIL. The gate has tree and archive modes only; neither reads a tag object. So a plain `git tag -a` stamps a personal address into a public, immutable object with nothing to stop it. CONTRIBUTING.md now carries the tag command, and reads the identity off the previous tag rather than naming it -- writing the address down is itself a finding (APPROVED_IDENTITY_ CONTEXT), which is how the first draft of this paragraph failed the gate. THE GUARD'S MARGIN. Removing the duplicated Run-All guard was right, but it took the redundancy with it: one unpinned cell now stands between Run All and live operations, and nothing asserted it existed. test_the_cookbook_guard_still_stands_ before_every_live_example pins the property -- exactly one guard, first code cell, before %run. Also: the release-evidence archive in test.yml was still named olaf-1.0.0; it now reads the version from the runtime, so the evidence cannot be filed under a version the tree is not. The MINOR bump over a public-API removal is recorded in the CHANGELOG as a deviation with its reasons, rather than left to look like an oversight. And the workbook note is back to one row's worth of text -- the longer wording was clipped by row 3's customHeight, so half of it was invisible in the one place a config author actually reads it. 1654 tests pass. Release gate PASS. lint clean.
…rkflow The previous commit computed the release-archive prefix with a Python heredoc inside the release_hygiene `run:` block. The heredoc body sits at column 0; the block scalar is indented ten. YAML ends a literal block at the first line indented less than the block, so the workflow file stopped being parseable at `import json, re` -- and GitHub does not fail a run it cannot read. It ran ZERO jobs and reported "no checks reported on the branch", which reads like CI has not started yet rather than like CI is broken. Branch protection then blocked the PR waiting for five checks that could never appear. That is the same shape as the bug this release is about: a check that is absent looks exactly like a check that passed. scripts/print_version.py now reads `__version__` out of the notebook and prints it. The workflow calls it. No inline script, one source of truth, and the version is still not frozen into the evidence name. Verified rather than assumed: the workflow parses (4 jobs), the helper prints 1.1.0, two archives at that prefix are byte-identical, and the archive gate passes on it. 1654 tests, tree gate PASS, lint clean.
Round three. Two findings, and the first one is about a test I wrote badly. THE GUARD THAT GUARDED NOTHING. test_workflow_block_scalars_never_dedent_below_ their_own_block now catches an under-indented line inside a `run: |` block -- the break that made this workflow unparseable, so GitHub ran zero jobs and reported "no checks reported on the branch", which reads like CI has not started. The first version of it passed against the very file that carried the bug. It compared indentation: the heredoc body sits at column 0, which is BELOW the block's own key, and that read as "closing an outer mapping" rather than as the killer. What it should ask, and now asks, is whether the dedented line is a YAML key or list item at all. `import json` is neither. I only know that because I put the bug back and ran the test against it. A guard that has never seen the thing it guards is a guess. WHAT UPGRADING DOES. The release note omitted the one behaviour change an operator will actually observe, and it comes from the version bump, not from G3. The idempotent-skip fast path requires the stored framework_version to equal __version__, so the first generate after upgrading re-stamps the mapping even on a byte-identical config: changed True instead of False, success instead of skipped, two log rows instead of one. A pipeline gated on envelope["changed"] re-plans and re-applies once per deployment. config_hash does not move and the next run skips again. Also recorded: mapping_hash excludes provenance, so a 1.0.0-stamped saved plan still opens the apply gate and the mapping-history CSV is reused verbatim -- its framework_version column reads 1.0.0 while the table reads 1.1.0. That is the mechanism that lets a plan survive an upgrade, so it is worth stating rather than leaving to be discovered. The note sits after ### Removed so the Keep a Changelog sections stay in order. 1655 tests, lint clean, release gate PASS, workflow parses to 4 jobs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request is the release change for v1.1.0.
CONTRIBUTING.mdreserves version bumps and dated CHANGELOG sections for a PR that says so; this one does. It bumps__version__to1.1.0, files## [1.1.0] - 2026-08-27, and leaves## [Unreleased]empty. The tagv1.1.0is created on merge, not here.What ships
G3 is no longer a runtime warning.
RLS.null_safety_warning()is removed with it, and the trap it described moves into theKey invariantslist indocs/architecture.md— with both mitigations and the direction of failure, which the first draft of this branch omitted.Why the check went
The trap is real and unchanged:
NOT INagainst a NULL is UNKNOWN,WHEREkeeps only TRUE, and the row disappears from a deny-list's result.The check went because it could not tell that case from any other. It was a substring test on the raw condition rather than the literal-stripping lexer that rules C9/C11/C13 share, so:
status = 'CANNOT INVOICE'NOT INoperator at all — the literal contains the substringnote = 'DO NOT INCLUDE'region NOT IN ('a') AND type IS NULLregionis unguarded; a different column'sIS NULLmuted itWrong in both directions, on every run, for the life of a config — and unfixable without the lexer it never used. It also named only one of the two valid mitigations, so a config that deliberately asserts the column is never NULL could never come back clean.
Direction of failure
NOT INagainst a NULL over-rejects. It removes rows a role should see and cannot widen access to any row.CONTRIBUTING.mdrequires stating this for anything touching validation; the first draft did not, and a reader could reasonably have inferred an exposure risk was traded for quiet.Version
1.1.0. Strict SemVer reads a public removal as major —RLS.null_safety_warningwas listed indocs/api/functions.md. Nothing is distributed as a package (users copy a notebook),SECURITY.mdsupports only the latest Preview, and real-world breakage is nil. Say the word if you would rather it were2.0.0.Review
Three independent lenses reviewed this branch; their findings are in the last commit. The ones worth naming:
configs/onelake_security.xlsxstill told authorsEvery NOT IN needs OR <col> IS NULL— the single-mitigation framing this branch removed as wrong, stated more strongly, on the one artifact users copy from. Fixed.configs/README.mdsaid the workbook is versioned for the current release while the workbook stampedv1.0.0in three cells. Both now name no version.docs/api/functions.mdlistedRLSamong namespaces hosting callable helpers; its only row was the one removed.The workbook is patched a zip member at a time on purpose:
openpyxl.save()rebuilds the package and re-addsdocProps, which the release gate rejects as person metadata.test_candidate_tree_passes_the_release_gatecaught that, not me.1653 tests pass.
scripts/lint.shclean.