docs(6209): lesson links, design roles, version readout and endpoint, timestamp filters, settings defaults, opened ledger, surfaces

Eight more test files state their properties without issue, milestone and
rule numbers or dated regressions. The version tests describe what folding
the channel into the version breaks instead of the date it did; the surface
registry and settings-defaults tests state that floors move rather than
quoting the decision that made them move.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-21 21:32:13 -04:00
co-authored by Claude Opus 5.5
parent b3e6a27dc1
commit fa41e006a7
8 changed files with 71 additions and 87 deletions
+10 -11
View File
@@ -1,13 +1,13 @@
"""The starter role set (#2349).
"""The starter role set.
The premise: a literal gets written when there is no role to reach for. This
codebase demonstrated it — the house style had no "text on a filled colour"
role, so 76 call sites wrote `color: #fff` (#2275). Naming the roles at
creation removes the occasion.
The premise: a literal gets written when there is no role to reach for. A
house style with no "text on a filled colour" role collects `color: #fff` at
every call site that needs one. Naming the roles at creation removes the
occasion.
The tests that matter here are about the BOUNDARY, not the content. Roles ship
with the product; values never do. A default palette would be one operator's
taste shipped as product code (rule #115), and it would be very easy to add by
taste shipped as product code, and it would be very easy to add by
accident while "being helpful".
"""
import re
@@ -28,9 +28,8 @@ def test_no_role_carries_a_VALUE():
def _colour_literals(text: str) -> list[str]:
"""Hex colours in `text`, NOT counting issue references.
`#2275` is four hex-valid digits and also how this codebase cites an issue —
so the naive pattern flags its own documentation, which is the third time
that has happened here (#2353). A real colour either contains a letter
`#2275` is four hex-valid digits and also how an issue is cited — so the
naive pattern flags documentation. A real colour either contains a letter
a–f or is a full 6/8-digit value; an all-decimal 3- or 4-digit match is an
issue number.
"""
@@ -64,7 +63,7 @@ def test_the_colour_check_does_not_flag_issue_references():
def test_every_group_is_individually_selectable():
"""Operator's call: one flat list, all skippable. An install that wants
"""One flat list, all skippable. An install that wants
three tokens must be able to get three."""
only_text = roles.starter_tokens(["text"])
assert {r["group_name"] for r in only_text} == {"text"}
@@ -99,7 +98,7 @@ def test_text_on_action_is_in_the_starter_set():
"""The specific role whose absence produced 76 literals. It is separate
from text-primary on purpose: the surfaces it sits on do not change with
the mode, while the page does — so reusing text-primary there passes in
dark and fails contrast in light (#2275)."""
dark and fails contrast in light."""
names = {r["name"] for r in roles.starter_tokens(["text"])}
assert "--ds-text-on-action" in names
assert "--ds-text-primary" in names
+7 -7
View File
@@ -1,4 +1,4 @@
"""Real-Postgres tests for lesson → rule links (milestone 440 step 1, #4630).
"""Real-Postgres tests for lesson → rule links.
What a mock cannot show: that the CHECK holds the three states, that deleting
either end takes the link with it, that a rule the caller does not own cannot
@@ -162,7 +162,7 @@ async def test_one_row_per_pair(world):
await s.commit()
# ── #4631: "no rule fits" and the unjudged list ─────────────────────────────
# ── "no rule fits" and the unjudged list ────────────────────────────────────
async def _judgment(lesson_id: int) -> dict:
@@ -243,7 +243,7 @@ async def test_deleting_the_lesson_takes_its_answer_with_it(world):
assert left == []
# ── #4637: soft links from co-surfacing ─────────────────────────────────────
# ── soft links from co-surfacing ────────────────────────────────────────────
async def _link(lesson_id: int, rule_id: int):
@@ -316,12 +316,12 @@ async def test_confirming_a_suggestion_keeps_what_it_rested_on(world):
assert len(out["evidence"]["situations"]) == 3
# ── #4633: which links can carry a rule ─────────────────────────────────────
# ── which links can carry a rule ────────────────────────────────────────────
async def test_only_a_confirmed_link_can_carry_its_rule(world):
"""A suggested link never expands: it would manufacture the co-arrival
that #4637 counts, and prove itself. A rejected one obviously not."""
that co-surfacing counts, and prove itself. A rejected one obviously not."""
owner, lesson = world["owner"], world["lesson"]
await links_svc.co_surfaced(owner, [lesson], [world["r1"]], arm="p", situation="one ask here")
await links_svc.judge_link(owner, lesson, world["r2"], "reject", "not this")
@@ -335,7 +335,7 @@ async def test_only_a_confirmed_link_can_carry_its_rule(world):
async def test_a_project_rule_stays_in_its_project_even_through_a_lesson(world):
"""Milestone 414's scope, kept on the side door: the lesson surfaces
"""Rule scope, kept on the side door: the lesson surfaces
everywhere, its project rule only in its project."""
owner, lesson = world["owner"], world["lesson"]
await links_svc.set_lesson_rules(owner, lesson, [world["r1"]])
@@ -349,7 +349,7 @@ async def test_a_stranger_reaches_no_rule_through_someone_elses_link(world):
assert await links_svc.confirmed_lessons(world["stranger"]) == set()
# ── #4634: which lessons count toward convergence ───────────────────────────
# ── which lessons count toward convergence ──────────────────────────────────
async def test_only_answered_lessons_are_no_rule_lessons(world):
+10 -19
View File
@@ -1,19 +1,11 @@
"""The surface registry, and the identity the whole tuning story rests on (#4102).
"""The surface registry, and the identity the whole tuning story rests on.
WHY THIS EXISTS
Six arms used to own their own numbers: a settings key, a default, and a limit
that was usually a module constant nobody could change. That survived while the
values were shipped constants. It stops surviving once the operator's decision
is that the numbers MOVE:
"the floor should be chosen and adjusted by the model using it… the user
should be able to touch it but the model should be the thing handling it 9
times out of 10."
A tuning surface cannot be model-consistent across arms that each spell their
configuration differently, so the arms now read `{floor, budget}` from one
registry.
The retrieval floors MOVE: the model using an arm chooses and adjusts them, and
a person can too. A tuning surface cannot be model-consistent across arms that
each own a settings key, a default and a limit spelled their own way, so the
arms read `{floor, budget}` from one registry.
WHAT THIS PINS
@@ -24,9 +16,8 @@ WHAT THIS PINS
different file. Renaming one without the other produces a surface that can
be tuned and cannot be measured, or measured and not tuned, and both fail
silently.
2. **Keys are unique.** Two surfaces sharing a settings key is how the
completion-report arm (since retired) spent its first release moving
whenever the prompt arm was tuned (#3860) — one dial wearing two labels.
2. **Keys are unique.** Two surfaces sharing a settings key move together
whenever either is tuned — one dial wearing two labels.
3. **An unknown surface is refused.** Settings keys are free-form strings in
a generic table, so a typo'd name would write a key nothing reads: a
change that appears to succeed, reports a new value, and alters nothing.
@@ -63,7 +54,7 @@ def test_every_surface_name_is_a_real_telemetry_source():
to justify a change describes a different arm from the one the change hits.
"""
blob = "\n".join(_service_source(n) for n in SERVICES)
# The rule arms record through the one pipeline (milestone 456), where
# The rule arms record through the one pipeline, where
# `source` is the spec's own field — so for them the spec IS the string
# `record_retrieval` receives, and the join key is checked against it.
from scribe.services.retrieval_pipeline import NOTE_ARMS, RULE_ARMS
@@ -81,7 +72,7 @@ def test_every_surface_name_is_a_real_telemetry_source():
def test_no_two_surfaces_share_a_settings_key():
"""One dial, one label. #3860 is what the other way costs."""
"""One dial, one label."""
floors = [s.floor_key for s in rs.SURFACES.values()]
budgets = [s.budget_key for s in rs.SURFACES.values()]
assert len(set(floors)) == len(floors), f"duplicate floor key in {floors}"
@@ -189,6 +180,6 @@ async def test_the_write_path_config_carries_every_arm_it_drives():
"tool_rule_threshold", "tool_rule_top_k",
# Not a surface, and that is why it is spelled out here: it
# has no floor/budget pair to derive from, so nothing else in
# this file would notice it going missing (#4214).
# this file would notice it going missing.
"checkpoint_threshold"):
assert key in cfg, f"{key} missing — its arm will silently no-op"
+10 -11
View File
@@ -1,17 +1,16 @@
"""Named is not read: the ledger's third state (#4100).
"""Named is not read: the ledger's third state.
WHY THIS EXISTS
Milestone 386 made a repeat REFERENCED rather than withheld, and the line it
chose says "You saw it earlier this session". That claim was never checked.
The arms emit a TEASER — title, trigger, `get_rule(N)` — so a session can be
A repeat is REFERENCED rather than withheld, and a line that says "You saw it
earlier this session" claims something nobody checked. The arms emit a TEASER — title, trigger, `get_rule(N)` — so a session can be
shown a rule twenty times and never read a word of it, and a compaction
summarises the teaser away leaving nothing at all. The server was asserting
something about the reader's context it had no way to know.
summarises the teaser away leaving nothing at all. The server would be
asserting something about the reader's context it has no way to know.
`.opened.ids` is the observable half, written by a PostToolUse hook from the
`get_rule` call itself. That is why this is not the self-report 386 rejected:
the objection was to ASKING a model about its own context, and a tool call is
`get_rule` call itself. That is why this is not a self-report: the objection
is to ASKING a model about its own context, and a tool call is
an event the harness reports whether anyone asks or not.
WHAT THIS PINS
@@ -82,8 +81,8 @@ def test_the_three_states_produce_three_different_lines():
def test_held_outranks_seen_regardless_of_kind():
"""`kind` moves the head and the ledger moves the tail; #3497's history is
the two being reasoned about together and one of them being forgotten."""
"""`kind` moves the head and the ledger moves the tail; reasoning about
the two together is how one of them gets forgotten."""
for kind in ("rule", "preference"):
line = _rule_hint_line(_rule(kind=kind), where="here", seen=True, held=True)
assert "opened it earlier" in line
@@ -139,7 +138,7 @@ def test_an_event_with_nothing_usable_records_nothing_and_still_exits_zero(event
# that would be worst to get wrong — is asserted against the running hook in
# tests/test_session_ledger_clear.py, so there is one home for it.
#
# Since #4217 the recorder writes TWO files and only this one is cleared.
# The recorder writes TWO files and only this one is cleared.
# `.opened.ids` says "this context holds rule 156", which a compaction makes
# false, and three hooks read it to decide whether to stay quiet. Its twin
# `.opened.keep.ids` says "rule 156 was opened", which a compaction does not
+6 -7
View File
@@ -1,4 +1,4 @@
"""The Settings UI shows the default the server actually uses (#3927).
"""The Settings UI shows the default the server actually uses.
WHY THIS EXISTS
@@ -17,8 +17,7 @@ WHAT THIS PINS
The RELATIONSHIP, never the number: for each threshold, the Vue ref's initial
string equals the Python default. Retuning either stays free as long as both
move — which is the point, because these are tuning values and #3853 moved one
of them the day this was written.
move — which is the point, because these are tuning values and they move.
The Vue side is read from SOURCE, because that file cannot be imported at all.
The Python side is read from source for a loose constant and IMPORTED for a
@@ -42,7 +41,7 @@ ROOT = pathlib.Path(__file__).resolve().parents[1]
_SERVICES = ROOT / "src" / "scribe" / "services"
_VUE = ROOT / "frontend" / "src" / "views" / "SettingsView.vue"
# THE SIX RETRIEVAL FLOORS now live in one registry (#4102), so their side of
# THE SIX RETRIEVAL FLOORS live in one registry, so their side of
# the pairing is a SURFACE NAME rather than a module constant. That is strictly
# better for this guard: the surface name is also the telemetry source, so a row
# here names the same arm the readout does, and a floor that moved cannot be
@@ -62,7 +61,7 @@ _SURFACE_PAIRS = (
# exist on both sides, so a rename fails loudly rather than silently dropping
# that threshold from the check.
_CONSTANT_PAIRS = (
# The plan gate (milestone 415): it blocks a create, so a form showing a
# The plan gate: it blocks a create, so a form showing a
# looser bar than the one in force would be the more misleading drift. Not
# a push surface, so it has no registry entry and keeps the older shape.
("dedup.py", "PLAN_MATCH_DEFAULT_THRESHOLD", "kbPlanMatchThreshold"),
@@ -87,7 +86,7 @@ def _vue_default(ref_name: str) -> float:
assert m, (
f"{ref_name} is no longer a `ref(\"<number>\")` in SettingsView.vue. "
f"If the control was renamed, update _PAIRS; if it was removed, the "
f"setting has lost its UI and that is the thing to fix (rule 25)."
f"setting has lost its UI and that is the thing to fix."
)
return float(m.group(1))
@@ -112,7 +111,7 @@ def test_the_form_shows_the_floor_the_surface_uses(surface, ref_name):
def test_every_tunable_surface_has_a_control():
"""Rule 25/27: a number an operator may need different has a UI or it is
"""A number an operator may need different has a UI or it is
not shipped. Derived from the registry, so a seventh surface arrives here
as a failure rather than as a setting only the model can reach."""
from scribe.services.retrieval_surfaces import surface_names
+10 -11
View File
@@ -1,4 +1,4 @@
"""Timestamp filters must bind datetimes, never strings (#1727, #2254).
"""Timestamp filters must bind datetimes, never strings.
`AppLog.created_at` is `timestamp with time zone`. asyncpg binds a Python `str`
as VARCHAR, and Postgres has no `timestamptz >= text` operator — so comparing
@@ -22,7 +22,7 @@ from pathlib import Path
SRC = Path(__file__).resolve().parents[1] / "src" / "scribe"
# --- #1727: the reminder dedup window ----------------------------------------
# --- the reminder dedup window ----------------------------------------------
def test_utc_day_start_is_an_aware_datetime_not_a_string():
from scribe.services.notifications import utc_day_start
@@ -38,7 +38,7 @@ def test_utc_day_start_is_an_aware_datetime_not_a_string():
assert (start.hour, start.minute, start.second, start.microsecond) == (0, 0, 0, 0)
# --- #2254: the admin log viewer's date filters ------------------------------
# --- the admin log viewer's date filters ------------------------------------
def test_filter_datetime_parses_dates_and_datetimes_to_aware_utc():
from scribe.services.logging import parse_filter_datetime
@@ -83,17 +83,16 @@ def test_filter_datetime_rejects_garbage_instead_of_raising():
def _timestamp_comparison_offenders() -> list[str]:
"""Every place a `*_at` column is compared against a known-string name.
TWO shapes, because the two real bugs had different ones and a guard built
from only the first would have missed the second:
TWO shapes, because a guard built from only the first misses the second:
1. a local bound to `.isoformat()` — #1727, `today_str = today.isoformat()`
2. a parameter annotated `str` — #2254, `date_from: str | None`, straight
off `request.args`, never touching
isoformat at all
1. a local bound to `.isoformat()` — `today_str = today.isoformat()`
2. a parameter annotated `str` — `date_from: str | None`, straight
off `request.args`, never touching
isoformat at all
Shape 2 is the nastier one: nothing in the file looks date-ish, so grepping
for isoformat finds nothing. It was caught by reading, not by tooling —
which is exactly why it belongs in tooling now.
for isoformat finds nothing. Only reading catches it — which is exactly
why it belongs in tooling.
Deliberately narrow on both. A blanket "no strings near SQL" rule would fire
on every to_dict() in the codebase and get muted, and a muted guard is worse
+8 -10
View File
@@ -1,11 +1,9 @@
"""`/api/version` reports three values, and never folds them together.
WHAT THIS IS ABOUT (rule 149). Until 2026-08-31 the endpoint returned
`{"version": "main"}` — CI set `BUILD_VERSION` to the CHANNEL, so a running
instance answered the question "which build are you?" with the name of a
branch. The cost was concrete rather than theoretical: during #3244's live
acceptance a deploy was behaving as though it held older code, and the one
endpoint whose job is to settle that could not.
WHAT THIS IS ABOUT. A CI that sets `BUILD_VERSION` to the CHANNEL makes a
running instance answer "which build are you?" with the name of a branch — and
when a deploy behaves as though it holds older code, the one endpoint whose job
is to settle that cannot.
The three values answer different questions and so cannot be one value:
@@ -78,7 +76,7 @@ def test_the_channel_is_never_inside_the_name():
})
assert NAME_RE.match(out["version"]), (
f"the version name is {out['version']!r} — not YYYY.MM.DD.HHMM. A "
f"channel or branch name here is the 2026-08-31 bug returning."
f"channel or branch name here is the channel folded into the version."
)
assert "stable" not in out["version"]
@@ -120,7 +118,7 @@ def test_a_malformed_ordering_key_is_dropped_not_passed_through():
def test_the_channel_is_reported_verbatim():
"""Never validated against an enum — a build claiming something
unexpected is better shown than dropped (rule 149)."""
unexpected is better shown than dropped."""
out = _version_payload({"APP_VERSION": "dev", "APP_CHANNEL": "canary"})
assert out["channel"] == "canary"
@@ -138,7 +136,7 @@ def test_ci_does_not_stamp_the_channel_as_the_version():
for wrong in ('BUILD_VERSION="main"', 'BUILD_VERSION="dev"'):
assert wrong not in text, (
f"CI sets {wrong} — that is the channel in the version field, "
f"which is the 2026-08-31 regression."
f"folding the three values into one."
)
@@ -168,7 +166,7 @@ def test_ci_passes_all_three_plus_the_commit():
@pytest.mark.parametrize("commit_epoch,expected", [
# Midnight, where a naive formatter drops the leading zeros and yields
# "2026.01.05.0" — rule 149 names this case specifically.
# "2026.01.05.0" — the case the padding exists for.
(datetime(2026, 1, 5, 0, 0, tzinfo=timezone.utc), "2026.01.05.0000"),
(datetime(2026, 1, 5, 0, 7, tzinfo=timezone.utc), "2026.01.05.0007"),
(datetime(2026, 12, 31, 23, 59, tzinfo=timezone.utc), "2026.12.31.2359"),
+10 -11
View File
@@ -6,11 +6,10 @@ shells. They are deliberately few and deliberately about ONE property each,
because a grep-shaped test that asserts a whole file's contents fails on every
refactor and gets deleted.
WHY THIS FILE EXISTS. #3298: with a deploy misbehaving, nothing on the instance
could say which commit was serving it, and the one endpoint whose job that is
answered with the name of a branch. The value was fixed then. This is the other
half — the value reaching a person — and #3127 checklist 12 is specific about
the way it goes wrong: *never let a blank stand in for `unknown`*. A readout
WHY THIS FILE EXISTS. With a deploy misbehaving, the instance has to be able to
say which commit is serving it. `/api/version` answers that; this is the other
half — the value reaching a person — and it goes wrong one way in particular:
*never let a blank stand in for `unknown`*. A readout
that renders a plausible value it never received is worse than one that renders
nothing, because it ends the investigation instead of starting it.
"""
@@ -25,9 +24,9 @@ FRONTEND = Path(__file__).resolve().parents[1] / "frontend" / "src"
def test_something_actually_reads_the_version_endpoint():
"""The endpoint is not enough; something must ask it.
`/api/version` answered correctly for weeks with no caller — an endpoint
reachable only by someone who already knew to curl it. Rule 27: a
capability with no surface the operator can touch is not shipped.
An endpoint with no caller is reachable only by someone who already knows
to curl it, and a capability with no surface the operator can touch is
not shipped.
"""
hits = [p for p in FRONTEND.rglob("*.ts") if "/api/version" in p.read_text()]
assert hits, "nothing under frontend/src fetches /api/version"
@@ -52,8 +51,8 @@ def test_the_footer_does_not_default_to_a_plausible_version():
decl = match.group(0)
assert '"dev"' not in decl and "'dev'" not in decl, (
f"appVersion defaults to a version-shaped literal: {decl}\n"
"A failed fetch would render as a real-looking version (#3127 "
"checklist 12). Start from a not-answered-yet value instead."
"A failed fetch would render as a real-looking version. "
"Start from a not-answered-yet value instead."
)
@@ -75,7 +74,7 @@ def test_optional_version_fields_are_read_by_absence_not_falsiness():
def test_the_version_request_carries_a_deadline():
"""Rule 156. A wait with no deadline cannot report that it failed.
"""A wait with no deadline cannot report that it failed.
This readout is consulted when an instance is misbehaving, which is exactly
when it may never answer. Without a deadline the surface sits on "still