diff --git a/tests/test_design_starter_roles.py b/tests/test_design_starter_roles.py index 7f9e79c6..b6ef637c 100644 --- a/tests/test_design_starter_roles.py +++ b/tests/test_design_starter_roles.py @@ -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 diff --git a/tests/test_integration_lesson_rule_links.py b/tests/test_integration_lesson_rule_links.py index fab8bb67..b8b1022a 100644 --- a/tests/test_integration_lesson_rule_links.py +++ b/tests/test_integration_lesson_rule_links.py @@ -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): diff --git a/tests/test_retrieval_surfaces.py b/tests/test_retrieval_surfaces.py index cc524e2c..aa9f13e4 100644 --- a/tests/test_retrieval_surfaces.py +++ b/tests/test_retrieval_surfaces.py @@ -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" diff --git a/tests/test_rule_opened_ledger.py b/tests/test_rule_opened_ledger.py index a06c97b0..c2530738 100644 --- a/tests/test_rule_opened_ledger.py +++ b/tests/test_rule_opened_ledger.py @@ -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 diff --git a/tests/test_settings_defaults_agree.py b/tests/test_settings_defaults_agree.py index e35691cb..bc105249 100644 --- a/tests/test_settings_defaults_agree.py +++ b/tests/test_settings_defaults_agree.py @@ -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(\"\")` 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 diff --git a/tests/test_timestamp_filters.py b/tests/test_timestamp_filters.py index 4f2af640..aadee639 100644 --- a/tests/test_timestamp_filters.py +++ b/tests/test_timestamp_filters.py @@ -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 diff --git a/tests/test_version_endpoint.py b/tests/test_version_endpoint.py index ebe01e73..57ce0d8b 100644 --- a/tests/test_version_endpoint.py +++ b/tests/test_version_endpoint.py @@ -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"), diff --git a/tests/test_version_readout.py b/tests/test_version_readout.py index 3d4d867b..84104f52 100644 --- a/tests/test_version_readout.py +++ b/tests/test_version_readout.py @@ -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