feat: add master notebook library scaffolding and review tooling
Add CLAUDE.md defining the Palladium master notebook conventions and Red Panda Approval criteria, plus a review-notebook slash command for LLM-driven notebook review. Expand .gitignore to block client/engagement documents and generated exports, keeping masters client-clean while allowing text/image sources. Normalize slider widget numeric values from floats to integers in notebook JSON.
This commit is contained in:
117
tests/nbcheck.py
Normal file
117
tests/nbcheck.py
Normal file
@@ -0,0 +1,117 @@
|
||||
"""Structural checks for every master notebook in the library.
|
||||
|
||||
Kernel-free: everything here reads the committed ``.ipynb`` JSON with
|
||||
nbformat — no study venv, no execution. Content and engine pins stay in
|
||||
each master's own ``tests/`` (run in its venv); this layer guards only
|
||||
STRUCTURE: the notebook parses, was executed cleanly top-to-bottom, and
|
||||
(for notebook-first masters) carries the tagged-cell taxonomy the
|
||||
Assessment Pattern requires.
|
||||
|
||||
Classification is explicit and non-silent: every notebook on disk must be
|
||||
listed in exactly one of NOTEBOOK_FIRST or GRANDFATHERED (a completeness
|
||||
test enforces it), so a new master cannot dodge the suite, and every
|
||||
exemption carries its reason — grandfathered notebooks run the structural
|
||||
tier and skip the notebook-first tier with that reason shown by
|
||||
``pytest -rs``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import nbformat
|
||||
|
||||
REPO = Path(__file__).resolve().parent.parent
|
||||
|
||||
# Masters built on the notebook-first content model (Assessment Pattern):
|
||||
# full check set, including the tagged-cell taxonomy.
|
||||
NOTEBOOK_FIRST = {
|
||||
"assessments/CX_Discovery_Workshop/notebooks/cx_discovery.ipynb",
|
||||
}
|
||||
|
||||
# Structural tier only, each with its recorded reason (see CLAUDE.md,
|
||||
# Known liabilities). Redesigning one of these to notebook-first means
|
||||
# moving it up to NOTEBOOK_FIRST — never deleting it from here silently.
|
||||
GRANDFATHERED = {
|
||||
"assessments/CX_AI_Diagnostic/notebooks/diagnostic.ipynb":
|
||||
"generated notebook; pre-dates the notebook-first model (redesign pending)",
|
||||
"studies/202512_TEI_Genesys_CX_Cloud/notebooks/business_case.ipynb":
|
||||
"pre-tag TEI master (redesign pending)",
|
||||
"studies/202602_TEI_Amazon_Connect/notebooks/business_case.ipynb":
|
||||
"pre-tag TEI master (redesign pending)",
|
||||
"studies/202607_CTM_GenesysCX/notebooks/ctm_business_case_corrected.ipynb":
|
||||
"real-client study, frozen (see CLAUDE.md known liabilities)",
|
||||
"studies/202607_CTM_GenesysCX/notebooks/ctm_business_case_no_current_state.ipynb":
|
||||
"real-client study, frozen (see CLAUDE.md known liabilities)",
|
||||
"studies/202607_CTM_GenesysCX/notebooks/ctm_business_case_virtual_agents.ipynb":
|
||||
"real-client study, frozen (see CLAUDE.md known liabilities)",
|
||||
"studies/202607_CTM_GenesysCX/notebooks/ctm_migration_wfm.ipynb":
|
||||
"real-client study, frozen (see CLAUDE.md known liabilities)",
|
||||
"studies/202607_CTM_GenesysCX/notebooks/ctm_token_calculator.ipynb":
|
||||
"real-client study, frozen (see CLAUDE.md known liabilities)",
|
||||
"template/MercuryNotebook/notebooks/business_case.ipynb":
|
||||
"template encodes the py-engine model (rework pending)",
|
||||
}
|
||||
|
||||
ALL_CLASSIFIED = sorted(NOTEBOOK_FIRST | set(GRANDFATHERED))
|
||||
|
||||
# Tags that may appear at most once per notebook.
|
||||
UNIQUE_TAGS = ("topic-bank", "engagement-data", "gate", "data-appendix")
|
||||
|
||||
|
||||
def discover() -> list[str]:
|
||||
"""Every notebook on disk under the master roots (repo-relative)."""
|
||||
found: list[str] = []
|
||||
for base in ("studies", "assessments", "template"):
|
||||
root = REPO / base
|
||||
if not root.is_dir():
|
||||
continue
|
||||
for p in root.rglob("*.ipynb"):
|
||||
if ".ipynb_checkpoints" in p.parts or ".venv" in p.parts:
|
||||
continue
|
||||
if p.parent.name != "notebooks":
|
||||
continue
|
||||
found.append(p.relative_to(REPO).as_posix())
|
||||
return sorted(found)
|
||||
|
||||
|
||||
def load(rel: str) -> Any:
|
||||
return nbformat.read(REPO / rel, as_version=4)
|
||||
|
||||
|
||||
def cell_tags(cell: Any) -> list[str]:
|
||||
return list(cell.metadata.get("tags", []))
|
||||
|
||||
|
||||
def cells_tagged(nb: Any, tag: str) -> list[int]:
|
||||
return [i for i, c in enumerate(nb.cells) if tag in cell_tags(c)]
|
||||
|
||||
|
||||
def execution_problem(nb: Any) -> str | None:
|
||||
"""None if executed cleanly top-to-bottom, else what's wrong.
|
||||
|
||||
Non-empty code cells must carry integer execution counts, strictly
|
||||
increasing 1..N in document order (proof of one clean linear run);
|
||||
empty cells may be unexecuted (``None``).
|
||||
"""
|
||||
prev = 0
|
||||
for i, c in enumerate(nb.cells):
|
||||
if c.cell_type != "code" or not c.source.strip():
|
||||
continue
|
||||
ec = c.get("execution_count")
|
||||
if not isinstance(ec, int):
|
||||
return f"cell {i} has no execution count (notebook not executed?)"
|
||||
if ec != prev + 1:
|
||||
return f"cell {i} has execution count {ec}, expected {prev + 1}"
|
||||
prev = ec
|
||||
return None
|
||||
|
||||
|
||||
def error_outputs(nb: Any) -> list[int]:
|
||||
return [
|
||||
i
|
||||
for i, c in enumerate(nb.cells)
|
||||
if c.cell_type == "code"
|
||||
and any(o.get("output_type") == "error" for o in c.get("outputs", []))
|
||||
]
|
||||
163
tests/test_notebooks.py
Normal file
163
tests/test_notebooks.py
Normal file
@@ -0,0 +1,163 @@
|
||||
"""Repo-level notebook validation — structure only, kernel-free.
|
||||
|
||||
Layer 0 of the testing story (see CLAUDE.md and the pattern docs): runs
|
||||
from the ROOT venv against every master's committed ``.ipynb``. Content
|
||||
and engine pins live in each master's own ``tests/``.
|
||||
|
||||
Tiers (defined in ``nbcheck.py``):
|
||||
structural — every notebook: parses, python3 kernel, no error outputs,
|
||||
cleanly executed top-to-bottom, no duplicate claimed-unique
|
||||
tags.
|
||||
notebook-first — NOTEBOOK_FIRST masters only: the tagged-cell taxonomy
|
||||
(topic-bank / engagement-data / gate / data-appendix) with
|
||||
its uniqueness and position rules. GRANDFATHERED notebooks
|
||||
skip this tier with their recorded reason (``pytest -rs``).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from .nbcheck import (
|
||||
ALL_CLASSIFIED,
|
||||
GRANDFATHERED,
|
||||
NOTEBOOK_FIRST,
|
||||
UNIQUE_TAGS,
|
||||
cell_tags,
|
||||
cells_tagged,
|
||||
discover,
|
||||
error_outputs,
|
||||
execution_problem,
|
||||
load,
|
||||
)
|
||||
|
||||
|
||||
def notebook_first_only(rel: str) -> None:
|
||||
if rel in GRANDFATHERED:
|
||||
pytest.skip(GRANDFATHERED[rel])
|
||||
|
||||
|
||||
# ── Classification is complete — a new master cannot dodge the suite ──
|
||||
def test_every_notebook_on_disk_is_classified():
|
||||
on_disk = set(discover())
|
||||
classified = set(ALL_CLASSIFIED)
|
||||
assert on_disk == classified, (
|
||||
f"unclassified notebooks: {sorted(on_disk - classified)}; "
|
||||
f"classified but missing from disk: {sorted(classified - on_disk)} — "
|
||||
"add each notebook to NOTEBOOK_FIRST or GRANDFATHERED (with a reason) "
|
||||
"in tests/nbcheck.py"
|
||||
)
|
||||
assert not (NOTEBOOK_FIRST & set(GRANDFATHERED)), "a notebook is in both tiers"
|
||||
|
||||
|
||||
# ── Structural tier — every notebook ─────────────────────────────────
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_parses_as_nbformat_4(rel):
|
||||
nb = load(rel)
|
||||
assert nb.nbformat == 4
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_kernel_is_python3(rel):
|
||||
nb = load(rel)
|
||||
assert nb.metadata.get("kernelspec", {}).get("name") == "python3"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_no_error_outputs(rel):
|
||||
assert error_outputs(load(rel)) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_executed_cleanly_top_to_bottom(rel):
|
||||
problem = execution_problem(load(rel))
|
||||
assert problem is None, problem
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_no_duplicate_unique_tags(rel):
|
||||
nb = load(rel)
|
||||
for tag in UNIQUE_TAGS:
|
||||
hits = cells_tagged(nb, tag)
|
||||
assert len(hits) <= 1, f"tag {tag!r} appears in cells {hits}"
|
||||
|
||||
|
||||
# ── Notebook-first tier — the Assessment Pattern tagged-cell taxonomy ─
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_has_exactly_one_topic_bank(rel):
|
||||
notebook_first_only(rel)
|
||||
assert len(cells_tagged(load(rel), "topic-bank")) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_engagement_data_present_and_above_widgets(rel):
|
||||
notebook_first_only(rel)
|
||||
nb = load(rel)
|
||||
hits = cells_tagged(nb, "engagement-data")
|
||||
assert len(hits) == 1
|
||||
# Position: client data must never re-run on a sidebar change, so the
|
||||
# cell precedes the widget-defining presentation cell (identified by a
|
||||
# widget constructor in its source; if a master builds widgets another
|
||||
# way, its own tests carry the precise rule).
|
||||
widget_cells = [
|
||||
i
|
||||
for i, c in enumerate(nb.cells)
|
||||
if c.cell_type == "code"
|
||||
and "presentation" in cell_tags(c)
|
||||
and "mr.Select(" in c.source
|
||||
]
|
||||
if widget_cells:
|
||||
assert hits[0] < min(widget_cells), (
|
||||
"engagement-data cell must sit above the widget cell"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_has_one_gate_with_asserts(rel):
|
||||
notebook_first_only(rel)
|
||||
nb = load(rel)
|
||||
hits = cells_tagged(nb, "gate")
|
||||
assert len(hits) == 1
|
||||
assert "assert" in nb.cells[hits[0]].source, "gate cell carries no assert"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_data_appendix_is_last_code_cell(rel):
|
||||
notebook_first_only(rel)
|
||||
nb = load(rel)
|
||||
hits = cells_tagged(nb, "data-appendix")
|
||||
assert len(hits) == 1
|
||||
last_code = max(
|
||||
i for i, c in enumerate(nb.cells) if c.cell_type == "code"
|
||||
)
|
||||
assert hits[0] == last_code, "data-appendix must be the last code cell"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_presentation_cells_do_not_print(rel):
|
||||
notebook_first_only(rel)
|
||||
nb = load(rel)
|
||||
for i in cells_tagged(nb, "presentation"):
|
||||
streams = [
|
||||
o for o in nb.cells[i].get("outputs", [])
|
||||
if o.get("output_type") == "stream"
|
||||
]
|
||||
assert not streams, (
|
||||
f"presentation cell {i} emits stream output — route diagnostics "
|
||||
"through backstage()"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rel", ALL_CLASSIFIED)
|
||||
def test_content_cells_are_exec_safe(rel):
|
||||
# topic-bank / engagement-data are exec'd by tag in per-master tests
|
||||
# and by scripts — magics or shell escapes would break that contract.
|
||||
notebook_first_only(rel)
|
||||
nb = load(rel)
|
||||
for tag in ("topic-bank", "engagement-data"):
|
||||
for i in cells_tagged(nb, tag):
|
||||
for line in nb.cells[i].source.splitlines():
|
||||
stripped = line.lstrip()
|
||||
assert not stripped.startswith(("%", "!")), (
|
||||
f"{tag} cell {i} uses a magic/shell escape: {line!r}"
|
||||
)
|
||||
Reference in New Issue
Block a user