Implementation review: PB-111, PR 798

Field	Value
Backlog item	PB-111 Check now covers SQL scripts and scheduled notebooks (feature FEAT-GOV-09). It came from findings 2 and 5 of the PB-108 review (`docs/product/reviews/PB-108-643.md`).
Pull request	#798, merged as squash commit 8809db10 (parent 134e708d).
Package	none
Reviewer	Engineering quality review thread. It didn't write #798.
Date	2026-10-27
Environment	Linux container, main 6159e507, Python venv for the engine, Playwright Chromium. No tenant, no Windows, no second PC.

Outcome#

Changes Required (docs only). Once the pipeline-schedule gap is documented (finding 1), this review recommends Accepted with Documented Limitations.

Criterion 1 is met. Check now now collects saved queries and the .sql files of warehouse projects, and a policy that names SQ rules gets pass or findings from them. Criterion 2 is met for notebooks that a schedule on this PC runs directly. Two kinds of scheduled notebook still don't get NB004 or NB008. The first is a notebook that only a Fabric schedule runs, and users are told about it. The second is a notebook that only a pipeline schedule on this PC (#791) runs. The PR names this one as a known limitation, but the docs and the inventory don't mention it.

Recommended by: Engineering quality review. Decided by (product owner): pending.

Acceptance criteria#

