Lessons reach the session that needs them, every kind is a full citizen, and a slow disk no longer takes the instance down #167

Merged

bvandeusen merged 5 commits from dev into main 2026-09-19 12:02:16 -04:00

Conversation 0 Commits 5 Files Changed 19

+1898 -65

bvandeusen commented 2026-09-19 12:02:08 -04:00

Owner

Copy Link

Copy Source

Five commits from dev. CI green on each; 7049 green on 1fcfd47 (1829 passed, integration, lint, typecheck, plugin hooks, image build).

Why this is being merged now

1fcfd47 is the reason the merge is being surfaced rather than stacked (rule 34). It fixes the defect behind today's three-hour outage, and a resilience fix sitting on dev protects nothing — the running instance is still the code that produced the outage, so the next storage stall repeats it.

What is in it

1fcfd47 — a slow database costs seconds, not the instance (#4181).
A host storage stall made one Postgres checkpoint of 14 buffers take 281 seconds against a 1.3-second baseline. The app restarted into the tail of it, get_maintenance_hour() — the first DB read in before_serving — hung with no deadline, Hypercorn killed the worker at its 60-second lifespan timeout, and nothing retries a failed lifespan. A five-minute hiccup became a three-hour outage clearable only by hand.

Three changes, none of which prevents a stall: the startup read is bounded and falls back to the default hour it already used for unparseable values; the embedding backfill no longer races startup for the connection pool, released by a flag set in a finally; and the engine gets a connect deadline (asyncpg's default is 60s — the whole lifespan budget). command_timeout is deliberately not set, with a guard asserting the omission.

3075de1 — a lesson reaches the moment it applies (#3732).
The write-path arm filtered kinds, so lessons were not outranked there but unreachable. Both note arms now reach lessons, and across projects. A reserved lesson_slot in the prompt menu, logging its own retrieval so it is falsifiable, extending rather than evicting. One header clause, only when a lesson is on the menu, saying it binds nothing — deliberately not the rule arms' phrasing, with a guard that the binding phrase never appears.

31b478b — the two new report kinds say what to DO about a duplicate (#4164).
_REPORT_KINDS gained lesson and process without _KIND_SUGGESTION gaining either, so the report would have listed pairs with no advice. Caught by CI's per-kind guard, which now asserts the property rather than spot-checking two.

6a2476a — every typed kind gets all five doors and a duplicate report (#4164).
list_lessons and delete_lesson; lessons and processes into the duplicate report; when_to_apply projected in _note_to_item.

1d201d2 — a lesson can be written, and it keeps every incident that taught it (#3731).
create_lesson / get_lesson / update_lesson, sources as a list in data["taught_by"] with a body mirror, and a 0.96 dedup gate for the kind.

🤖 Generated with Claude Code

https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

Five commits from `dev`. CI green on each; **7049 green on `1fcfd47`** (1829 passed, integration, lint, typecheck, plugin hooks, image build). ## Why this is being merged now `1fcfd47` is the reason the merge is being surfaced rather than stacked (rule 34). It fixes the defect behind today's three-hour outage, and a resilience fix sitting on `dev` protects nothing — the running instance is still the code that produced the outage, so the next storage stall repeats it. ## What is in it **`1fcfd47` — a slow database costs seconds, not the instance (#4181).** A host storage stall made one Postgres checkpoint of 14 buffers take 281 seconds against a 1.3-second baseline. The app restarted into the tail of it, `get_maintenance_hour()` — the first DB read in `before_serving` — hung with no deadline, Hypercorn killed the worker at its 60-second lifespan timeout, and nothing retries a failed lifespan. A five-minute hiccup became a three-hour outage clearable only by hand. Three changes, none of which prevents a stall: the startup read is bounded and falls back to the default hour it already used for unparseable values; the embedding backfill no longer races startup for the connection pool, released by a flag set in a `finally`; and the engine gets a connect deadline (asyncpg's default is 60s — the whole lifespan budget). `command_timeout` is deliberately not set, with a guard asserting the omission. **`3075de1` — a lesson reaches the moment it applies (#3732).** The write-path arm filtered kinds, so lessons were not outranked there but unreachable. Both note arms now reach lessons, and across projects. A reserved `lesson_slot` in the prompt menu, logging its own retrieval so it is falsifiable, extending rather than evicting. One header clause, only when a lesson is on the menu, saying it binds nothing — deliberately not the rule arms' phrasing, with a guard that the binding phrase never appears. **`31b478b` — the two new report kinds say what to DO about a duplicate (#4164).** `_REPORT_KINDS` gained lesson and process without `_KIND_SUGGESTION` gaining either, so the report would have listed pairs with no advice. Caught by CI's per-kind guard, which now asserts the property rather than spot-checking two. **`6a2476a` — every typed kind gets all five doors and a duplicate report (#4164).** `list_lessons` and `delete_lesson`; lessons and processes into the duplicate report; `when_to_apply` projected in `_note_to_item`. **`1d201d2` — a lesson can be written, and it keeps every incident that taught it (#3731).** `create_lesson` / `get_lesson` / `update_lesson`, sources as a list in `data["taught_by"]` with a body mirror, and a 0.96 dedup gate for the kind. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

bvandeusen added 5 commits 2026-09-19 12:02:08 -04:00

feat(lessons): a lesson can be written, and it keeps every incident that taught it (#3731) ... 1d201d2ff7

Milestone 385 step 4 — the write path.

ITS OWN TOOL MODULE, not create_note(note_type="lesson"), on the snippet and
process precedent and for the reason that precedent exists: a kind whose value
depends on one field being filled needs a door that ASKS for that field by
name. create_note would take a lesson through a generic body parameter and the
trigger — the whole of why a lesson is findable — would be something the writer
had to know to include.

THE TRIGGER IS REQUIRED, refused rather than flagged. Step 1 left the choice
open. Refusing is right for the same reason create_rule makes enforcement the
deciding question: a lesson with no trigger is not a weaker lesson, it is a
note that will never surface, and nothing downstream can tell the difference —
it saves, reads correctly in every listing, and is silently absent from the one
moment it was written for. A flag is a warning nobody is present to read; the
write path is where the writer still is. The message says SYMPTOM, because
"required" alone produces a topic where a situation was wanted.

The docstring carries the distinction this milestone exists to fix, in a line a
reader can apply: the difference between a lesson and a rule is FORCE, not
importance. If ignoring it would be a mistake it is a rule and needs the
operator's yes; if ignoring it just means someone re-derives it the slow way it
is a lesson, and nobody is bound.

CARDINALITY: a LIST, in notes.data under `taught_by`. The founding example
generalised three incidents into one claim about failure classes no CI lane can
see — generalising across incidents is the shape a good lesson HAS, and
arose_from_id holds one, so a single id keeps the first and drops two while
reading as complete. It lives in `data` rather than a join table for the reason
decision #4157 put the trigger there: a table would settle, for every note kind
at once, whether provenance is multi-valued — a question nothing has measured.
`arose_from_id` is filled only when there is exactly ONE source, because every
surface that renders it renders it as THE origin, and one of three would make
those surfaces state something false.

THE DUPLICATE GATE, which step 4 asked to check: a lesson is judged at a bar
ABOVE the sibling band, not the general 0.90. #2518 measured deliberately
parallel variants at 0.92 on a document that is mostly prose about the thing,
which is exactly a lesson's shape now — so at 0.90 two genuinely different
lessons about one area ("CI cannot see this class of failure") would refuse each
other. Its own constant rather than reusing the snippet's: the two are separate
facts that coincide today, and this number is inherited from a structurally
analogous corpus rather than measured on lessons, of which there are none yet.

Follows canon #2846 including the third registration point it names and this
change would otherwise have missed: get_lesson is in server._READ_ONLY_TOOLS
and the two writers in _WRITE_TOOLS, which test_mcp_auth requires.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

feat(records): every typed kind gets all five doors and a duplicate report (#4164) ... 6a2476addb

Arising from #3731, which shipped a lesson with three of five tools and logged
the rest rather than widening its own scope. The operator's framing on reading
that: each kind deserves CRUD functions and to show up in the search and report
functions. So this fixes the property, not the two instances.

WHAT WAS MISSING FOR A LESSON: no delete, no list, and no duplicate report.
`delete_lesson` is the #2250 situation exactly — the trash is kind-agnostic so
`delete_note` always reached a lesson, but nothing said so, and a kind whose own
tools offer create/read/update reads as one you cannot retire. `list_lessons` is
the only way to ask what has been learned at all: `get_lesson` needs an id you
already have, and semantic search returns what resembles a query, never the set.

APPLYING THE RULE FOUND THE SAME REPORT GAP FOR PROCESSES, which have had full
CRUD for months and have never been in `_REPORT_KINDS` either. Both are in now,
each compared only against its own kind.

The lesson report default is 0.90 — the general semantic floor, deliberately
BELOW its own write-path bar of 0.96. The gate is permissive on purpose so it
does not refuse two genuinely different lessons whose triggers read alike, and
that tolerance is precisely what wants reviewing later, so the report looks at
the band the gate was told to let through. Safe there and not at the gate,
because a report proposes and the operator picks where the gate blocks a write.

A BUG CAUGHT BEFORE IT SHIPPED: `list_lessons` first read the trigger from
`it["data"]`, which `_note_to_item` does not carry — it projects named keys off
the mirror (`language`, `verification`) rather than the column. Every row would
have listed an empty trigger, which on a kind whose whole point is the trigger
is the failure looking like the feature. `when_to_apply` is now projected there
beside the others, so every listing surface gets it, including step 7's UI.

The guard asserts the PROPERTY rather than the instances: for each typed kind,
all five tools exist, are actually offered by register(), are classified for
auth, and the kind has a duplicate report. Derived from the kinds themselves, so
a fourth inherits the bar. A per-tool test cannot catch a missing tool, which is
why four steps of milestone 385 went green over this.

`find_duplicate_records` now validates against `_REPORT_KINDS` instead of its
own literal — the second copy is what would have refused a kind the service
already supported.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

fix(dedup): the two new report kinds say what to DO about a duplicate (#4164) ... 31b478b7ac

`_REPORT_KINDS` gained `lesson` and `process` without `_KIND_SUGGESTION`
gaining either, so the report would have listed pairs with no advice —
and the suggestion is the report's point. CI's per-kind guard caught it.

Lesson: read both triggers first (a lesson is retrieved by the situation
it names, so alike insights under different triggers are two lessons);
same trigger means one lesson learned twice, so fold the `taught_by`
union into the survivor and delete the other. Process: a process arrives
as a skill, so a duplicate runs the wrong procedure rather than merely
cluttering a list — keep the one in use, fold the missing steps, delete
the loser.

The guard now asserts the property per kind rather than spot-checking
two: no non-snippet suggestion may propose merge_snippets, so a sixth
kind inherits the bar without anyone editing the test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

feat(lessons): a lesson reaches the moment it applies, and says it binds nothing (#3732) ... 3075de19eb

WHICH ARM — the two note arms, and no new one.

`write_path` filters kinds, so a lesson was not outranked there but
unreachable, which is #3702's shape: an arm that never had the candidate
reports a healthy bar. It now asks for lessons alongside snippets and
issues. The founding example of the kind is a lesson about a code shape,
and this is the arm that fires when code is written.

`auto_inject` does not filter kinds, so lessons were already candidates —
but scoped to the bound project, which for a kind whose whole claim is
that it transfers is the same silence. Both arms now pass
`include_global_kinds` (#3730).

WHOSE BUDGET — a reserved slot in the prompt menu, none on the write path.

The step's premise needs a correction: the notes menu and the rule hints
are separate functions with separate budgets, so a line reserved here
displaces a note, never a rule. (`RULEHINT_LIMIT` is also 5, not 1, since
#4102 made it a default rather than a cap.)

The trade taken: a note crowded out is a lost convenience and a rule
crowded out still fires at an act arm, but a lesson crowded out is the
feature failing — a lesson exists only to be met at the moment it
applies, so the arm IS its delivery and the loss is total and silent.
That is `preference_slot`'s argument, and the rarity is `reuse_slot`'s.
It buys position, never a lower bar, and it EXTENDS rather than evicting:
a displaced hit sits in the general search's own log row, and evicting it
would make two tables disagree about one call (#3668, #379).

No slot on the write path: that arm fires before every Write and Edit,
where a guaranteed extra line is a guaranteed extra interruption, and its
field is already just snippets, issues and lessons rather than the whole
corpus.

`lesson_slot` logs its own retrieval and its own surfacing from the first
deploy, and the general contest stays open to the kind — otherwise "the
slot earns its line" would be true by construction.

THE VOICE — "they don't always have to be followed".

The menu's register is already the non-binding one. What it lacked is
that a lesson reads as one more title in a list of material when it is
advice someone paid for. One clause, in the header, only when a lesson is
on the menu: weigh it, use your judgement, it is not a rule and binds
nothing. It deliberately does not borrow the rule arms' "before deciding
it does not apply", and a guard asserts that phrase never appears.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

fix(startup): a slow database costs seconds, not the instance (#4181) ... 1fcfd47ab6

On 2026-09-19 a host storage stall made one Postgres checkpoint of 14
buffers take 281 seconds against a 1.3-second baseline. The app restarted
into the tail of it, `get_maintenance_hour()` — the first DB read in
`before_serving` — hung with no deadline, Hypercorn killed the worker at
its 60-second lifespan timeout, and nothing retries a failed lifespan. A
five-minute disk hiccup became a three-hour outage that only a human
restart could clear. Every MCP call returned 405, which reads like a
routing fault and was nothing of the kind: nothing was serving.

Three changes, none of which prevent a stall — they stop a transient one
becoming a permanent one.

1. THE STARTUP READ IS BOUNDED (rule 156). `get_maintenance_hour` already
   answered `_DEFAULT_HOUR` for a value it could not parse; a database
   that will not answer in three seconds is the same class of "no usable
   value here". The failure is now a WARNING naming the symptom — the
   breadcrumb whose absence meant this was only diagnosable from
   Postgres's own log — and a default run-hour, instead of the app.

2. THE BACKFILL NO LONGER RACES STARTUP. Its comment said it "never
   blocks the server from accepting requests": true of requests, false of
   startup, because the task began while `before_serving` was still
   running and competed for the same pool. Both of the incident's
   cancelled statements were in flight together. It now waits on a flag
   released on the hook's way out — in a `finally`, never after the work
   (rule 157), because an undeadlined wait is only safe when the wake-up
   cannot be missed.

3. THE ENGINE CANNOT WAIT FOREVER TO CONNECT. asyncpg's default is 60s,
   the whole lifespan budget spent before a query is sent. `command_timeout`
   is deliberately NOT set alongside it and the comment says why: it would
   apply to every statement, and this app runs long ones on purpose.

tests/test_startup_survives_a_slow_database.py asserts the shape rather
than the stall: a read that never returns still yields an hour, the
warning names the symptom, a healthy read is unaffected, the backfill
does no work before release, the flag is released even when startup
raises, and the engine's connect args carry a deadline but no blanket
statement timeout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

bvandeusen merged commit 25d8655ad4 into main 2026-09-19 12:02:16 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledScribe#167