#	Criterion (copied from the backlog item)	Result	Evidence
1	A policy naming SQ rules shows pass or findings from this PC's SQL scripts, never not checked when scripts exist	Met	`governance_check_now` adds SQL items when a policy names a `sql` rule (`central/checks_service.py:314`). `_gc_sql_items` (`checks_service.py:226`) reads saved queries (`sql_queries_list`: My queries and each repository's sql-queries folder) and every `.sql` file of the warehouse projects in the repositories opened in Source control (`wh_projects_list`, `_wh_walk`). Each item is `{kind: "sql", query, target: "warehouse"}`. The SQL adapter returns a list for any `sql` item, never `None` (`best_practice_checkers.py:33-46`), so every collected script counts as checked. Unreadable, too large or non-UTF-8 files go into `problems` instead. `test_check_now_checks_saved_queries_and_warehouse_project_files` covers this: 2 scripts checked, 1 passed, 1 finding on the saved query, and the bad file is listed under problems. With no scripts, the policy still shows "not checked" (`test_check_now_runs_notebook_rules_and_says_when_nothing_is_there_to_check`).
2	NB004 and NB008 run in Check now on notebooks that have a schedule	Met for schedules on this PC; not met for Fabric-only and pipeline-only schedules	`LocalSchedules.notebooks()` (`local_schedules.py:232`) returns the names of notebooks that an enabled `kind: "notebook"` schedule targets. `_gc_notebook_items` sets `scheduled` from that list (`checks_service.py:207`). `run_rule` now runs a `scheduled_only` rule on items marked scheduled and skips the others (`governance_checks.py:149`). Before this change it never ran them. `test_check_now_runs_scheduled_only_rules_on_notebooks_a_schedule_runs` covers a scheduled notebook (1 finding), an unscheduled one (skipped), a paused one (skipped), a meta schedule (ignored), and a torn `schedules.json` (listed under problems and left in place). A notebook that only a Fabric schedule runs is not marked; `docs/governance.md:217` and the FEAT-GOV-09 limitations say so. `notebooks()` ignores `kind: "pipeline"` targets (`local_schedules.py:240`), so a notebook that only a pipeline scheduled on this PC (#791) runs is also not marked. No user doc says so (finding 1).

Checks#

Check	Result	Evidence or note
Scope matches the item and package	Met	It collects SQL scripts and marks scheduled notebooks, and nothing else. It also adds a wording change in the Best practices "How it works" text (`app/ui/governance-checks.js:26`) and the docs.
New tests fail without the change, pass with it	Met	I built a copy of main 6159e507 with the pre-#798 `checks_service.py`, `governance_checks.py` and `local_schedules.py` (`git show 8809db10^:...`). `test_best_practice_checkers.py` gave 3 failed, 6 passed: all three new tests fail. On main it gave 9 passed.
Full suites pass (pytest, UI, cargo)	See below	Related engine tests on main 6159e507: `test_best_practice_checkers.py`, `test_best_practice_checks.py`, `test_notebook_practices.py` and `test_sql_practices.py` gave 80 passed. `test_schedules.py`, `test_sql_queries.py`, `test_warehouse_project.py` and `test_governance*.py` gave 107 passed. `node --test app/tests/ui.test.cjs`: 297 pass, 0 fail. Browser `best-practice-checks.test.cjs` plus `mesh-project.test.cjs`: 10 pass, 0 fail. Full `pytest sidecar/tests` on a copy of main, 4982 passed, 77 skipped, 1 failed in 22 min. The failure, `test_agent.py::test_agent_runs_a_queued_environment_with_run_details` (agent run table count), is unrelated to this PR and passed on rerun (`test_agent.py` 38 passed): a timing flake under load. cargo: not relevant (no Rust change).
Reachable where docs say; Preview behavior right; packaged into the engine	Met	It's reached from Check now on Governance > Best practices. That page is still behind `preview("bestPractices")`, and nothing new is outside it. All changes are engine Python and docs.
OneLake data safety	Met	Scripts, saved queries and `schedules.json` are only read. `notebooks()` uses `read_text`, not `read_json`, so a torn file is not set aside; the test checks the file is still there. The SQL test checks that the only file written under its temp folder is `governance_checks.json`. `_wh_walk` skips symlinks and caps files per project; files over `MAX_FILE` are reported, not read.
Authentication and distribution	Met	No new scope, no network call. Saved queries, repositories and schedules are local.
No unproven local-equals-Fabric claim; Preview tags kept	Met	Every script is checked as warehouse T-SQL. A saved query meant for a SQL analytics endpoint or a SQL database is still checked as warehouse (finding 3). The docs say this ("SQL rules check each script as warehouse T-SQL", `docs/governance.md:217`).
Docs match the code; linked; strict docs build passes	One gap	`docs/governance.md:127,191,195,215,217` match the code, including the changed meaning of `scheduled_only`. The pipeline-schedule gap is missing (finding 1). `zensical build` not run (not installed here).
Accessibility of new controls	Not applicable	No new controls.
Inventory and backlog updated; `product_data.py all` consistent	Partly	FEAT-GOV-09 known limitations were updated: the SQL gap is removed and the Fabric-schedule limit added. `#798` is not in FEAT-GOV-09's PR list. PB-111 in `PRODUCT_BACKLOG.json` is still `Proposed` with no `prs` (finding 2). `product_data.py all` on a copy of main: "Product data is consistent."

Findings#

#	Severity	Finding	Where	Status
1	should fix	A notebook that only a pipeline scheduled on this PC (#791) runs isn't marked scheduled, so NB004 and NB008 never check it. The PR names this as a known limitation, but `docs/governance.md:217` only mentions Fabric schedules, and so does the FEAT-GOV-09 limitation. Users with a pipeline-run notebook would read a "not checked" result as covering their case. Either say so in both places and track it in a backlog item, or count the notebooks that a scheduled pipeline's notebook activities run.	`local_schedules.py:232-240`; `docs/governance.md:217`; `FEATURE_INVENTORY.md` FEAT-GOV-09	Open
2	should fix	The backlog and inventory don't record the work. PB-111 is still `Proposed` with no `prs`, and FEAT-GOV-09's PR list lacks #798.	`docs/product/PRODUCT_BACKLOG.json` PB-111; `FEATURE_INVENTORY.json` FEAT-GOV-09	Open (with the outcome)
3	note	Every SQL item is checked as `target: "warehouse"`, including saved queries written for an endpoint or a SQL database. Rules whose result depends on the target can then be wrong for those scripts. This is documented.	`checks_service.py:243,265`	Documented
4	note	SQL items and notebook items cover the whole PC (every saved query and every opened repository's warehouse projects), not only the open project's, though the page is "Check this project". This was already true for notebooks (#664 nit).	`checks_service.py:207-266`	Note
5	note	The meaning of `scheduled_only` changed from "never run by Check now" to "only on items marked scheduled". The old test `test_scheduled_only_rules_arent_run_by_check_now` was replaced, which is correct. Any future checker with a `scheduled_only` rule now runs in Check now on items it marks scheduled. The registry docs say so (`docs/governance.md:191`).	`governance_checks.py:15,149`	Note

Documented limitations (only for Accepted with Documented Limitations)#

These apply once finding 1 is fixed.

Limitation	Where users are told	Backlog item that tracks it
NB004 and NB008 don't run on a notebook that only a Fabric schedule runs	`docs/governance.md:217`; FEAT-GOV-09	none yet: needs one
NB004 and NB008 don't run on a notebook that only a pipeline schedule on this PC runs	not yet (finding 1)	none yet: needs one
SQL scripts are checked as warehouse T-SQL	`docs/governance.md:217`	none

Inventory and backlog changes#

PB-111: state → In review (Accepted with Documented Limitations once finding 1 is fixed); prs 798; this record added to evidence.
FEAT-GOV-09: add #798 to the PRs; add the pipeline-schedule limitation next to the Fabric-schedule one, with the backlog item that tracks them; this record added to evidence.