main
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
14ff41faf5 |
Merge pull request 'feat(rules): rule creation becomes propose-then-approve (#3557)' (#141) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 22s
CI & Build / integration (push) Successful in 30s
CI & Build / Python tests (push) Successful in 1m2s
CI & Build / Build & push image (push) Successful in 19s
|
||
|
|
c3ecdf0972 |
feat(rules): the rule gate becomes a practice with a question, not a prohibition (#3557)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 25s
The first cut opened "NOT YOURS TO CALL UNPROMPTED", and that is the wrong instrument. A caller reading a prohibition stops NOTICING rule-shaped things rather than noticing them and asking — which trades a small failure for a larger one. The wanted behaviour is more proposals, not fewer. So both docstrings now describe the practice: propose readily, state the four things, and close with a question the operator answers in one word — approve it as written / let's talk about it / no. Named options where the interface has them, three written-out options where it does not. "Approve it as written" is what makes element 1 load-bearing: they approved TEXT, so that text is stored verbatim. "Let's talk about it" is framed as the expected answer rather than a setback. "No" routes the observation to create_note, which records without binding. The argument for asking is also better than consent. The operator's yes is the one moment the rule is certainly in front of them: afterwards a conditional rule is not read aloud at session start, and a project rule is absent from an unfiltered list_rules(). The proposal IS the review. Guard gains the answers-offered-back element and drops the wording that forbade; its header records why the framing changed, so the prohibition does not get reintroduced as a tidy-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
1a34363059 |
feat(rules): the rule-creation tools ask for approval, and ask what would enforce it (#3557)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 32s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 41s
Every gate on create_rule and create_project_rule was about SHAPE — rule vs process vs snippet, one-thing-you-could-violate, general-enough, not-a-dupe. All of them improve a rule someone has already decided to write. None asked the prior question: has the person this will bind agreed to be bound by it? Both docstrings now open with the gate, and with the four things a proposal carries: what it would require in the words it would carry, its intent, why now, and how it would be enforced. The fourth is the one that decides it. "A test, a CI check, a hook, a schema constraint... or nothing" is a question that sometimes dissolves the rule: what a test can assert should BE that test, and a rule is what is left when nothing mechanical can hold the thing. A rulebook grows by default and shrinks only on purpose. create_project_rule needs the gate more, not less, and says so: a project rule is absent from an unfiltered list_rules(), and a conditional one is absent from session start too, so one written there can bind for months without ever having been in front of the person it binds. test_rule_creation_asks_first pins structure, never wording — each element matches a family of synonyms, and the gate must precede the Args: block, because a caller who has decided to make the call reads the parameters and not the prose under them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
67df41ae00 |
Merge pull request 'feat(rules): the instruction surfaces say to RETRIEVE a rule, not only to receive one (#3523)' (#140) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / TypeScript typecheck (push) Successful in 3m18s
CI & Build / Build & push image (push) Successful in 16s
|
||
|
|
30d87e461a |
feat(rules): the instruction surfaces say to RETRIEVE a rule, not only to receive one (#3523)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / TypeScript typecheck (push) Successful in 4m18s
CI & Build / Build & push image (push) Successful in 25s
Every instruction surface told a session to load the always-on rules and stopped there. None said the loaded set is partial, so an empty one read as "no rule applies" when it only ever meant "none was pushed" — different claims, and only one of them has been checked. That is #2198's asymmetry one level in. The earlier defect was trusting the SessionStart push over the explicit pull; this is trusting the resident TIER as if it were the whole rulebook. It is also why the always-on tier was the only one that worked, on any install rather than this one (rule 115): a rule nothing retrieves must be resident to bind at all, and a resident rule costs tokens in every session forever — so a rulebook that only delivers cannot grow past what one session holds. Retrieval lifts that ceiling, and it fires only if something asks. A tool-choice reflex asks least of all (#3476, #161). The same obligation now lands on all three session-start surfaces, because rule 119 makes them the specification jointly and a surface stating it differently IS the product behaving differently (#2497). Pinned by test_every_session_start_surface_states_the_conditional_retrieval, mirroring the pull test beside it. THE BUDGET TRADE. _INSTRUCTIONS sat at 1978 against a 2000 test budget, and its own comment says an addition there is a trade, never an append. Bought the new clause by trading out "Processes are saved procedures (follow verbatim)" and "Deletes are trash-recoverable" — both already in DISPLACED_TOPICS and already stated on a delivered surface, and both per-tool guidance, which by this block's doctrine belongs in the tool docstring. Now 1976. Recorded in the comment above the block so it is not silently reversed. Plugin version minted: shipped plugin content moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
0915c48bb0 |
Merge pull request 'feat(telemetry): tell a ranker decline from a repeat before the observation window opens (#3497)' (#139) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 30s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / TypeScript typecheck (push) Successful in 5m20s
CI & Build / Build & push image (push) Successful in 17s
|
||
|
|
8be555d6dd |
feat(telemetry): tell a ranker decline from a repeat before the observation window opens (#3497)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 31s
CI & Build / TypeScript typecheck (push) Successful in 37s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Successful in 28s
Making the rule arms log every call exposed a second ambiguity in the same
row. `result_count == 0` is two unrelated events wearing one number:
- the ranker found nothing above the bar — the only evidence a threshold is
set too high; and
- the ranker found only what this session had already been shown — which
says nothing whatever about the bar.
A long session excludes its way into the second, so the arm reads worse the
longer it runs correctly. Rows written now carry the ambiguity permanently,
which is why this lands before any watch period rather than after.
`retrieval_logs.suppressed_count` (0095, nullable) holds what the caller
dropped as already-shown. Both rule arms report it; they filter in Python and
always know. The note arms pass exclusions INTO semantic_search_notes and
never see what was dropped, so they store NULL.
THE NULL IS LOAD-BEARING. It means "not measured here", and the readout
renders it as `suppression: null` rather than a zeroed dict. Defaulting to 0
would let an unmeasured surface read as a perfectly clean one — the same
substitution of an artifact for a measurement that #3311 made. No backfill,
for the same reason: existing rows genuinely do not know.
`retrieval_telemetry`'s `sources` gains `suppression` with `measured_calls`,
`calls_with_suppression` and `zero_because_already_shown`; subtract the last
from `zero_result_calls` for the true ranker declines. The MCP tool docstring
says to read the two together and warns against reading the null as a zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ
|
||
|
|
aea7b63b62 |
Merge pull request 'fix(telemetry): both rule arms logged only their hits, so the clear-rate could only read 100% (#3497)' (#138) from dev into main
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 30s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 17s
|
||
|
|
48804c437d |
fix(tests): the write-path telemetry test asserted the defect, not the split (#3497)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 28s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 28s
`assert_called_once` held only because the rule arm skipped its retrieval_logs row when it found nothing. With the arm logging every call, the test now asserts what it was always about — exactly one `write_path` row, no `auto_inject`, and the rule arm keeping its own separate source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
154a5de13e |
fix(telemetry): both rule arms logged only their hits, so the clear-rate could only read 100% (#3497)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 22s
CI & Build / integration (push) Successful in 33s
CI & Build / Python tests (push) Failing after 48s
CI & Build / Build & push image (push) Skipped
`write_path_rule` reported `zero_result_calls: 0` and `cleared_threshold: 133/133` — a perfect record no other surface comes near (`write_path` 421 zeroes of 613, `reuse_slot` 124/199, `auto_inject` 114/326). #3311 read that as a measurement and milestone 333 was scoped on it. It was an artifact. Both arms called `record_retrieval` inside a guard on having results — the write-path arm behind `if fresh:`, the pre-tool arm below `if not fresh: return out` — so a call that found nothing wrote no row. The statistic was a fact about the shape of the code, true at any threshold whatsoever. The call log moves out of the guard in both arms. The surfacing log stays in it: nothing was shown, so no surfacing occurred. `results=fresh` is kept deliberately — the note arms pass exclusions into `semantic_search_notes`, so what they log is already post-exclusion, and logging `hits` here would make this row mean something other than every other row in the same readout. The defect bites hardest on the pre-tool arm, which fires on every Bash call: with no rows at all, a ranker that declined is indistinguishable from a hook that never fired — the silent failure the arm exists to stop. Tests cover both arms behaviourally (found nothing; found only what the session already held; searched nothing at all, which must stay silent) plus a structural guard, because this was one level of indentation and it appeared independently in two places. #3311 and the `rule_usage` docstring corrected rather than quietly rewritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
5b02908dfd |
dev → main: rules become measurable at the preload, and retrievable at the tool call (#137)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 17s
|
||
|
|
2ee24b9d2b |
feat(rules): rules before tools — a PreToolUse arm keyed on the action (#3476)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 32s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / Python tests (push) Successful in 1m9s
CI & Build / Build & push image (push) Successful in 25s
The only just-in-time rule surface was registered on `Write|Edit` and queried with `code or path`, so a rule could be retrieved at the moment of a code write and nowhere else. Every rule about which tool to reach for — don't curl the forge, don't stand up a stack, don't run the suite locally, don't branch — was unreachable exactly when it mattered, and residency in the always-on preload was the only surface it had. That is the pressure that grew the resident set to 31 against #3089's ceiling of ~23; it was never a judgment anybody made. A reflex generates no query, so an instruction to check the rules cannot catch one. A mechanical trigger can: the tool call IS the query, and a reflex has to become a tool call before it can do anything. `build_tool_rule_hint` is deliberately tool-agnostic — a name and a string — so widening the matcher later is a hooks.json edit with no server change. The hook starts on Bash, which is where the action reflexes live. The two pre-tool arms share ONE session ledger of already-named rules (`<state>/<sid>.rules.ids`). Two ledgers would mean a rule named by one arm gets re-offered by the other, and the hint that fires most often is exactly the one that must not repeat itself. A test asserts both scripts build the same path, and another checks the shell hook and the Python route agree on every query-arg name (rule 33) — a rename there fails silently, looking like a surface that never finds anything rather than a broken one. Deliberately silent on outage, unlike the prior-art hook: a write is occasional, a Bash call is not, and an outage line before every command is what gets a channel muted. `tier="conditional"` matches the write arm and is the transition point — an always-on rule is already resident, so re-tier one and it starts arriving here instead of in every session's preamble. `pre_tool_rule` joins RANKED_SOURCES: this arm chose what it showed, so a pull can settle whether the choice landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
8b9b3a1d9b |
feat(telemetry): the preload emits, and the always-on set stops being unfalsifiable (#3473)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / integration (push) Successful in 31s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 27s
The ranked rule arm became measurable in M333. The preload did not — and that is the surface whose value is actually in question. `list_always_on_rules`, the SessionStart block and every `rules_payload` caller handed rules over wholesale and emitted nothing, so the resident set's token cost was certain and its usefulness could not be tested even in principle. Bulk deliveries now record as AMBIENT, beside the ranked count and never inside pull-through. Folding them in would mean growing the always-on set depressed the arm's measured precision and trimming it flattered the arm, neither for any reason to do with the arm. `RANKED_SOURCES` inverts the note twin's `AMBIENT_SOURCES` deliberately: there is one ranked rule source and this change adds seven bulk ones, so naming the rare half makes a forgotten surface default to ambient — under-counting it — rather than padding the denominator with surfacings nobody chose. Two lookalike call sites are deliberately left silent, with a test to keep them that way: the write-path etag arm and `rules_etag_for` read the rules to build or compare a MARKER and show nobody anything. No migration — `event` and `source` are plain Text with no CHECK (rule 36 does not apply). Snippet #2858 updated to the new `rules_payload` contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ |
||
|
|
34cd389371 |
dev → main: rule usage telemetry, the plugin's derived version (#136)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / integration (push) Successful in 30s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 16s
|
||
|
|
6627cfc2f0 |
feat(rules): a usage badge on the rule list, and the badge becomes canon instead of a second copy (#3319)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 32s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 33s
Milestone 333 step 5, and rule 27 — the counter had a tuning point from step 4 and no operator-facing one until now. The task said to reuse the snippet badge's classes rather than mint a parallel set, citing the eight duplicated CSS families the ledger already carries (#3207). `.usage-tag` lived in SnippetListView's SCOPED block, so "reuse" was not available: copying it into the rule pane would have been the ninth family, and importing it is not a thing a scoped block permits. So it was promoted rather than copied. Three pieces, each of which existed once and now exists once: - `components.css` gains `.usage-tag` / `.usage-dead`, geometry and colour only, with the scoped original deleted rather than left behind. - `UsageBadge.vue` holds the logic the two lists would otherwise duplicate — the >=3 dead-weight threshold, the empty-string-renders-nothing rule, the tooltip. - `types/usage.ts` holds `RecordUsage`, one client type over two tables. `SnippetUsage` becomes an alias, so no existing consumer changes. THE ADVICE IS A PROP, and that is the substance rather than the plumbing. The counts read identically for every kind; the remedy does not. A snippet offered and never opened should probably be rewritten or deleted — one action. A rule in the same position has TWO possible causes and the operator has to pick: its trigger may fire on the wrong work, in which case `when_to_apply` wants rewording, or it may genuinely not be wanted. Baking "delete it" into the component would give the wrong nudge half the time on the surface where being wrong is most expensive, since a deleted rule stops binding behaviour. The route zero-fills every row through `usage_for_rules`, one aggregate per page — per-row would be N+1 by construction. That matters more here than for snippets: every rule on every existing install predates `rule_usage_events`, so the zero-filled shape IS the common case for a while, and a route that attached the key only where it found events would leave the badge reading undefined on almost every row. `usage_for_rules` had no test at all — step 1 covered the write path and the zero shape and left the aggregate uncovered, which only became load-bearing when a list started rendering it. It now has an integration test over real Postgres, including that a rule with no events comes back zero-filled rather than absent. Recorded as snippet #3460, per the design system's own instruction that the component layer lives as snippets rather than as prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
238510080e |
feat(retrieval): the standing-rule arm gets its own bar, and asks for one rule not two (#3318)
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / integration (push) Successful in 31s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Successful in 35s
Milestone 333 step 4 — the split #2223 made one surface down, now made for the third corpus. The arm inherited WRITEPATH_DEFAULT_THRESHOLD = 0.68, a number measured against code-vs-note-PROSE and never re-derived for code-vs-RULE-TEXT. THE DEFAULT IS ARGUED STRUCTURALLY, NOT READ OFF A HISTOGRAM (rule 115). Two facts hold on any install, including one with six rules and no telemetry: - The eligible corpus is tiny — conditional rules only, a handful to a few dozen against thousands of notes. A top-k over forty candidates always returns something, so "the best match cleared the bar" stops meaning "a good match exists". A bar calibrated for best-of-thousands is cleared by best-of-forty as arithmetic, not relevance. - Rules are short imperative technical English, far more homogeneous than note prose. #2223 put the code-vs-prose floor at 0.55-0.63 and set 0.68 above it; a more homogeneous corpus has a HIGHER floor, so 0.68 is not merely inherited, it sits below where this corpus's noise lives. 0.72 errs deliberately toward silence on an asymmetry that is also structural: this hint fires on EVERY write. A missed rule is recoverable — it is still in Scribe and the agent can search it. A hint that cries wolf is not: it teaches the reader to skip the whole block, and the true positives go with it. The arm's own comment already said "noise on a hint that fires on every write is how a hint gets ignored". Pinned as an INEQUALITY, not a value: test_the_rule_bar_defaults_above_the_code_bar asserts RULEHINT > WRITEPATH, so tuning the number stays free while inverting the relationship — which would silently reinstate #3311 — does not. RULEHINT_LIMIT = 1, and deliberately not a knob. With a corpus this small, k=2 means the second line is almost always the second-best noise wearing the same confident framing as the first; halving k halves that regardless of the bar. It stays a constant because it is a decision about how loud one hint may be, not a per-install tuning question — and a knob nobody turns only adds a way to misconfigure the surface. Reachable from Settings, no restart (rule 25), with copy that says which way to move it and points at retrieval_telemetry's rule pull-through — which step 3 made readable — to tell "arriving unread" from "never arrived". Every config stand-in in the suite gained the key, not just the one that noticed. The arm reads `rule_threshold` while BUILDING its search arguments, so a missing key raises inside its fail-open except and turns the arm into a silent no-op — indistinguishable from it running and finding nothing. That is the same vacuous-pass shape that bit step 2, one layer down (rule 33). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
8901c904a9 |
feat(telemetry): retrieval_telemetry reports rule pull-through where it reported nothing (#3317)
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / integration (push) Successful in 32s
CI & Build / Build & push image (push) Successful in 29s
Milestone 333 step 3, the read half. Steps 1 and 2 built the table and filled it; until now nothing read it, and `usage` — sourced entirely from note_usage_events — described notes only while `sources` happily listed a write_path_rule row above it. A reader takes the aggregate as covering everything named above it. It did not. A SEPARATE `rule_usage` BLOCK, not folded into `usage`. Two reasons, and the second is the one that bites: the corpora differ by orders of magnitude, so a blended ratio would be the note ratio with noise on it and the rule arm would stay invisible inside it; and `usage` is what existing callers already read and compare across windows, so silently changing what it counts would move a number nobody was told had changed meaning. There is a test asserting rule events stay out of the note block. No `ambient` key, unlike the twin. Nothing surfaces a rule un-ranked — list_always_on_rules and enter_project hand rules over wholesale but emit no event — so there is no ambient class to subtract. The absence is a fact about the data, not an oversight, and it returns when a bulk loader starts emitting. Guarded separately, like `by_source`. This table did not exist a commit ago, and an instance running upgraded code against un-migrated schema would otherwise take down two readouts that work perfectly in order to report a third that cannot. On failure the FLAG is added and the SHAPE is kept — a caller must not have to choose between crashing on a missing key and quietly rendering zeros it has no right to. `pull_through` is None rather than 0.0 on an empty window, matching the note block. A ratio of zero asserts "rules were shown and none opened"; with an empty numerator and denominator that is a claim the data does not support, and it is the reading that would make a brand-new install look like a broken one. Also fixed, from #3311: the rule arm never timed its search, so it was the one source in the readout reporting a null p90_duration_ms — a gap that reads as "this surface is somehow not measurable" rather than "nobody passed the number". Both docstrings updated in the same change. The tool's is the agent-facing contract (rule 119) and it explicitly said rule surfacings were absent and had "no usage counter at all". Leaving that would have had a reader conclude the arm has zero pull-through rather than a separate one. Tests are integration for the reason the block above them is: real GROUP BYs and count(distinct) against a table a commit old, in a module whose one production outage was a SQL shape the database rejected inside a broad except. A mock would agree with whatever the code does, including nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
70761b16d9 |
test(telemetry): the rule-arm fixture never reached the arm — it returned at the guard (#3316)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / integration (push) Successful in 28s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 24s
The product code was right; the test was wrong, and wrong in a way that made two assertions fail and two others pass vacuously. `build_write_path_hint` returns early when a write matched nothing at all — no staleness, no synced record, no prior-art menu, no shape signal. The rule arm sits deliberately on the FAR side of that guard, because it runs a semantic search and hoisting it would mean an embedding query on every write in the session. My fixture stubbed every other arm to empty, so it hit the early return and the rule arm never ran: `record_rule_surfaced` was called zero times, and "the recorder was not called" is also what two of the four tests were asserting for their own reasons. The fixture now supplies one prior-art hit — 0.72 against a 0.6 threshold, so it clears the band and the top_k slice — with a comment saying the hit is the arm's precondition rather than scenery. And the gate got its own test, because the fixture now depends on it: a write matching nothing must NOT reach the arm. Without that, a future change to the guard would make every assertion in this file pass without exercising anything. #3311 is explicit that the gate stays until the arm's precision is fixed, so the test says to go read that issue rather than update the assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
8f7f447fda |
feat(telemetry): the rule arm records what it showed, and get_rule records the read (#3316)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 24s
CI & Build / integration (push) Successful in 37s
CI & Build / Python tests (push) Failing after 53s
CI & Build / Build & push image (push) Skipped
Milestone 333 step 2. Step 1 built the table; a counter nobody calls reads zero and looks exactly like a surface nobody uses, which is #2663's shape. SURFACED — the standing-rule arm in build_write_path_hint, beside the record_retrieval it already made. Two tables, and the split is not arbitrary: retrieval_logs is one row per CALL keyed on the score distribution a threshold is tuned from; rule_usage_events is one row per RULE per event, the grain "was this hint ever acted on" needs and the grain a JSONB result_ids array cannot be indexed at. The comment there said rule ids had nowhere to go — that note_usage_events remaps ids on restore, so a rule id would return attached to whatever note took that number. Still true of the NOTE table, and precisely why step 1 built its own. Rewritten to say the gap is closed rather than leaving a stale rationale that would have someone re-derive the same dead end. Records `fresh`, i.e. AFTER exclude_rule_ids. A rule the session already holds was considered and not shown; counting it would inflate the denominator with claims the agent never saw, and the ratio would then fall for a reason that has nothing to do with whether hints land. PULLED — two doors, both after their access check so a refused read is not a pull. mcp_get_rule is the one that matters: the arm's own message ends "Read it with get_rule(N)", so that call is the exact action a landed hint produces. rest_rule carries the other prefix, and the prefix is load-bearing — "is this rule dead weight?" is served by any pull, "did that injected hint land?" by agent pulls only. NOT a pull: rule_history. It loads the rule for its title and its own output says "The current wording is on the rule itself — get_rule(N)", so counting it would credit a read of the history as a read of the rule and double-count anyone who then follows that pointer. list_always_on_rules and enter_project are likewise bulk resident loads, not somebody choosing to open one record. tests/test_rule_usage_wiring.py is cross-cutting on purpose: the surfaced end is in plugin_context, the pull end in two other modules, and "both ends meet" is a property no module-shaped file asserts. It covers the exclusion boundary, that a failing recorder cannot break the write, that a refused read records nothing, and two completeness guards — every door records, and the bulk loaders still do not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
111eef7e30 |
fix(telemetry): the user-scoped rule_usage export read _rule_ids before it existed (#3315)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 25s
Ruff F821, twice, on the same two lines. The query was placed next to its note-usage counterpart — which reads `note_ids`, defined much earlier — while `_rule_ids` is not built until forty lines further down, beside the rules themselves. Moved to sit directly after the `rule_versions` query, which is the other consumer of that variable and the block whose scoping argument this one restates. Worth noting what did NOT catch this. The integration round-trip passed on the same commit: it drives `restore_full_backup` against a hand-built payload, so it exercises the import side and the full export, and never calls `export_user_backup` at all. A per-user export of any account owning a rule would have raised NameError at runtime. The lint lane found it because a static check does not need the path to be reachable by a test. The comment moved with it and got sharper, since the hazard is that the plausible column is the wrong one: `user_id` on a usage row is whoever the arm fired FOR, not who owns the rule, so scoping a per-user export by it would carry this user's surfacings of someone else's rule and drop the ones fired for someone else on theirs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
8826be7a91 |
feat(telemetry): rule_usage_events — the table, the service, and a restore that maps rule ids through the rule map (#3315)
CI & Build / Python lint (push) Failing after 3s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 30s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Skipped
Milestone 333 step 1. The write-path standing-rule arm is the only retrieval surface in Scribe whose usefulness cannot be observed — and, not coincidentally, the only one that has never declined to fire. 296 calls, zero zero-result, 100% clearing its threshold, while every other surface declines most of the time (#3311, and re-measured in note #3430). `retrieval_logs` gives it scores; scores say what the ranker thought, never whether the hint landed. WHY A SIBLING TABLE AND NOT A COLUMN ON note_usage_events. The row carries no note-specific field and the readout is the same shape, which is the strongest case for sharing that note #3163 admits. What decides against it is identity at RESTORE: the note importer maps note_id through note_id_map, so a rule id parked in that column comes back attached to whatever note holds that number in the target database. Not dropped — reattached. The restore reports success, the counters are populated, and every one is about the wrong record, with no other field to disagree with. rule_versions made the same call for the same reason; this is the third rule-side sibling and it reads like the first two. FK-free on rule_id and user_id, matching note_usage_events / retrieval_logs / app_logs, and deliberately unlike rule_versions. A version belongs to a rule's history and dies with it; telemetry outlives what it describes. Deleting a rule must not erase the evidence that it was surfaced forty times and opened never, because that evidence is the case for having deleted it. The service uses `background.spawn` rather than a third copy of the strong-reference dance — that module's own docstring says new callers should, and a fourth copy is how one of them drifts. The AppLog canary #2663 demands is kept, and since `rule_usage` needed exactly `note_usage`'s semantics, that canary moved into `background.report_telemetry_failure` and note_usage now calls it. `retrieval_telemetry` deliberately keeps its own: its canary is a different shape (one process-wide flag, no AppLog row), so repointing it would change behaviour rather than consolidate it. No ambient bucket, and that is a decision. The note twin splits ranked from ambient surfacings because enter_project and the skill sync deliver records without choosing them (#2477). Rules have the same problem waiting — list_always_on_rules loads them wholesale — but nothing emits here yet, so an empty AMBIENT_SOURCES would be machinery pretending to a distinction the data does not contain. `source` stays granular, so the split stays a readout-level change needing no migration. Backup carries it (v14). The round-trip test seeds a NOTE alongside the rule so the target database has a note id to collide with — without that decoy, a restore running rule ids through the wrong map would merely drop them and the test would pass by absence, rather than failing on the populated-and-wrong result that is the actual hazard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
e029a7db64 |
fix(frontend): every request carries a deadline, and expiry arrives as an error callers already handle (#3412)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Python tests (push) Successful in 1m10s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 39s
CI & Build / integration (push) Successful in 34s
CI & Build / Build & push image (push) Successful in 34s
Rule 156, across the whole client. `apiGet`, `apiPost`, `apiPut`, `apiPatch` and `apiDelete` each called bare `fetch`, whose default is to wait as long as the browser will — not a long timeout but the absence of one. The only AbortController in the frontend belonged to the SSE stream and was for cancellation. So every request in the app could hang forever, and there is no state a surface can render for "pending forever" that is not a lie: the spinner that never resolves looks exactly like work still in progress. Found while building the version readout (#3329), which had to tell "the fetch failed" apart from "still loading" and could not. ONE REQUEST PATH. The five verbs were near-identical bodies; they now delegate to a single `request()` that owns the deadline, so a sixth verb cannot be added without one. 30s by default — long enough to clear a cold embedding call and a list view under pool contention (#2384), so tripping it means something is wrong rather than merely busy. Overridable per call via `timeoutMs`. EXPIRY IS AN ApiError, which is the half of rule 156 that is easy to skip. A raw `DOMException: TimeoutError` reaches `apiErrorMessage(e, fallback)` as an object with no `body`, so all ~330 existing catch sites would have printed their generic fallback and the timeout would have been invisible in exactly the situation it exists to expose. Rethrown as `ApiError` with a 408 — a status no Scribe route returns, so it unambiguously means the client gave up — every one of those call sites now reports it correctly, untouched. Only TimeoutError is converted. A deliberate cancellation aborts with AbortError and passes through: a caller that cancelled its own request does not want that surfaced as a server failure. Pinned by a test, because collapsing the two is the obvious "simplification". STREAMS RELOCATE THE DEADLINE RATHER THAN ESCAPING IT. A wall-clock timeout would kill a long-lived SSE connection mid-flight, but two different waits are involved and only one of them is the stream: the CONNECT can fail to answer and now carries a 15s deadline, cleared the moment headers arrive; the BODY stays unbounded on purpose, since its failure mode is going quiet, which a timeout cannot distinguish from being idle — that is what reconnection and Last-Event-ID are for. Reading the connect as exempt because "the stream is long-lived" leaves an unreachable server looking like a quiet one. BULK TRANSFERS get their own value, not the default. Backup, notes export and admin restore walk the whole store and 30s would cut them off mid-work; they carry 10 minutes. Bounded, not unbounded — rule 156 asks for a deadline, not a short one, and no ceiling at all is what leaves a restore that died server-side spinning forever. Four source-inspection guards in the unit lane (no frontend test runner): no bare fetch anywhere; the default is actually applied — pinning the specific regression, since #3329's opt-in shape would pass every other check while leaving 330 callers unbounded; expiry converts to ApiError; and cancellation does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
9bb59b73ba |
feat(frontend): the app says what it is running, and says so honestly when it cannot find out (#3329)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 10s
CI & Build / integration (push) Successful in 31s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 38s
#3127 checklist 12, plus rule 27 — a capability with no surface the operator can touch is not shipped. The step was planned on the premise that nothing read `/api/version`. Two things did, and the state was worse than nothing: - `App.vue` fetched it, wrote `version` into a ref initialised to the literal `"dev"`, and swallowed the error. An instance that could not answer rendered EXACTLY what a healthy local build renders. That is checklist 12's named failure — a blank standing in for `unknown` — in the one readout whose whole job is to say what is running, and it would have made #3298's debugging session no cheaper. - `SettingsView.vue` fetched the same endpoint again on every mount and wrote the result into a local ref no template ever read. A duplicate request whose answer was discarded. So this is not "add a readout"; it is "make the existing one honest, and give it the three fields nobody could see." The readout — Settings → Config, first section, beside the other "what is this instance doing" facts. Three states kept apart, because collapsing any two of them is the defect: not asked yet (tab unopened) nothing answered the values, each ABSENT field as "unknown" the fetch itself failed its own message, with a retry `version` and `channel` prominent, `commit` in full with a copy button so it can be pasted into a `:sha` lookup (rule 145 — the registry's identity and the artifact's own must be checkable against each other), `build` kept because its ABSENCE is the diagnostic part: no ordering key means this build is not in any update order, which is what a local or hand-built image looks like. Absence, not falsiness. The payload omits what it does not know rather than sending `""` or `0` (see `build_version_payload`), so the renderer uses `??` throughout — `build` is a number and `0` is a legitimate ordering key, which `||` would report as unknown. `tests/test_version_readout.py` pins that operator specifically, along with the "no plausible default" property, because `||` is the form a person reaches for by habit. Rule 156 — the fetch carries a deadline. This readout is consulted when an instance is misbehaving, which is exactly when it may never answer; without one the surface sits on "still loading" forever, which is the same blank arrived at from the other direction. `apiGet` gains an OPT-IN `timeoutMs` rather than a default, so no existing call site's behaviour moves. Every other call in the client still has no deadline — reported separately, not fixed here. No frontend test runner exists, so verification is the typecheck lane plus four source-inspection guards in the unit lane, each pinning one property. Also folded in: `plugin/README.md` now leads with the mint script and offers `make` second, since `make` is not installed on every workstation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
f5a3643da8 |
refactor(plugin): retire what the hand-bump scheme left behind — the README that taught it, the floor test, the stale rationale (#3328)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 16s
#3127 checklist 19. The step's own deletion list turned out to be largely spent: `check_version_bump()` came out with #3327, and the machinery the step expected to delete alongside it is load-bearing for its replacement. `manifest_version(ref=…)`, `--base`, `--no-version` and the `origin/main` resolve path all STAY. Derivation makes the value right; it does not make the comparison unnecessary. `check_version_is_minted` still has to ask "did the version move when the shipped content did?", and that is a base-branch question no matter who chose the number. The step was planned before #3327 landed, when the assumption was that these died with the guard. What was actually still standing, all of it teaching or asserting the retired scheme: - `plugin/README.md` told the reader to "set a `version` bump per release." A shipped file, instructing the exact act the mint replaced — this is how a deleted control gets re-added by someone following the docs. Now says not to hand-edit the field, names `make mint-plugin`, and says what a forgotten mint costs. (`make` is not installed on every workstation, so the direct script invocation is given too.) - `test_plugin_version_bumped_with_the_hook` asserted `version >= (0,1,31)` as a tuple of ints. Under a minted value it passes vacuously — every date clears a floor of 0.1.31 — and `int("0415")` silently eats the padding the format exists to keep. Superseded by `test_the_shipped_manifest_carries_a_minted_version`, which asserts the canonical shape instead of an ordering the comparator does not perform. Removed whole (rule 22). - The module preamble still ended on "a written rule that depends on being remembered is not a control; this is" — true of the bump guard, and read as a stronger claim than the mint can support. Replaced with what the change did and did not remove: choosing a number is gone, running the mint is not, and the difference is that forgetting is now loud rather than silent. - An orphaned `# --- the version bump ---` section header with nothing under it, and a test docstring still naming `check_version_bump`. `--no-version` keeps its one legitimate case — on `main` the version is measured against itself — and now says so in both the usage block and its `--help`, so it does not read as an escape hatch. `check_session_context_ reports_its_version` stays untouched: a different check with a different job, and the only thing that makes step 6 readable from a transcript (#2220). Version minted 2026.09.02.0415 -> 2026.09.02.0438 for the README change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN |
||
|
|
64cb719a12 |
fix(plugin): mint() rendered whatever offset it was handed, not UTC (#3327)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 30s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m10s
CI & Build / Build & push image (push) Successful in 15s
Run 5175 red on the Python tests lane. The failing assertion was test_the_mint_is_UTC_not_local, and it was right: `strftime` renders the offset the datetime carries, so mint() only produced UTC because its DEFAULT argument happens to be datetime.now(timezone.utc). Hand it an aware datetime in any other zone and it formats that zone's wall clock -- 22:52Z and its +09:00 twin, the same instant, minted as 2026.09.01.2252 and 2026.09.02.0752. The docstring already claimed "UTC, always", so this was a contract the code did not hold rather than a test asking for something new. Two people minting the same instant would disagree, and the string IS the artifact's identity. Now converts explicitly. A naive datetime is read as UTC rather than as the machine's zone: that is this function's stated contract, and guessing the host's offset is how the same bug returns by another route. Two things found while walking the rest of the module by hand: - test_a_failed_diff_FAILS_rather_than_passing_quietly stubbed EVERY git call to fail, so it tripped the base-branch guard first and passed while proving nothing about the diff arm. rev-parse now succeeds and only the diff fails, and the assertion names the diff message instead of the substring both messages happen to share. - the base-branch failure still said "version-bump check", a name that went away with check_version_bump. The mint script is in the version-relevant set, so fixing it is itself a version-relevant change and forced a fresh mint -- 2026.09.02.0415. That is the asymmetry in #3127 section 3 working as intended rather than a quirk: a format change that did not re-mint would leave the manifest reporting a value the current deriver can no longer produce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DN4zBVFWhBST9YqjCfQmPb |
||
|
|
f1896bfe9d |
feat(plugin): mint the version, and make CI the control that it moved (#3327)
CI & Build / Python tests (push) Failing after 3s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 32s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / Build & push image (push) Skipped
Milestone 334 step 3. 0.1.48 was the last of 48 numbers a person typed by hand; forgetting to type the 49th is #2209, #1040 and #2220, three separate times a shipped fix reached the repo and stopped there. WHY A SCRIPT AND NOT A BUILD STEP. plugin/ is not in the image -- installs fetch it from this repo via marketplace.json, so the push IS the release and there is no moment at which CI could stamp a version in. Every other artifact in the family derives during a build (#3127 section 2). This one has no build to derive during, so the value is minted before the commit and CI's job is to prove it moved when it had to. MINT TIME, a fourth clock section 2 does not name. It prescribes commit time so two lanes building one source report one string; the plugin has one lane and no build, so that reason does not reach it. What is given up is reproducibility-from-history -- you cannot recompute the value, only verify it moved. That is acceptable ONLY because #3325 read the installer's code and found the refresh test is `P.version === H`, plain equality, with zero ordering comparisons anywhere. Where a comparator orders, an unreproducible version would be unverifiable too. Two artifacts in one repo now derive from different clocks on purpose, one directory apart. "Let's make these consistent" is the obvious tidy-up and breaks whichever loses, so the divergence is pinned in tests rather than only explained in a comment -- including an AST assertion that the mint script never imports subprocess, since a mint that can read history is a commit-time deriver wearing the wrong name. check_version_bump becomes check_version_is_minted. It gains the shape gate and a future-value gate, and it keeps deliberately NOT failing when the version moved without content changing: a needless re-mint costs one cache refresh, and failing the lane over a harmless act is how a check earns a --no-version in somebody's muscle memory and stops running at all. The implication that matters is one-directional. The mint script joins the version-relevant set, which is step 2's DERIVERS table finally being read by something. Section 3's asymmetry is why it is not optional: change the format string, change nothing else, and a diff over the shipped paths alone says "no content change" while the manifest keeps a value in the old format forever. Its own introduction demonstrates this -- adding the deriver is itself the version-relevant change that forced this mint. fetch-depth: 0 was NOT added, against this step's own brief. The plugin job carries a comment refusing it, backed by an observed act_runner failure (any `with:` block made checkout fail to extract, run 3027), and the reasoning holds: the check diffs two trees and the workflow already fetches main at depth 1. Checklist 6 is about jobs that derive; this one checks. Verified live before pushing: the session-context marker reports v2026.09.01.2252 keylessly, and both failure arms were probed by hand rather than assumed. The shape gate fires first on a reverted 0.1.48, so the stale arm is covered by unit test rather than by that probe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DN4zBVFWhBST9YqjCfQmPb |
||
|
|
ea972ac3f7 |
refactor(plugin): one definition of what ships, and the exclusion that makes the version check mean something (#3326)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 36s
CI & Build / integration (push) Successful in 35s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 31s
Milestone 334 step 2. The set of files that reach a plugin install lived in
two hand-kept copies -- SHIPPED in check_plugin.py and the workflow's paths:
filter -- with a comment asking a human to keep them in step. That is the
shape #3127 section 3 warns about, and both copies had drifted.
The load-bearing change is the exclusion. The version check reads "did
shipped content change against the base?", and plugin.json lives INSIDE
plugin/ -- so bumping the version is itself a change to the set, which then
reads as the change that justifies the bump. Every bump passed, no bump could
ever fail, and the check proved nothing while looking green.
manifest_differs_beyond_version compares parsed objects with `version`
dropped from both sides. One field, never the whole file: plugin.json also
carries description, mcpServers and userConfig, all of which reach an install,
and excluding the file wholesale would let a userConfig-only edit compute an
unchanged version and never refresh -- #2209 again with a narrower trigger.
Unreadable input answers "changed", because a spurious bump costs one cache
refresh while a missed one is the fix reaching the repo and stopping there.
shipped_content_changed returns None, not False, when the diff fails. #2663 is
why: a read that failed inside a broad except reported the same zero as an
empty window, and every counter read zero for weeks.
Two dead trigger paths removed, both found by writing the guard rather than by
review. fable-mcp/** outlived its directory by three months (deleted in
|
||
|
|
0d4b155699 |
feat(telemetry): pull-through per surface, not just per corpus (#3311)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 30s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 25s
The readout already grouped usage by source — `group_by(event, source)` — and the loop directly below it threw the source away, collapsing every surface into one corpus-wide ratio. So the question a threshold is actually tuned against, "is THIS surface worth its noise", could not be asked of any surface, while the data to answer it sat in the table. `usage.by_source` reports notes_surfaced / notes_pulled / pull_through per surface. The grain is the note, not the call: a pull records the door it came through, not the surface that led there, so grouping the pulled rows by source would answer a different question. Joining surfaced rows to pulled rows on note_id answers this one without the session identity #2085 declined to invent — at the cost of being an upper bound per surface, which the docstring says where it is read. Ambient surfaces report counts and a null ratio: nothing chose those records, so "surfaced often, opened never" is not a judgment about them. A surface that genuinely produced nothing reports 0.0, which must not look like the null. The join is guarded separately from the two reads above it. #2663 was a novel SQL shape the database rejected inside a broad except; this is the novel shape here, and it must not take down two readouts that work. Tests are integration for that same reason — a mock passes on a query Postgres refuses. They pin the distinct-first property (three surfacings of one note are one note), the ambient null, and the LIKE escape, since an unescaped `mcp_%` also matches `mcpXget_note` and nothing else in the payload would show the difference. |
||
|
|
05da26eb24 |
ci(integration): the run: shell is dash, not busybox (#3237)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 27s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m9s
CI & Build / Build & push image (push) Successful in 19s
The runner-facts step answered rule 81's check on its first run, and the answer is the one the check itself warned about: `/bin/sh` resolves to `/usr/bin/dash`, because ci-python is Debian-based. The constraint the rule exists for is unchanged — dash has no /dev/tcp, no arrays, no `[[ ]]` — but the shell has never been busybox, and this comment was repeating the wrong name at the one place a reader would trust it. Rule 81's own statement still says busybox; correcting it is a rulebook edit and goes through propose -> approve -> apply. |
||
|
|
70d84fbfd7 |
ci(integration): print the runner facts that rules 79 and 81 assert (#3237)
CI & Build / TypeScript typecheck (push) Failing after 2s
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Successful in 30s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Skipped
Three conditional rules state facts about this act_runner — services are not reachable by hostname (79), the service container's name is derived from the job's truncated display name (80), and `run:` steps execute under a shell without bash features (81). None had ever been verified, because each check reads "add a step to a live CI job and read the log" and nobody wants to arrange a throwaway run to do it. So the step is not throwaway. Two lines on every integration run turn the next sweep of these rules into a log read. Rule 80 needs nothing new: the container listing the suite step already prints for the name filter is its evidence, and run 5055's log already answers it. Every command is guarded with a fallback. This observes the lane; it must not be able to break it. |
||
|
|
9d8104f7a5 |
fix(embeddings): key_share alone is FOR NO KEY UPDATE, not FOR KEY SHARE (#3262)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 10s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / integration (push) Successful in 24s
CI & Build / Build & push image (push) Successful in 24s
SQLAlchemy spells Postgres's four row locks as a read/key_share pair, so `with_for_update(key_share=True)` renders FOR NO KEY UPDATE — an exclusive lock that two refreshes of the same record would fight over, and that an ordinary concurrent edit would block. The claim needs `read=True` as well to be the FOR KEY SHARE the docstring describes. Caught by the unit test that compiles the statement, which is the whole reason it asserts on the rendered lock mode rather than on behaviour that looks identical either way. |
||
|
|
7827b4ce63 |
fix(embeddings): the index refresh loses the race it used to deadlock (#3262)
CI & Build / integration (push) Successful in 38s
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 37s
CI & Build / Python tests (push) Failing after 55s
CI & Build / Build & push image (push) Skipped
An embedding refresh replaces a record's vectors as delete-then-insert, which takes the chunk rows first and the parent row second (via the insert's foreign key). A cascading delete of the parent takes exactly those two locks in the other order. Postgres calls the cycle a deadlock and kills one side: sometimes the detached embedder, silently, and sometimes the user's delete, as a 500 on an operation that should have worked. Both upserts now claim the parent row with FOR KEY SHARE NOWAIT before touching any chunk row. That removes the cycle instead of narrowing it — either the embedder is first and the delete queues behind it, or the delete already holds the row and the embedder loses at once, which is the side designed to lose. FOR KEY SHARE is the lock the insert would take anyway, so an ordinary edit is unaffected. The note twin, recorded as unverified on the issue, has the same shape and the same fix; a trash purge is the hard delete that reaches it. Unit tests pin the ORDER and the lock mode by compiling the statement; the integration pair holds a real delete open in one transaction and proves the embedder returns having written nothing, with a deadline so a regression fails instead of hanging. |
||
|
|
69ce7afc45 |
fix(ci): /api/version reported the channel where the build belongs (rule 149)
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / Build & push image (push) Successful in 28s
CI & Build / Python lint (push) Successful in 5s
CI & Build / integration (push) Successful in 31s
CI & Build / Python tests (push) Successful in 1m3s
CI set BUILD_VERSION to the CHANNEL — literally "dev", "main", or the tag —
so a running instance answered "which build are you?" with the name of a
branch: {"version":"main"}. 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.
Rule 149's three values, now three fields:
version the NAME, YYYY.MM.DD.HHMM from COMMIT time — "is this the same
code?", so two lanes carrying one commit report one string
build the ORDERING KEY, minutes since 2020-01-01 from BUILD time —
"may this be installed over that?", and the only value anything
may compare
channel its own field. Never a suffix, never a segment of the name
Plus `commit`, so the artifact's claim about itself can be checked against
the :<sha> it was published under (rule 145) — which is exactly the question
that could not be answered tonight.
THE TWO CLOCKS ARE DELIBERATE and look like an inconsistency. The name comes
from the commit so two lanes building one source agree; the key comes from
the build so it cannot go backwards when an older commit is rebuilt. A test
pins both derivations against being "tidied" into one.
ABSENT RATHER THAN EMPTY when unknown. A local build has no ordering key and
no channel; emitting "" or a placeholder would let it claim a position in an
update order it is not part of. A malformed key is dropped rather than passed
through — a reader that cannot order is correct, one that orders on garbage
is not. The key is an int, because a string ordering key is how a comparison
silently becomes lexicographic ("9" > "10").
The payload builder is extracted from the route so it can be tested as a
dict rather than through app startup and a request context.
Tests pin the SHAPE the lanes emit, not the values, including the midnight
leading-zero case rule 149 names specifically — and assert CI never stamps a
branch name as the version again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
b267037911 |
Two milestones: a note can carry its own check (317), and a rule keeps what it used to say (323) (#135)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 38s
CI & Build / integration (push) Successful in 55s
CI & Build / Python tests (push) Successful in 1m37s
CI & Build / Build & push image (push) Successful in 29s
|
||
|
|
7985f8c7d7 |
fix(scribe): a delete must not depend on the lookup that names it (#3273)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 38s
CI & Build / Python tests (push) Successful in 1m15s
CI & Build / Build & push image (push) Successful in 36s
CI caught the naming work reaching for Postgres from the unit lane, and the connection error was the symptom of a real design fault rather than a test gap: reading the title BEFORE the delete put a live query on the delete path, so a lookup that failed would have stopped the delete happening. That is a decoration breaking its payload — the same mistake just fixed in rules_etag, made again two commits later. Every title lookup now fails open: delete_task, delete_note, delete_milestone, delete_snippet and rule_history lose the name, never the operation. The five unit tests mock the lookup rather than reaching for a database, and delete_note gains one asserting the delete still happens when the lookup raises — the behaviour, not just the absence of a crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
efabba58dd |
fix(rules): the staleness signal must not wait for prior art to match (#3244)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 24s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Failing after 52s
CI & Build / Build & push image (push) Skipped
CI caught two things, and the first is the feature not working rather than a test being wrong. THE SIGNAL WAS GATED ON A COINCIDENCE. build_write_path_hint returns early when no prior art, stamp, divergence or derive matched, and that guard sat ABOVE the new arm — so a session whose rules had changed was told only if the file it happened to be editing also matched something else. A staleness signal that fires on that coincidence is not a staleness signal. The arm now runs above the guard, collecting into its own list that `lines` is seeded from, and the guard accounts for it. The standing-rule arm (milestone 307) is deliberately LEFT below that guard, and this is a finding rather than a fix: it has the same gating and probably should not, but it runs a SEMANTIC search, so lifting it would put an embedding query on every write in every session. That is a cost decision, not a bug fix, and not this task's to make. THE MARKER MUST NOT BREAK THE PAYLOAD IT DECORATES. rules_etag is computed on the SessionStart path, where `max()` raising costs the whole context payload — every rule title, the project, all of it — to save a hint. A row with no usable timestamp is now skipped and a set with none degrades to a count-only marker, which still catches a rule added or deleted and only loses edits. That is the right way round to lose information. CI found it because build_session_context's tests pass MagicMock rules and `max()` over those raises TypeError. Also: list_always_on_rules on an install with no always-on rulebooks returns `rules_etag: "empty|0"`. Its exact-dict test is updated rather than loosened — the key being present on an empty install is the behaviour, not noise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
5c9bb40777 |
feat(rules): a session is told when its rules move under it (#3244, milestone 323 step 5)
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 32s
CI & Build / Python lint (push) Successful in 6s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / Python tests (push) Failing after 1m0s
CI & Build / Build & push image (push) Skipped
The rules payload carries a marker; the write-path hook hands it back; the server says which rules moved. Nothing is said when nothing moved. THE COUNT IS NOT DECORATION. max(updated_at) alone cannot see a DELETED rule — it moves no timestamp — and that is the single change that takes an instruction OUT of force, which is the one a session most needs to hear about. The marker is `<max updated_at>|<count>`, and a deletion is reported through the count because there is no row left to name. THE HOOK IS THE CARRIER because it already fires before a write, which is the moment acting on a stale rule costs something. One comparison, no payload, no extra round trip. WHERE THE MARKER IS CAPTURED, and it could not be anywhere else: the SessionStart hook, from /api/plugin/context. The model also receives one from list_always_on_rules, but a hook cannot see an MCP tool's result — so the value the write path compares has to be stored where a shell script can reach it. Keyed by session id in the state dir the prior-art hook already uses, so "changed since" means since THIS session loaded its rules. NOT ON rules_payload, against the task's letter. Those are applicable_rules — a different, subscription-derived set. One key name over two sets is how a comparison starts reporting phantom changes, and the write path compares against the always-on set. WHAT IT CANNOT SEE is stated in both the service and the write-path arm as a table, because a reader who finds an etag will assume it covers staleness generally: another session edits a rule mid-flight | caught the session is misremembering a rule read hours ago | caught compaction summarised the rules out of context | NOT caught The third is the most common, and the marker is blind to it — the etag was in context too and went with the rules. The SessionStart nudge is that case's only mechanism and must not be softened because this shipped. A test asserts both modules still explain that. Instance-agnostic (rule 115): an install with no rules produces a stable marker rather than an error, and "no rules" reads as a state rather than as a change. An unreadable or absent marker reports nothing — a signal that cries wolf is worse than none, because it trains a reader to skip the line that will one day be true. The arm fails open like every other arm on this hook. The delivery is tested through the real build_write_path_hint rather than the helper alone: the feature IS a line arriving in a session, and the arithmetic being right proves nothing about that. Live acceptance is deploy-gated and not yet recorded on the task. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
a8b2040216 |
feat(rules): the edit history is visible in the slide-over (#3243, milestone 323 step 4)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / integration (push) Successful in 35s
CI & Build / Python tests (push) Successful in 1m20s
CI & Build / Build & push image (push) Successful in 40s
Rule 27: a history nobody can read is not shipped. `RuleHistoryPanel.vue` sits below the fields in `RuleEditorSlideOver`, where a rule is read in full — not on the list row, where a history entry point would compete with the row's job. REUSE, DECIDED FIELD BY FIELD RATHER THAN ALL AT ONCE. DiffView.vue is reused unchanged: it takes DiffLine[] and nothing note-shaped. HistoryPanel.vue is NOT, and its props are the reason — noteId + currentBody, a NoteVersion carrying tags and pin columns, a fetch of /api/notes/…, a restore emit, pin/unpin buttons. Rules have no tags, no pins, and deliberately no restore, and a rule's text is EIGHT fields rather than one body, which changes the reader's question from "what changed" to "which fields moved". Recorded here rather than forked silently, per #3207. THE FORK THAT WAS ALREADY THERE. The LCS walk existed three times — privately in useAssist.ts, and again inside HistoryPanel.vue and VersionHistorySection.vue — character-identical apart from quote style, because computeDiff was never exported. Rather than add a fourth copy, it moves to utils/diff.ts and the three become imports; the extraction was verified equivalent to all three before anything was deleted. DiffLine is re-exported from useAssist so its existing importers are untouched. WHAT A ROW SHOWS: when, and which fields moved. A version holds the text the edit REPLACED, so the edit is the step from a row to the next NEWER state — the row above it, or, for the newest row, the rule as it stands now. Comparing against the row below would attribute every change to the wrong edit. A field nobody has fetched yet reads as neither changed nor unchanged. An edit that touched verify_with is badged "check reset", because that edit silently cleared verified_at (milestone 312) and put the rule back at the top of the staleness sweep — a moment visible nowhere else. The badge is a 12% color-mix TINT, not solid `--fs-warning`. `--fs-warning-fg` is defined in theme.css as "warning TEXT on a warning tint", so painting it over the solid token is exactly the same-hue contrast failure #3141 records. Every var() the component references resolves against theme.css, checked before pushing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
0704988528 |
feat(rules): the history is readable — service, REST and MCP (#3242, milestone 323 step 3)
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / integration (push) Successful in 38s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 14s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Successful in 30s
`list_rule_versions` / `get_rule_version` in the rulebooks service, a pair of REST routes beside the note-version ones, and an MCP `rule_history` tool. The ACL-scoped reads live in services/rulebooks.py rather than in services/rule_versions.py because rulebooks already imports rule_versions for the write path and the reverse would be a cycle. It is also the honest split: rule_versions owns what a version IS, rulebooks owns who may read one. Scoping is through the RULE, never the version's user_id, and both directions of that mistake are now pinned by tests. That column is the ACTOR — scoping by it would hand someone the snapshots they personally wrote on a rule that has since moved out of their reach, and would hide from the rule's owner every edit anyone else made. `get_rule_version` takes the rule id as well as the version id so the ownership check and the fetch agree about which rule is in play; the test for that uses a second rule the caller genuinely owns, because a nonexistent id would pass on the ownership check alone and prove nothing. An unreadable rule returns None, not an empty list. The two mean different things — "not your rule" versus "never reworded" — and the MCP tool keeps them apart: None raises, empty says so in band. THE DIFF QUESTION, ANSWERED — and the task's premise was half wrong. It says "notes have DiffView.vue and a diff endpoint already". The component exists and is reusable as-is: it takes `DiffLine[]` and nothing note-shaped, so step 4 can render a rule diff with it unchanged. The ENDPOINT does not exist — diffs are computed client-side by `computeDiff` in useAssist.ts. So no diff route is needed here, and none was written. For the MCP door the answer is different again: an agent has no client to compute a diff, but it also does not need one. Each entry holds the text the edit REPLACED, so "what did this say before the most recent change?" is the first entry, and the text that change produced is the rule as it stands. The docstring says so, and a test pins that sentence — read the other way round, every diff comes out backwards. No restore, per the task. Putting an old wording back goes through update_rule, which snapshots what it replaces, so the undo stays visible like any other edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
255c43a8fe |
fix(tests): the rule-history fixture must not delete a book the embedder is still writing (#3241)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Successful in 31s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m17s
CI & Build / Build & push image (push) Successful in 16s
The integration lane failed on a deadlock, not an assertion — 103 passed, and the one error was in teardown: `DELETE FROM rulebooks` blocked against another process holding a lock on a rule row. `update_rule` fires a detached asyncio.create_task(upsert_rule_embedding(...)) that opens its OWN connection and UPDATEs the rule it just saved. The teardown's rulebook delete cascade-locks that same row, and Postgres resolves the cycle by killing one of them. The sibling test_integration_rule_verification never hit this because it calls update_rule but never deletes its rulebook. Cleanup moves to setup, which runs on a fresh loop after the previous test's loop has closed and cancelled whatever it left in flight. That also keeps the #3240 constraint intact: no database call after a yield. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
6fa66f202b |
feat(rules): an edit leaves behind what it replaced (#3241, milestone 323 step 2)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Failing after 33s
CI & Build / TypeScript typecheck (push) Successful in 42s
CI & Build / Python tests (push) Successful in 1m9s
CI & Build / Build & push image (push) Successful in 30s
`update_rule` now snapshots the rule's text before it writes. Rescoping rule 79 meant hand-copying the superseded statement into a task log to keep it (#3237); the history is that, done by the write path instead of by somebody remembering. The snapshot is taken BEFORE the field loop, which is the one ordering that matters. `update_rule` drops `verified_at` when `verify_with` changes, and rewriting a check is exactly the edit whose history is worth most — a snapshot taken afterwards would file the NEW check against the OLD wording. Taking it up front also covers `clear`, which is a separate argument from the field loop and is how a rule that stops being a constraint loses its check entirely. Session-bound rather than opening its own like note_versions.create_version: the version and the edit that caused it commit together, so a failed update cannot leave a history entry for an edit that never happened. Two guards from the sibling are deliberately absent, and both are now pinned by tests rather than only by comments — "make it consistent with note_versions" is a plausible-sounding change that would silently start dropping history: - No MIN_VERSION_INTERVAL_SECONDS. That 300-second gate exists because note autosave fires every 60. Every version here comes from a deliberate update_rule, so three edits in one second are three edits. - No MAX_VERSIONS and no pruning. A rule is edited a handful of times in its life; a cap could only ever discard the one edit somebody went looking for. Kept from the sibling: the identical-content skip. Both doors resend every field, so without it a form saved twice would file an identical snapshot. `order_index` is excluded from the snapshot fields for the same reason — reordering a rulebook is not an edit to what any rule says. No delete-time snapshot, against the task's original scope and on the operator's call. A delete goes through trash_svc and is SOFT: the rule row keeps its full text and restores untouched, so there is nothing for a snapshot to preserve. Anything that survived a purge would be data the operator explicitly asked to be gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
7a0dc93270 |
fix(tests): the rule-version round trip must not touch Postgres after dispose (#3240)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / integration (push) Successful in 30s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 23s
`_dispose_engine` is a usefixtures entry, so it sets up AFTER an autouse fixture and tears down BEFORE it. The purge running after this file's `yield` therefore opened a fresh pooled connection that the closing loop immediately orphaned, and the next test to touch Postgres died on "Future attached to a different loop" — two of this file's own tests and test_run_maintenance_vacuums_real_tables, which shares nothing with it but the engine. The autouse fixture is setup-only now, matching its sibling in test_integration_backup_note_roundtrip.py, and the cleanup moved into `restored`, whose teardown runs while the engine is still live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
9006affda8 |
feat(rules): a rule keeps what it used to say — rule_versions (#3240, milestone 323 step 1)
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Failing after 31s
CI & Build / Python tests (push) Successful in 1m3s
CI & Build / Build & push image (push) Successful in 23s
The sibling notes already had. `note_versions` snapshots a note's every meaningful edit; a RULE, which binds behaviour on every session that loads it, had nothing — an edit destroyed the previous wording with no record anywhere. Rescoping rule 79 meant hand-copying the superseded statement into a task log to keep it (#3237). The more consequential record had the weaker protection. Schema and transport only. Nothing writes a version yet — that is step 2. Three guards are deliberately NOT copied from note_versions, each defending against autosave, which rules do not have: no pruning or MAX_VERSIONS, no pin columns, no minimum interval. A rule is edited a handful of times in its life, and capping invites losing the one edit somebody needed. `user_id` is the ACTOR rather than the owner, and SET NULL rather than CASCADE: deleting a user must not erase the history of the rules they edited. The restore diverges from its NoteVersion sibling accordingly — an unmappable user leaves the row with a null actor instead of dropping it, which is the whole point of choosing SET NULL. The integration round trip pins that, because nothing in the code says which of the two shapes is intended and "make it match the sibling" would silently delete the record. Backup goes to v13. Both export paths carry the table; the per-user one scopes through the rule rather than the version's user_id, or it would carry the versions this user wrote on someone else's rule and drop the ones someone else wrote on theirs. The restore remaps rule_id through rule_id_map — #3182's arose_from_id trap on a new table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
9657478500 |
docs(notes): the guidance gains the sharper test and the three false candidates (#3180, milestone 317 step 6)
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / integration (push) Successful in 35s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Python tests (push) Successful in 1m9s
CI & Build / Build & push image (push) Successful in 29s
Step 6 measured the claim the whole milestone rests on — that the notes corpus divides into norms and constraints, with constraints a minority worth sweeping — against a stratified sample of the real thing. It holds: ~29% in `reference`, ~8% general, ~0-5% in `decision`, 0% in `dev-log`; roughly 6-11% of ~395 plain notes. Written up as note 3210. The step was allowed to return "revert" and does not. Two things the measurement found that the guidance did not say, now added to both the skill and the create_note docstring: A SHARPER TEST. Every note that earned a check was about somebody ELSE's software — a signing service, a forge, a hub, an SDK, a model, a dependency set. Not one was about the operator's own code. "Is the thing this note describes yours to change?" is decidable from the title in nearly every case, where the abstract form needs thought. THREE FALSE CANDIDATES, one of them a live hazard. Resume pointers and "current state" notes go stale faster than anything else in the corpus, which is exactly why they tempt — but the cure is to update or delete them, not to schedule a check, and a sweep full of pointers is a sweep nobody reads. Measurements of our own system go false because we changed something and knew. And a decision RESTING on someone else's behaviour is still a decision — the check belongs on the note asserting the fact. Also recorded, not fixed: the corpus already contains a note titled "CONSTRAINT: software only — no DIY hardware", using the word for a self-imposed scope limit — a NORM in this taxonomy, exactly backwards. Both surfaces already lead with the question rather than the label, which is the mitigation; note 3210 names the collision so it is not rediscovered. |
||
|
|
1d65e98ac2 |
docs(notes): the surfaces say WHEN a note earns a check, and a guard keeps them saying it (#3168, milestone 317 step 5)
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 43s
CI & Build / Python tests (push) Successful in 1m16s
CI & Build / Build & push image (push) Successful in 28s
The step that decides whether steps 1-4 were worth building. `verify_with` is a free-text field on the highest-volume record kind in the product; described only as "how to verify this note" it gets filled in on every note within a week, and at that point the sweep returns the whole corpus and means nothing. The signal is not "has a check" — it is "has a check AND almost nothing else does". Rule 119 puts this in the app's own instruction surfaces, never in a Scribe rule. So: - the using-scribe skill gains the reflex, next to "state updates in place; chronicles don't" — its other half, since supersession only fires once somebody has read a note and disagreed, which is the case where it was already believed; - update_note's docstring now states the DEFAULT rather than only deferring to create_note for the test. Found by the new guard on its first run; - plugin 0.1.48. THE _INSTRUCTIONS BUDGET, decided rather than skipped. The payload is ~1980 of the client's ~2048-char cap, so everything in it competes for the last ~68 characters. The operator declined a line for this milestone: the map's own closing sentence says each tool's description carries the full contract, and the sweep is a curation act rather than a session-start reflex like enter_project or list_always_on_rules. That reasoning is now a comment beside the constant, with the accepted cost named — an agent that never opens create_note's docstring never learns the field exists — so the question is not re-litigated blind. The guard pins STRUCTURE, never wording, for the disambiguator's reason (a test that punishes rewriting is a test that gets deleted): each write surface must still draw the norm-vs-constraint distinction, say the empty case is normal, and name where NOT to reach for it — plus that the skill carries the one-question form, because the docstrings only reach a caller who already opened the tool. `_doc` moves to tests/helpers as `tool_doc`; it had been written twice. |
||
|
|
1ec44071d2 |
feat(ui): a note's check is editable, dated and sweepable (#3167, milestone 317 step 4)
CI & Build / TypeScript typecheck (push) Successful in 39s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Python tests (push) Successful in 1m12s
CI & Build / integration (push) Successful in 1m41s
CI & Build / Build & push image (push) Successful in 1m1s
Rule 27: no UI, no ship. Three surfaces. THE EDITOR ASKS, but only where the answer can be saved: the fields appear for a plain note and not for a task or a snippet, matching the service gate from step 2 so the form never offers a write the save would reject. The labels are phrased as the QUESTION rather than the field name — "how would someone check this is still true?" and, underneath, "could this become false without anyone editing it?". "Verify with" gets filled in on every note; the question gets filled in on the few that can go stale. `expires_when` appears only once a check exists, and asks for a state rather than a date in the placeholder itself. THE NOTE SHOWS ITS AGE beside the field — "checked 2026-08-28" or "never checked", italic, and nothing at all when no check exists. No red/amber ramp, matching RuleSweepPane: a colour scale would restate the sweep's ordering and force an invented staleness threshold. "Never" is marked because it is categorically different from a date, not a worse one. THE SWEEP is a pane in the Knowledge view, not beside the rules sweep — operator's call, taken over a unified "everything due" surface and over a second pane under /rules. Notes stay where notes live. The cost, accepted knowingly: no single screen shows every unconfirmed record. It REPLACES the feed rather than filtering it, because a facet answers "show me this kind" and this answers "show me what nobody has confirmed" — a question the type chips cannot narrow without under-reporting. Two REST routes for it, since step 3 built only the service and the MCP door. Along the way: NoteEditorView spelled its write payload out at three call sites (save, create, auto-save), so every new field had to be added three times — which is how one of them ends up not carrying it. Now one `payload()` and one `snapshot()`. Known and filed, not fixed: NoteSweepPane copies ~12 scoped CSS rules from RuleSweepPane (#3207). The clean extraction needs prefixed names, because `.age`, `.row-title`, `.lede` and `.actions` all exist scoped in other components and an unscoped global would leak into them — which means editing the shipped rules sweep, blind, inside a step whose acceptance is the operator looking at a different surface. |
||
|
|
b51621fca7 |
fix(tests): the sweep assertions read the WHERE clause, not the SELECT list (#3166)
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / integration (push) Successful in 25s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / Build & push image (push) Successful in 24s
select(Note) names every column, so searching the whole statement for
"notes.note_type" always finds the projection, and sql.index("notes.id") finds
the first column rather than the ORDER BY tiebreak. Both tests were asking the
wrong string.
The ordering test now asserts on the END of the statement, and the filter test
reads the WHERE clause — extracted by regex rather than split on a literal,
because the exact whitespace SQLAlchemy puts around WHERE is not something a
test should depend on.
The product is unchanged: the two assertions that mattered — NULLS FIRST
present, and no legal-carrier filter in the predicate — were both already
true.
|
||
|
|
8489206224 |
feat(notes): the sweep — which notes assert a fact nobody has confirmed (#3166, milestone 317 step 3)
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Failing after 46s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Successful in 28s
The read half. `notes_due_for_verification` + `mark_note_verified` + the MCP
pair, ordered `verified_at ASC NULLS FIRST`: never-checked outranks
checked-long-ago, because a note nobody has ever confirmed is a claim with no
evidence behind it at all. Postgres sorts NULLs LAST on ASC by default, so
getting this wrong would not error — it would silently invert the one signal
the sweep exists to carry, which is why it has a test of its own.
A SIBLING of rules_due_for_verification, not a shared implementation, and this
milestone is a deliberate self-application of note 3163: the row could have
been shared, the QUERY could not. That sweep scopes by rulebook ownership XOR
project ownership because rules have no sharing ACL at all; a note scopes by
the note ACL — browse, not read, so a record shared one-to-one never arrives
in a passive surface unasked (decision 2094).
What genuinely IS common moved to services/verification.py: how a stamp reads,
how old it is, and the three states `last_verified` distinguishes — None ("a
decision, the question does not apply"), "never" ("a fact nobody has
confirmed"), a date. Rulebooks now imports it rather than defining it, so this
is a consolidation and not a third copy.
A failed check writes NOTHING, carried over from 312: there is no "verified
false" state, because a note whose check failed is not in a special condition
— it is WRONG, and recording the failure as a flag would let it sit there
being false with the sweep quietly satisfied that somebody had looked.
Two decisions worth naming:
The sweep does NOT filter to non-task, non-snippet records even though the
write path permits a check on nothing else. Such a row would be in an ILLEGAL
state and this is the one surface that could say so; hiding it to match the
invariant would make the sweep agree with a database it had stopped
describing.
A negative `older_than_days` raises instead of meaning "everything" — silently
answering a different question is the failure shape this guards.
`notes_due_for_verification` is classified read-only in server.py, spelled out
because its name matches none of the prefixes the completeness test derives
from. `rules_due_for_verification` is in the same position and is NOT listed,
so it fails closed for read keys today — filed as #3191 rather than fixed
here, since widening an auth boundary on a tool I did not write is the
operator's call.
|
||
|
|
4736a0a0ba |
fix(tests): the recurrence stand-ins come from fake_note, not a bare MagicMock (#3164)
CI & Build / Python lint (push) Successful in 5s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 26s
CI & Build / Build & push image (push) Successful in 23s
Four tests predating the helper built their note with MagicMock(), which is truthy on every attribute nobody set. update_note now reads verify_with, so the stand-in claimed to carry a check and the milestone-317 guard refused the write. That is note 2109 exactly, and the reason fake_note exists: a stand-in has to be able to say NO. The product behaviour is right — a real column is None or a string, so this cannot happen outside a test. Observation, not changed here: fake_note sets is_task=False but leaves `status` unset, so it too is a truthy mock on the column is_task is derived FROM. Nothing depends on it today; worth making self-consistent when something does. |
||
|
|
700ef20eb0 |
feat(notes): a note's check is writable through both doors, and empty means empty (#3164, milestone 317 step 2)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Failing after 49s
CI & Build / Build & push image (push) Skipped
CI & Build / TypeScript typecheck (push) Successful in 38s
The rules path's three lessons (#3096), inherited: EMPTY MEANS NULL. The sweep's whole signal is `verify_with IS NULL` = "this is a decision, there is nothing to go and check". A "" that is not NULL makes a norm look like a constraint nobody has verified — and never-checked sorts FIRST, so it would sit at the top of the sweep forever. CLEARING IS EXPLICIT. At the MCP door "" means "leave this alone", so an agent updating a body does not wipe a check it was never asked about — which leaves no value meaning "remove it". `clear` names the field, and naming it cannot happen by accident. The REST door, where a cleared form input arrives as "", reaches the same place through normalisation: two idioms, one outcome. THE STAMP CERTIFIES A CHECK, NOT A RECORD. Rewrite or clear `verify_with` and `verified_at` is dropped, so the note re-enters the sweep. A note wrongly listed as due costs one look; a note wrongly vouched for costs exactly what the sweep exists to catch. `verified_at` is also no longer settable through an ordinary edit — a stamp says somebody performed THIS check, and minting one from a write that ran no check is the one thing that would make the sweep lie. And one this path adds: not every record may carry a check. A task's decay is its status — a done issue records what happened rather than asserting something that can go false — and a snippet already has verify_snippet, which compares its recorded location and code against the repo and expires its own verdict. Both are refused with a message naming the alternative, never dropped silently (minted_kind's reasoning, #3129), and the gate lives at the service so the two doors cannot come to disagree. Written as an INVARIANT over the resulting record, not a filter on which fields were passed. That is what catches the sideways route — a checked note being turned into a task, a write that names no check at all and would sail past any per-field gate. The MCP docstrings carry the norm-vs-constraint test, because at that door the docstring IS the contract and a field described only as "how to verify this" gets filled in on every note. Step 5 does this properly across the instruction surfaces; this is the minimum that stops the field being misused on arrival. tests/helpers gains `drive_update_note` — the patch stack for driving update_note, written twice before this and now once. The note-shaped fakes gain the trio explicitly, for fake_note's own stated reason: an unset attribute is a truthy MagicMock, and a truthy verify_with reads as a check that is there. |
||
|
|
b134fe9aa1 |
fix(tests): the column guard names join tables as _BACKED_UP holds them (#3182)
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / Python lint (push) Successful in 4s
CI & Build / integration (push) Successful in 27s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 28s
_BACKED_UP carries REAL table names — project_rulebook_subscriptions, project_rule_suppressions, project_topic_suppressions, project_rulebook_exclusions — not the shorter keys the payload uses for the same sections. The registry-coverage assertion used the payload spelling and reported four tables unguarded. The integration round trip passed on this run, which is the half that matters: the real restore_full_backup remaps arose_from_id onto the restored origin. |
||
|
|
a6ef3a6a5a |
fix(backup): a restore stops flattening the record vocabulary, and a column guard stops the next one (#3182)
CI & Build / Python lint (push) Successful in 6s
CI & Build / Plugin hooks (push) Successful in 18s
CI & Build / integration (push) Successful in 48s
CI & Build / Python tests (push) Failing after 56s
CI & Build / Build & push image (push) Skipped
CI & Build / TypeScript typecheck (push) Successful in 39s
`_note_rows` carried 16 of the `notes` table's 27 columns. A backup -> restore cycle reported success and handed back a corpus with every snippet and process flattened into a plain note, every issue and spike into `work`, every provenance edge gone, and recurring tasks no longer recurring. The record-type and kind vocabulary is what #3128 and milestone 312 were about, and a restore erased it. Two more found by auditing every row helper rather than only the one being edited: `_milestone_rows` dropped `body` — a milestone IS the plan (0066), so every plan restored as a title with no reasoning behind it — and `_repo_binding_rows` dropped `ref`, the branch a ledger follows (#2873), so a restored binding silently accounts for a different tree. `arose_from_id` is deferred to a second pass beside `parent_id`, never written in the constructor: it is an id in the SOURCE database, so copying it through lands the edge on whatever record happens to hold that number here. An edge whose target did not survive stays NULL rather than being guessed at. This is the trap that kept the fix out of milestone 317 step 1. THE STRUCTURAL HALF. The coverage guard from #2293 checks TABLES against Base.metadata; nothing checked COLUMNS, which is how nine went missing from a table that had been "covered" for years — added to the model and the migration, both of which fail loudly, and never to the serialiser, which fails silently. `_COLUMN_EXCLUSIONS` now declares, per table, every column deliberately not exported and why, and a parameterised guard walks all 23 helpers and asserts the two agree. Forgetting is no longer expressible. Reconciling all 23 turned up one more deliberate exclusion worth naming: the `code_shapes` proposal columns are the machine's standing suggestion, cleared by judgment and recomputed by every refresh, so carrying them would restore stale guesses over a tree the proposer has not seen. Tests: the round trip drives the REAL restore_full_backup against Postgres, not a reimplementation of its loop — a test that re-derives the remap it is checking would agree with whatever the product does, including nothing. Backup v12. |
||
|
|
2263fd04a4 |
fix(tests): the notes table has 27 columns — deleted_batch_id is the second deliberate exclusion (#3165)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 38s
CI & Build / Python tests (push) Successful in 1m18s
CI & Build / Build & push image (push) Successful in 24s
The pin test caught its own inaccuracy on the first run, which is what it is for. `deleted_batch_id` comes from SoftDeleteMixin alongside `deleted_at` and is excluded for the same reason: trashed rows are not exported, so neither is the batch id that groups them for restore(). The nine-field gap #3182 tracks is unchanged. |
||
|
|
2065781302 |
feat(notes): a note can carry its own check — verify_with, expires_when, verified_at (#3165, milestone 317 step 1)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / integration (push) Successful in 33s
CI & Build / Python tests (push) Failing after 45s
CI & Build / Build & push image (push) Skipped
The sibling of migration 0090, one table over. Same distinction: a NORM is a decision with no truth value; a CONSTRAINT asserts a fact about someone else's software and goes false with nobody watching. Notes hold far more constraints than rules do and hold them longer — a cross-project reference asserting what a signing service does on a duplicate upload is believed by every project that reads it, and nothing in the record says when anyone last looked. note_supersessions only fires once a human has already believed it. Three nullable columns, no backfill, no index. The index margin is thinner than 0090's — thousands of note rows against hundreds of rules — so the comment says to decide it in step 3 against a real query plan rather than guessing here. The columns land on every row in `notes`, but only non-task, non-snippet records will be OFFERED them (gated at the service in step 2): a task's decay is its status, and a snippet already carries a richer location-aware verdict in data.verification. A schema-level gate would have meant a CHECK across three columns to say what the write path says in two lines. Backup carries the trio (v11), with `verified_at` restored through _dt_or_none — _dt substitutes now(), which would restore every never-checked note as checked at the moment of the restore, inverting the one signal the sweep reads. Found while doing that, NOT fixed here, and now pinned by a test: `_note_rows` carries 16 of the `notes` table's 26 columns. note_type, task_kind, arose_from_id, the recurrence pair, the lifecycle stamps, description and data have all been missing for a long time, so a restore flattens every snippet and process into a plain note and every issue and spike into `work`. The coverage guard cannot see it — it checks TABLES, not columns, which is #2293's failure mode one level down. #3182 tracks it; arose_from_id needs the second id-remapping pass parent_id gets, which is why it is not a drive-by fix. |
||
|
|
454c617ca0 |
docs(models): the record-splitting rule gets a findable home — note 3163 (#3128 rec 9)
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / integration (push) Successful in 37s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 22s
Spike #3128's fourth question was whether a "when does a record type earn its own table" rule was worth writing down. It turned out to already exist, in full, in the RuleEmbedding docstring — the only written statement of a rule Scribe applies to every record type, sitting where nobody would look for it. Promoted to note 3163, with the three grounds (scoping different in kind, machine-written at volume, edge-or-event-not-document), the worked cases across the whole schema, and the bill: what `rules` had to re-import after leaving `notes`, including the two cells it left empty on purpose. The docstring stays put — it is where the decision was made — and now points at the note. |
||
|
|
f80401d58e |
fix(knowledge): the browse vocabulary catches up three kinds, and a snippet's mirror survives the generic door (#3128 recs 2-6)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 27s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 33s
Spike #3128 found the storage sound and the retrieval vocabulary frozen before `issue` shipped (0065). Five things, in the order they had to land. **The mirror (rec 5, the data-integrity one).** `notes.data` is DERIVED from a snippet's body, but only `update_snippet` knew that. `update_note` is a hasattr loop with no snippet awareness, and both doors reach it — so PATCH /api/notes/<snippet_id> {body} rewrote the body and left the mirror behind. `snippet_fields` PREFERS the mirror, so the row went on reporting its old repo/path/symbol to the location reverse lookup and to prior-art recall while displaying its new body: surfaced with full authority, and wrong. `snippets.recompose_data` rebuilds it from the body, carrying `verification` and `provenance` (neither is in the body to parse). An explicit `data` still wins, so every snippet-service write is untouched. **One facet table (rec 3), before adding any facet.** The type predicate was written three times — SQL, Python over semantic candidates, and a ternary computing the `is_task` pre-filter — and agreed only by luck. Adding `issue` to the SQL arm alone would have set the pre-filter to is_task=False, handed the Python arm a candidate set with no tasks in it, and returned an empty semantic half for the Issues facet forever with nothing red. `_FACETS` now generates all three. The Python arm also regains the `status IS NULL` half its SQL twin always had. **Issue and spike become facets (rec 2).** 435 issues — 17% of every task — were filterable nowhere on the human surface, while retired `plan` (90 rows) had a chip of its own. `_VALID_TYPES` was a hand-kept copy and is now derived. `plan` stays a valid facet for its legacy rows; it loses its chip. **Snippets stop being half-present in the feed (rec 4).** All 90 were in the All list, in no count, wearing an empty badge, and opening in the note editor. Counts now group by task_kind — every kind for the same two round-trips, which is why `issue` had no number — and total includes snippets, so the All chip matches the list it labels. Snippet cards route to /snippets/:id. **The prose that excused it (rec 6).** `snippet_fields` and the `data` column both still said pre-0070 rows were "never backfilled". True when 0070 landed, false since `backfill_snippet_data` shipped, and it read as licence for a stale mirror. Tests: the pre-filter can never exclude a row its own facet accepts (the regression, parameterised over every facet); both dialects select exactly their own rows; an unknown facet matches nothing; the mirror follows a body or title write, carries the verdict, and yields to an explicit `data`. `compiled_sql` moves to tests/helpers rather than becoming a third copy. Write-up: note #3161. |
||
|
|
d0a2733cb6 |
fix(design): text on a tint of itself now clears AA app-wide, and the check gates it (#3141)
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 36s
CI & Build / integration (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m12s
CI & Build / Build & push image (push) Successful in 1m2s
The badge fix (#3132) exposed the same defect everywhere: 48 rules painting a token as TEXT on an inline color-mix tint of that same token. Worst raw measurements, across every tint strength in use, both modes, over page/raised/hover: accent 1.53:1 · success 1.67:1 · text-tertiary 2.15:1 warning 2.32:1 · error 2.36:1 against AA's 4.5 THE DEFECT IS IN THE HOUSE, NOT IN SCRIBE. The semantic hues are shared family-wide, and the accent case was measured against every app's real accent, not assumed from Scribe's: Minstrel 1.81, Forge 1.87, Steward 1.65, Roundtable 3.01 — all failing. So the six -fg tokens are recorded on FabledSword (design system 1), where their parents live, rather than copied into each app. 45% toward --fs-text-primary clears AA for ALL FIVE accents (4.56-5.00), so this is one house token rather than five overrides, and it keeps deriving from --fs-accent — an app that overrides its accent still gets a legible tinted-text colour in its own colour, the same mechanism as --fs-accent-soft. The tokens are additive: a sibling app is unaffected until it regenerates its own stylesheet. One token is honestly redundant. --fs-text-secondary already passes at 4.82:1, and --fs-text-secondary-fg barely moves it. It exists so the rule has NO exceptions, because the alternative is a permanent allow-list entry for the one case that happens to pass — and a guard with an invisible exception is a guard that erodes. 46 substitutions across 18 files, each rewriting only the `color:` inside a block that tints its own background. THE CHECK NOW GATES BOTH SPELLINGS. It previously reported the inline form, because a gate nobody can satisfy on the day it lands gets switched off. Both are clean, so both fail the build now. And the check had a false-positive bug worth naming: its `color\s*:` regex matched the tail of `border-color`, `border-left-color` and `outline-color`, so it flagged seven rules that were already correct. A border is a non-text graphic with a 3:1 floor, not text at 4.5. A check that cries wolf on correct code is one that gets muted, so that mattered more than the noise. Verified by construction, not by passing: reintroduced each defect form (exit 1 each), and confirmed a legitimate border-only rule still exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
93d660b710 |
Tool disambiguators, kind badges, and a badge layer that clears AA (#3123, #3124, #3132)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 31s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m17s
CI & Build / Build & push image (push) Successful in 15s
|
||
|
|
ce1376edc9 |
refactor(ui): the badge layer gets one owner per shape (#3132 items 1-3)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 33s
CI & Build / TypeScript typecheck (push) Successful in 40s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 39s
ITEM 1 — the dead canon. StatusBadge is recorded canon (#2960) and its only consumer, TaskCard, has been unreachable since 2026-04-08, when TasksListView was deleted in favour of the Knowledge view. Four and a half months of a canon that rendered nowhere, which is worse than no canon: a session pulls #2960, builds from it, and matches a component nobody has seen. TaskCard is deleted (rule 22), and the canon is made real by adoption rather than by being left as a museum piece. ITEM 2 — MY OWN ISSUE OVERSTATED THIS, and the correction is the finding. "Three scoped re-spellings" assumed one shape spelled thrice. Reading them: KnowledgeView a task-status chip, just smaller -> a real duplicate WorkspaceTaskPanel a CLICKABLE cycler: pointer, outlined, transparent background -> a control, not a chip ProjectView PROJECT lifecycle (active/paused/ completed/archived) -> a different vocabulary Only the first was ever a duplicate. The others shared a class NAME and nothing else — which is exactly what would make a future consolidation merge three unrelated things. So: KnowledgeView adopts StatusBadge/PriorityBadge via the `compact` variant the canon already anticipated ("interactive/compact re-spellings are variants of it"); the cycler becomes `.status-cycler`; and project status becomes its own vocabulary. And there was a FOURTH, in ProjectListView — the genuine duplicate of ProjectView's project pill, differing by the amounts two hands differ by: 0.68rem vs 0.7rem, a 14% tint vs 15%, one bordered and one not. Both now use one ProjectStatusBadge. `statusLabel` went with its only caller. ITEM 3 — weight. StatusBadge and PriorityBadge used font-weight 600; the house style allows 400 and 500 only. Also "In Progress" -> "In progress", which was invisible under `text-transform: uppercase` and becomes visible the moment the compact variant turns that off. THE GUARD MISSED FOUR LIVE SITES, which is the part worth keeping. The project pills painted a hue on an inline `color-mix` tint of itself — measured 1.61-2.39:1 — and the checker only knew the `--fs-X-bg` token form. Widened, it finds 48 across the app, 26 of them --fs-accent. That backlog is not this task, so the check now splits: it GATES the token form, which is clean, and REPORTS the inline form with a count and its worst offenders. A gate nobody can satisfy today gets switched off, and then it guards nothing. Gate re-verified by reintroducing a defect — exit 1 with it, exit 0 without. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0c74dc8275 |
fix(design): badge text clears AA — the ladder was painting a hue on a tint of itself (#3132)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Successful in 36s
Every status and priority badge used its raw hue as TEXT on a 12% tint of that same hue. Measured on the dark palette, all six pairs failed the kit's own AA floor: todo 1.60:1, in-progress 1.97:1, done 2.06:1, low 2.02:1, high 2.92:1, medium 2.97:1, against 4.5. Four also failed in light mode. The cause is structural, not a bad colour pick. A 12% tint sits near the surface it composites over, so the hue as text on it has almost nowhere to go. Strengthening the tint was measured and REJECTED: on a dark palette a heavier tint moves the chip toward the light text and makes it worse. 12% was already optimal. So each pair gains a `-fg` sibling: the hue mixed toward --fs-text-primary until it clears 4.5:1 worst-case over surface-raised AND surface-hover in BOTH modes. Mixing toward that token rather than a literal is what makes one declaration cover both — it inverts by mode, so the text follows. Recorded in the DESIGN SYSTEM, not hand-written into theme.css: seven tokens on design system 2, each carrying its measurement and its reasoning, then the sheet regenerated. theme.css says not to hand-edit the --fs-* block and it is right — a hand-edit would be silently reverted by the next regeneration. The ladder keeps its shape. High priority still holds 52% saturation and medium 31% — the rungs that need to shout still shout. Low, todo and done wash toward neutral, which is what their own rationales ask for: status-todo is derived from the border colour precisely so not-yet-started recedes. Receding and illegible are different things and the old value was the second. --fs-status-cancelled-fg was found by measuring, not by reasoning. Cancelled derives from --fs-text-tertiary, which looks like the obviously-correct "quiet" choice and is a HINT colour tuned for plain surfaces — 2.63:1 on a badge tint in light mode. StatusBadge additionally dropped a `color-mix(..., #000 15%)` that darkened the hue: a light-mode instinct that made these worse on a near-black surface, and a literal besides. THE GUARD IS THE POINT. check_design_tokens.py now FAILS on any rule that paints text with a token on a tint of that same token, and names the -fg sibling as the fix. Verified by reintroducing the defect: exit 1 with it, exit 0 without. Unlike a raw literal there is nothing to weigh up, so it gates rather than reports. Two `border-top-color` uses keep the raw hue, correctly — a border is a non-text graphic and needs 3:1, which is what the hue is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a0b54ff6a3 |
feat(ui): task rows show their kind — a badge for issue and spike (#3124)
CI & Build / integration (push) Successful in 29s
CI & Build / Python tests (push) Successful in 1m3s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 10s
CI & Build / Build & push image (push) Successful in 32s
task_kind was only visible inside the task editor's Kind select, so every list surface rendered work, issue and spike identically and a list of tasks hid the fact that three different things were in it. ONE component, not a fifth spelling. The badge layer had already drifted — StatusBadge.vue is the recorded canon (#2960) but WorkspaceTaskPanel, ProjectView and KnowledgeView each carry their own scoped `.status-badge`. KindBadge is modelled on PriorityBadge, its closest sibling, which already does the thing that matters here: the DEFAULT value renders nothing. `work` is most tasks, so badging it would put a chip on nearly every row and say nothing — the same reason RuleListPane marks only `conditional`. COLOUR BY TEMPERATURE, measured rather than eyeballed. Issue and spike are opposite in character — corrective vs exploratory — so they split warm (warning) against cool (info), which survives being small and stays distinguishable without reading the word. Neither uses the accent; kind is not one of the places it is allowed. The raw semantic colour FAILS the contrast floor on the dark palette: warning on its own 12% tint measures 2.97:1 against AA's 4.5. So the text is the hue mixed toward --fs-text-primary, which passes and, because that token inverts by mode, follows light/dark for free. Measured both ways — issue 5.23:1 dark / 6.68:1 light, spike 5.33:1 / 9.26:1. `plan` renders hue-free and italic: retired since 0066, so a legacy row should read as archival rather than as a fourth kind competing for attention. In KnowledgeView it is passed as null instead, because the type badge beside it already says "Plan" and two chips reading the same word would look like two facts. Weight is 500, not the 600 the two older badges use — the house style allows 400 and 500 only, and copying 600 would spread it. SERVER FIX, without which this was decorative: dashboard's `_task_row` omitted task_kind entirely. The badge would have rendered nothing there while working everywhere else, which reads as "this list has no issues" rather than as a missing field. The guard is on the payload, where the omission was. Surfaces: ProjectView's three status columns, WorkspaceTaskPanel's two task lists, DashboardView's milestone and no-milestone rows, KnowledgeView's result rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
16805ca22c |
docs(mcp): every create_* tool says what it is NOT for (#3123)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 28s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 23s
Scribe's record kinds get reached for interchangeably, and the moment of
choice is the only moment a correction is cheap. Rule 119 puts product
guidance in the instruction surfaces, so the docstring is where this
belongs — but a docstring that only documents parameters answers "how do I
call this" and leaves "should I be calling this at all" unasked.
The gap was lopsided. create_rule and start_planning already carried real
disambiguators; create_note — far and away the highest-volume surface —
carried none at all. The guidance sat in the rarest tool and was missing
from the most common one.
Each surface now opens with ONE deciding question in its own terms rather
than a pasted block:
create_note WHAT ELSE COULD HOLD THIS? note is right when nothing
is owed and nothing enforces
create_task IS ANYTHING ACTUALLY OWED? nothing owed -> note;
an arc -> start_planning
create_snippet SHAPE, OR ADVICE? a snippet is code with a
LOCATION
create_process FOLLOWED, OR READ? applies uninvoked -> rule
create_project_rule now points at the entity check too; it had only ever
covered rule-vs-rule scope.
The guard asserts STRUCTURE, never wording: each surface must name at least
two siblings. Pinning phrasing would make every improvement a test failure,
and a test that punishes editing is a test that gets deleted. Its second
half asserts the Args: block survives — the first check is satisfiable by
turning a docstring into an essay about the other tools, which would be a
worse contract than the one being fixed.
The guard caught two gaps on its first run, one of them its own: "design
system" is hard-wrapped across a line break in create_rule, so matching the
raw docstring reported it absent. _doc() now flattens whitespace. It also
caught start_planning naming only one alternative, which was true and is
now fixed.
Deliberately NOT built: an intent-router tool. It has a bootstrapping
problem — it is itself a tool that must be reached for — and MCP clients
already list every tool's description. Recorded in #3123; build it only if
wrong-surface reaches survive this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
056c7c75da |
A task's kind is correctable — the Kind select stops lying (#3129)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / integration (push) Successful in 28s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 15s
|
||
|
|
63036ed52e |
merge: bring main's rebased history into dev after PR #132
PR #132 was rebase-merged, so main carries rewritten copies of dev's
milestone-312 commits under new SHAs. The merge base stayed at
|
||
|
|
2e39dca9cf |
fix(tasks): import minted_kind by name — a stubbed service must not stub the guard (#3129)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 21s
CI & Build / integration (push) Successful in 23s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 31s
CI 4682: test_create_task_issue_sets_kind_provenance_and_systems asserted task_kind == "issue" and got a MagicMock. That test patches the whole notes_svc module to keep the database out, so reaching validation through `notes_svc.minted_kind(...)` handed back a mock — the guard approved anything and returned nothing real. The product was wrong, not the test. minted_kind is pure validation, not a service call, so it is imported by name. A test that stubs the service to avoid I/O now keeps the guard intact, which is the behaviour you want from a guard: the only way to disable it should be to say so explicitly. That test now exercises the real validation, so it doubles as the guard against this recurring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
69d93898d9 |
fix(tasks): a task's kind is correctable — the Kind select stops lying (#3129)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Failing after 48s
CI & Build / Build & push image (push) Skipped
`kind` was accepted at CREATE on both doors and dropped at UPDATE on both: update_task had no such parameter, and the REST PATCH allow-list never read the field. So a task filed under the wrong kind could never be corrected. The frontend made it worse by looking like it worked. TaskEditorView binds a Kind select, marks the form dirty, and HAS ALWAYS SENT `kind` in the update payload — the store even types it. The route ignored it, returned 200, the view optimistically updated, the toast said "Task saved", and the old value came back on reload. Silent success, same class as #2709. Found by trying to re-file #3126 as a spike after deploying 0091. It could not be done; the task had to be recreated as #3128 and the original cancelled. One seam, not two doors. `minted_kind()` lives in services/notes.py because the REST route cannot import an MCP tool module and a second spelling of the list is how the doors would come to disagree. Both create and update route through it, so a bogus kind is now a readable error rather than a CheckViolationError surfacing as a 500. TaskKind joins TaskStatus and TaskPriority as a real enum, and update_note validates task_kind exactly as it already validated those two — the field had been reaching setattr through the hasattr guard with no validation at all, unnoticed only because no door ever offered it. The `-> plan` question #3129 raised is answered in code rather than left implicit: MINTABLE_KINDS is work/issue/spike, deliberately NARROWER than the column's CHECK. `plan` stays a valid stored value because historical plan-tasks carry it and must stay writable; it is simply not a value any door hands out, and the refusal names start_planning because a caller reaching for it wants a plan. The whitelist and the policy answer different questions and are not the same list. Every new test reads the value BACK. One that only asserted the call succeeded would have passed against the broken code — the route returned 200 while dropping the field, which is how this survived long enough to be found by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
5aabc31ee7 |
Rules that can go stale say so — verify_with, expires_when, the sweep, and task_kind='spike' (milestone 312, steps 1–5)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 28s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Successful in 17s
|
||
|
|
4be1eaecf6 |
fix(tests): a Note's is_task cannot be set — status is what makes one (#3099)
The spike CHECK tests constructed Note(is_task=True). `is_task` is a derived read-only property — `status is not None` — so SQLAlchemy raised "property 'is_task' of 'Note' object has no setter" before any row reached the database. All three failed for that, not for anything about migration 0091; the other 80 integration tests passed, including 0090's. status="todo" is what makes a note a task. Noted inline, since the field appears in to_dict output and reads like an ordinary column from there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e2e64b94c0 |
feat(tasks): task_kind gains 'spike' — the investigation, not the change (#3099, milestone 312 step 5)
A spike is a shape the other kinds cannot hold. `work` ships a change; `issue` fixes something broken. A spike is time-boxed and its output is KNOWLEDGE — it succeeds by producing an answer, and nothing ships at the end of it. Filing one as `work` makes a finished investigation look like an abandoned change, which is why the distinction earns a value rather than a convention. It is also the record a failed check asks for. This milestone gave rules a verify_with; when one fails the rule is wrong, and the next move is often to go and find out what replaced it. notes.arose_from_id already exists (0065), so constraint -> spike provenance needed no schema at all — only a docstring saying it is there. Rule 36: the value and the widened CHECK land in the same migration, DROP then ADD, exactly as 0065 did for 'issue'. The two whitelists live in one tuple each so upgrade and downgrade cannot disagree about what the list was on either side. The downgrade demotes existing spikes to 'work' first — lossy, deliberately, because the alternative is a downgrade that fails on real data, and one that says what it did beats one that cannot run. 'plan' stays whitelisted though retired: historical plan-tasks carry it, and a row that cannot be rewritten cannot be edited, restored or migrated. The integration test asserts both halves. A test that only proved 'spike' is accepted would pass just as happily against a table whose CHECK had been dropped and never re-added — which is the other way rule 36's failure happens — so an unknown kind is asserted to still raise. Not in scope, deliberately: any special lifecycle, time-box enforcement, or gating relationship. It is a kind, not a workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3345be84d1 |
feat(rules): the check is editable, visible, and sweepable in the UI (#3098, milestone 312 step 4)
Rule 27 — the milestone was backend-only until this. Four surfaces:
RULE EDITOR — verify_with and expires_when under a legend that asks the
actual question ("Can this rule go stale?") and says empty is the normal
answer, because most rules are decisions and a form that implies a missing
field would get them filled in out of tidiness. When the SAVED rule carries
a check, the stamp shows with Still true / No longer true beside it. The
stamp reads the stored value, not the draft: an unsaved edit to the textarea
has not been run against anything.
SWEEP PANE — its own surface, not a filter on the rule list. That list can
only ever show one topic of one rulebook, and a rule that has gone false
belongs to no one rulebook; filtering it would under-report, which is the
failure this whole surface exists to catch. Reached from the rulebook list,
below the rulebooks, because that is where you go to look at rules.
RULE ROWS — a chip only on rules carrying a check, so its presence is the
signal. PROJECT RULES TAB — the check shows beside `why` when a rule has
one, read-only: that tab is the project's view of what binds it.
NO AGE-GRADED COLOUR anywhere, deliberately. The sweep is already ordered by
urgency, so a red/amber ramp would restate the ordering AND require an
invented "stale after N days" threshold — a magic number nobody could defend
and the first thing to go out of date. --fs-overdue is error red and reserved
for a broken promise like a missed due date; a verification age is not one,
and colouring it that way makes a rule someone just wrote look broken. Only
"never" is marked, because it is categorically different from a date rather
than a worse one — and it is marked by weight, not hue.
An empty sweep says "Nothing to check", not nothing: good news must not read
as a broken page.
Two chips (tier, then verification) turned out byte-identical, so .rule-chip
moves to rules-shared.css and snippet #2906 is updated to match rather than
left describing a file that has moved on. Its header comment counted the
panes it served; that count went stale the moment a fourth arrived, so it no
longer counts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
35c632f834 |
docs(rules): a project rule is shaped differently, not just scoped differently (milestone 312)
The three surfaces already agree on WHERE a rule goes — the using-scribe skill's "Where a new rule goes" section and both tool docstrings frame it as one question, who should this bind. What they did not say is that the two homes want differently SHAPED rules, and one deferral was actively misleading. `create_project_rule` said `tier: "always_on" or "conditional" — see create_rule`. That imports a bar calibrated for a different blast radius. On a rulebook rule always_on means every session in every project, so the test is severe: the trigger must be nameless. A project rule is already scoped by construction, so always_on costs only that project's sessions — and being specific, which the family test treats as the signal for conditional, is what project rules are FOR. The instance's own data says so: rules 78, 115 and 119 are all project rules and all always_on. Not zero bar, a different one: conditional is right when the rule is about one AREA of a large project, because forty always-on rules on one project reproduces locally the preload bloat milestone 307 fixed globally. Also: - create_rule now says to write the general form WITHOUT hedging for exceptions — a project needing to narrow it writes its own and links with overrides/elaborates. A rulebook rule padded with "unless…" for two projects is two project rules that were never written. Only the project side mentioned that relationship; the side that benefits from it did not. - arose_from_id: reach for it harder on a project rule, which usually comes from one traceable incident in the repo, where a family rule is more often a standing preference with no single origin. - system_ids is worth setting on a project rule too — it is what lets a conditional one arrive with its area. - when_to_apply no longer claims to "decide" the tier here, which stopped being true one entry down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b97f57ee7f |
feat(rules): the staleness sweep — which standing rules assert a fact nobody has confirmed (#3097, milestone 312 step 3)
The query the last two steps were storage for. `rules_due_for_verification` returns every rule carrying a `verify_with`, ordered by `verified_at` ASC NULLS FIRST, each row carrying the check IN FULL — the opposite call from rule_brief, because the reader is about to go and run it. NULLS FIRST is the ordering this turns on. Postgres sorts NULLs last on an ASC ordering, which would put the rules nobody has ever confirmed BEHIND every rule someone once looked at. Exactly backwards: a claim with no evidence at all outranks an old one. Rules with no check never appear, and that is the property that keeps the list worth reading. Most rules are decisions — no truth value, nothing to go and check. If they appeared here the sweep would be the rulebook. `mark_rule_verified(rule_id, still_true)` closes the loop, asymmetrically: passing writes a stamp, FAILING WRITES NOTHING. There is no "verified false" state because a rule whose check failed is not in a special condition, it is wrong — and recording the failure as a flag would let it sit there being false with the sweep satisfied that someone had looked. So it stays at the top until someone corrects or retires it, and the response says so. An unrecognised `tier` filter raises rather than falling back. _valid_tier's silent always_on default is right for a WRITE — a typo should leave a rule binding — and wrong for a FILTER, where the same fallback quietly answers a different question and returns a short list that reads as good news. Deliberately NOT filterable by project: a project reaches rules through project scope, subscriptions, always-on rulebooks and exclusions, and a filter missing one of those paths would UNDER-report — the exact failure this surface exists to prevent. Said so in the docstring rather than shipping a half-correct filter. Ownership-scoped like every other rule read (owned rulebook, or owned project), in ONE statement with an OR across the XOR rather than two queries merged in Python, so the ordering is the database's and cannot disagree with itself. Note that rules have no sharing ACL in this schema — no rule_shares, no rulebook_shares — so there is no wider set for access.py to consult here. Also fixes a test title that had been lying for ten tools: "all sixteen tools" asserted 26. The number now lives only in the assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
874f7cacdb |
test(rules): the kwargs assertion learns about clear (#3096, milestone 312 step 2)
test_update_rule_only_sends_non_default_fields pins that the MCP door forwards only what the caller actually gave. `clear` is now always forwarded — an empty tuple is "clear nothing", a value rather than an absent argument — so the expected kwargs gained it. The property under test is unchanged: everything left at its default still stays out. Two tests added beside it while the shape is in view: naming a field for clearing reaches the service as `clear`, and the check fields are forwarded when given. CI 4630 otherwise green — the integration lane ran all six of the new real-Postgres cases (72 selected, was 66) and applied 0089 -> 0090. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
91b34619f9 |
feat(rules): the write path carries a rule's check, and empty finally means empty (#3096, milestone 312 step 2)
verify_with / expires_when now reach a rule through both doors and come back on every read. The open question this step existed to settle was how to UNSET a nullable field, and the answer is one convention per door: - MCP: "" still means "leave unchanged" — an agent filling three fields must not wipe the other five — so clearing is explicit, clear_fields=["..."]. Naming the field is the one form that cannot happen by accident. - REST: a cleared form input arrives as "", and the service normalises "" to NULL for every nullable rule column, so an emptied input does what it looks like it does. Two idioms, one outcome, and the normalisation is what makes the step-3 sweep correct: `verify_with IS NOT NULL` would otherwise be true for every rule ever touched through the UI, and the sweep would list the whole rulebook and mean nothing. to_dict renders "" and NULL identically, so this is only visible against a real column — hence the integration module rather than a mock. Editing verify_with drops verified_at. A stamp certifies A CHECK, not a rule; reword the check and the old stamp vouches for something that no longer exists. Safe direction, same asymmetry as _valid_tier: a rule wrongly listed as due costs one look, a rule wrongly vouched for costs the thing the sweep exists to catch. Editing anything else leaves the stamp alone, or a rulebook tidy-up would reset every constraint and the ordering would carry nothing. Reads: rule_brief attaches `last_verified` ONLY to a rule that carries a check — its presence is the signal, and it says both "this asserts a fact that can go false" and "here is how long ago anyone confirmed it". "never" rather than null, per #2483. The check text itself stays in get_rule; a listing needs to know which rules can rot, not how to test them. Search hits carry the full trio, since a hit is exactly the moment someone is about to act on a rule. Also folds in the #3078 finding, which had been sitting as a note: create_rule now teaches that when_to_apply is the retrieval surface and must carry the SYMPTOM — the words you would type while stuck — not just the situation. fake_rule gains the three fields as None for the reason the helper already documents one line up: unnamed, verify_with is a truthy MagicMock and every stand-in rule would claim a check it does not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3d4f5be711 |
feat(rules): a rule can carry its own check — verify_with, expires_when, verified_at (#3095, milestone 312 step 1)
A rulebook holds two kinds of row in one table. A NORM is a decision: no truth value, changes only when its author changes it, and they know they did. A CONSTRAINT asserts a fact about someone else's software, and goes false with nobody present. Milestone 307's audit found nine stale sites; every one was a constraint, and not one norm had rotted. Three nullable columns so a rule can say how to check itself. expires_when is a STATE, not a date — constraints expire when the ground moves, not on a schedule. verified_at NULL means never checked and sorts FIRST in the sweep to come: unexamined outranks examined-long-ago. Most rules set none of the three; a null verify_with is the marker for "this is a decision, there is nothing to go and check," and it only reads that way while it stays honest. Nothing is backfilled and nothing is indexed. A migration cannot invent a check any more than 0088 could invent a trigger, and the sweep reads a whole rulebook — hundreds of rows, on operator demand, never on a request path. Also, in the backup service the fields had to pass through: - Restore now remaps arose_from_id through note_id_map. It has been exported since 0088 and silently dropped on the way back in ever since, so every restore lost every rule's provenance link. - _dt_or_none, because _dt substitutes now() for an absent value. That is right for created_at/updated_at and wrong here: a rule nobody ever checked would restore looking freshly checked and fall to the bottom of the sweep it should top. Column additions do not move BACKUP_VERSION; only new sections do, as when 0088 added when_to_apply/tier/arose_from_id to the same helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
15659e2c57 |
fix(tests): a Note's is_task cannot be set — status is what makes one (#3099)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 17s
The spike CHECK tests constructed Note(is_task=True). `is_task` is a derived read-only property — `status is not None` — so SQLAlchemy raised "property 'is_task' of 'Note' object has no setter" before any row reached the database. All three failed for that, not for anything about migration 0091; the other 80 integration tests passed, including 0090's. status="todo" is what makes a note a task. Noted inline, since the field appears in to_dict output and reads like an ordinary column from there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
88e9c0b0bd |
feat(tasks): task_kind gains 'spike' — the investigation, not the change (#3099, milestone 312 step 5)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Failing after 32s
CI & Build / TypeScript typecheck (push) Successful in 37s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 31s
A spike is a shape the other kinds cannot hold. `work` ships a change; `issue` fixes something broken. A spike is time-boxed and its output is KNOWLEDGE — it succeeds by producing an answer, and nothing ships at the end of it. Filing one as `work` makes a finished investigation look like an abandoned change, which is why the distinction earns a value rather than a convention. It is also the record a failed check asks for. This milestone gave rules a verify_with; when one fails the rule is wrong, and the next move is often to go and find out what replaced it. notes.arose_from_id already exists (0065), so constraint -> spike provenance needed no schema at all — only a docstring saying it is there. Rule 36: the value and the widened CHECK land in the same migration, DROP then ADD, exactly as 0065 did for 'issue'. The two whitelists live in one tuple each so upgrade and downgrade cannot disagree about what the list was on either side. The downgrade demotes existing spikes to 'work' first — lossy, deliberately, because the alternative is a downgrade that fails on real data, and one that says what it did beats one that cannot run. 'plan' stays whitelisted though retired: historical plan-tasks carry it, and a row that cannot be rewritten cannot be edited, restored or migrated. The integration test asserts both halves. A test that only proved 'spike' is accepted would pass just as happily against a table whose CHECK had been dropped and never re-added — which is the other way rule 36's failure happens — so an unknown kind is asserted to still raise. Not in scope, deliberately: any special lifecycle, time-box enforcement, or gating relationship. It is a kind, not a workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c83bedf3be |
feat(rules): the check is editable, visible, and sweepable in the UI (#3098, milestone 312 step 4)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m13s
CI & Build / Build & push image (push) Successful in 37s
Rule 27 — the milestone was backend-only until this. Four surfaces:
RULE EDITOR — verify_with and expires_when under a legend that asks the
actual question ("Can this rule go stale?") and says empty is the normal
answer, because most rules are decisions and a form that implies a missing
field would get them filled in out of tidiness. When the SAVED rule carries
a check, the stamp shows with Still true / No longer true beside it. The
stamp reads the stored value, not the draft: an unsaved edit to the textarea
has not been run against anything.
SWEEP PANE — its own surface, not a filter on the rule list. That list can
only ever show one topic of one rulebook, and a rule that has gone false
belongs to no one rulebook; filtering it would under-report, which is the
failure this whole surface exists to catch. Reached from the rulebook list,
below the rulebooks, because that is where you go to look at rules.
RULE ROWS — a chip only on rules carrying a check, so its presence is the
signal. PROJECT RULES TAB — the check shows beside `why` when a rule has
one, read-only: that tab is the project's view of what binds it.
NO AGE-GRADED COLOUR anywhere, deliberately. The sweep is already ordered by
urgency, so a red/amber ramp would restate the ordering AND require an
invented "stale after N days" threshold — a magic number nobody could defend
and the first thing to go out of date. --fs-overdue is error red and reserved
for a broken promise like a missed due date; a verification age is not one,
and colouring it that way makes a rule someone just wrote look broken. Only
"never" is marked, because it is categorically different from a date rather
than a worse one — and it is marked by weight, not hue.
An empty sweep says "Nothing to check", not nothing: good news must not read
as a broken page.
Two chips (tier, then verification) turned out byte-identical, so .rule-chip
moves to rules-shared.css and snippet #2906 is updated to match rather than
left describing a file that has moved on. Its header comment counted the
panes it served; that count went stale the moment a fourth arrived, so it no
longer counts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
9d7485df2d |
docs(rules): a project rule is shaped differently, not just scoped differently (milestone 312)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 28s
The three surfaces already agree on WHERE a rule goes — the using-scribe skill's "Where a new rule goes" section and both tool docstrings frame it as one question, who should this bind. What they did not say is that the two homes want differently SHAPED rules, and one deferral was actively misleading. `create_project_rule` said `tier: "always_on" or "conditional" — see create_rule`. That imports a bar calibrated for a different blast radius. On a rulebook rule always_on means every session in every project, so the test is severe: the trigger must be nameless. A project rule is already scoped by construction, so always_on costs only that project's sessions — and being specific, which the family test treats as the signal for conditional, is what project rules are FOR. The instance's own data says so: rules 78, 115 and 119 are all project rules and all always_on. Not zero bar, a different one: conditional is right when the rule is about one AREA of a large project, because forty always-on rules on one project reproduces locally the preload bloat milestone 307 fixed globally. Also: - create_rule now says to write the general form WITHOUT hedging for exceptions — a project needing to narrow it writes its own and links with overrides/elaborates. A rulebook rule padded with "unless…" for two projects is two project rules that were never written. Only the project side mentioned that relationship; the side that benefits from it did not. - arose_from_id: reach for it harder on a project rule, which usually comes from one traceable incident in the repo, where a family rule is more often a standing preference with no single origin. - system_ids is worth setting on a project rule too — it is what lets a conditional one arrive with its area. - when_to_apply no longer claims to "decide" the tier here, which stopped being true one entry down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
410d616c22 |
feat(rules): the staleness sweep — which standing rules assert a fact nobody has confirmed (#3097, milestone 312 step 3)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 35s
The query the last two steps were storage for. `rules_due_for_verification` returns every rule carrying a `verify_with`, ordered by `verified_at` ASC NULLS FIRST, each row carrying the check IN FULL — the opposite call from rule_brief, because the reader is about to go and run it. NULLS FIRST is the ordering this turns on. Postgres sorts NULLs last on an ASC ordering, which would put the rules nobody has ever confirmed BEHIND every rule someone once looked at. Exactly backwards: a claim with no evidence at all outranks an old one. Rules with no check never appear, and that is the property that keeps the list worth reading. Most rules are decisions — no truth value, nothing to go and check. If they appeared here the sweep would be the rulebook. `mark_rule_verified(rule_id, still_true)` closes the loop, asymmetrically: passing writes a stamp, FAILING WRITES NOTHING. There is no "verified false" state because a rule whose check failed is not in a special condition, it is wrong — and recording the failure as a flag would let it sit there being false with the sweep satisfied that someone had looked. So it stays at the top until someone corrects or retires it, and the response says so. An unrecognised `tier` filter raises rather than falling back. _valid_tier's silent always_on default is right for a WRITE — a typo should leave a rule binding — and wrong for a FILTER, where the same fallback quietly answers a different question and returns a short list that reads as good news. Deliberately NOT filterable by project: a project reaches rules through project scope, subscriptions, always-on rulebooks and exclusions, and a filter missing one of those paths would UNDER-report — the exact failure this surface exists to prevent. Said so in the docstring rather than shipping a half-correct filter. Ownership-scoped like every other rule read (owned rulebook, or owned project), in ONE statement with an OR across the XOR rather than two queries merged in Python, so the ordering is the database's and cannot disagree with itself. Note that rules have no sharing ACL in this schema — no rule_shares, no rulebook_shares — so there is no wider set for access.py to consult here. Also fixes a test title that had been lying for ten tools: "all sixteen tools" asserted 26. The number now lives only in the assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
469b43f222 |
test(rules): the kwargs assertion learns about clear (#3096, milestone 312 step 2)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Successful in 37s
test_update_rule_only_sends_non_default_fields pins that the MCP door forwards only what the caller actually gave. `clear` is now always forwarded — an empty tuple is "clear nothing", a value rather than an absent argument — so the expected kwargs gained it. The property under test is unchanged: everything left at its default still stays out. Two tests added beside it while the shape is in view: naming a field for clearing reaches the service as `clear`, and the check fields are forwarded when given. CI 4630 otherwise green — the integration lane ran all six of the new real-Postgres cases (72 selected, was 66) and applied 0089 -> 0090. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c61925be76 |
feat(rules): the write path carries a rule's check, and empty finally means empty (#3096, milestone 312 step 2)
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Failing after 45s
CI & Build / Build & push image (push) Skipped
CI & Build / integration (push) Successful in 29s
verify_with / expires_when now reach a rule through both doors and come back on every read. The open question this step existed to settle was how to UNSET a nullable field, and the answer is one convention per door: - MCP: "" still means "leave unchanged" — an agent filling three fields must not wipe the other five — so clearing is explicit, clear_fields=["..."]. Naming the field is the one form that cannot happen by accident. - REST: a cleared form input arrives as "", and the service normalises "" to NULL for every nullable rule column, so an emptied input does what it looks like it does. Two idioms, one outcome, and the normalisation is what makes the step-3 sweep correct: `verify_with IS NOT NULL` would otherwise be true for every rule ever touched through the UI, and the sweep would list the whole rulebook and mean nothing. to_dict renders "" and NULL identically, so this is only visible against a real column — hence the integration module rather than a mock. Editing verify_with drops verified_at. A stamp certifies A CHECK, not a rule; reword the check and the old stamp vouches for something that no longer exists. Safe direction, same asymmetry as _valid_tier: a rule wrongly listed as due costs one look, a rule wrongly vouched for costs the thing the sweep exists to catch. Editing anything else leaves the stamp alone, or a rulebook tidy-up would reset every constraint and the ordering would carry nothing. Reads: rule_brief attaches `last_verified` ONLY to a rule that carries a check — its presence is the signal, and it says both "this asserts a fact that can go false" and "here is how long ago anyone confirmed it". "never" rather than null, per #2483. The check text itself stays in get_rule; a listing needs to know which rules can rot, not how to test them. Search hits carry the full trio, since a hit is exactly the moment someone is about to act on a rule. Also folds in the #3078 finding, which had been sitting as a note: create_rule now teaches that when_to_apply is the retrieval surface and must carry the SYMPTOM — the words you would type while stuck — not just the situation. fake_rule gains the three fields as None for the reason the helper already documents one line up: unnamed, verify_with is a truthy MagicMock and every stand-in rule would claim a check it does not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e08e999406 |
feat(rules): a rule can carry its own check — verify_with, expires_when, verified_at (#3095, milestone 312 step 1)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / TypeScript typecheck (push) Successful in 37s
CI & Build / integration (push) Successful in 28s
CI & Build / Build & push image (push) Successful in 25s
A rulebook holds two kinds of row in one table. A NORM is a decision: no truth value, changes only when its author changes it, and they know they did. A CONSTRAINT asserts a fact about someone else's software, and goes false with nobody present. Milestone 307's audit found nine stale sites; every one was a constraint, and not one norm had rotted. Three nullable columns so a rule can say how to check itself. expires_when is a STATE, not a date — constraints expire when the ground moves, not on a schedule. verified_at NULL means never checked and sorts FIRST in the sweep to come: unexamined outranks examined-long-ago. Most rules set none of the three; a null verify_with is the marker for "this is a decision, there is nothing to go and check," and it only reads that way while it stays honest. Nothing is backfilled and nothing is indexed. A migration cannot invent a check any more than 0088 could invent a trigger, and the sweep reads a whole rulebook — hundreds of rows, on operator demand, never on a request path. Also, in the backup service the fields had to pass through: - Restore now remaps arose_from_id through note_id_map. It has been exported since 0088 and silently dropped on the way back in ever since, so every restore lost every rule's provenance link. - _dt_or_none, because _dt substitutes now() for an absent value. That is right for created_at/updated_at and wrong here: a rule nobody ever checked would restore looking freshly checked and fall to the bottom of the sweep it should top. Column additions do not move BACKUP_VERSION; only new sections do, as when 0088 added when_to_apply/tier/arose_from_id to the same helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0e5aed58a9 |
Rules become findable: canon catalog, triggers, tiers, edges, retrieval, surfacing (milestone 307, steps 1–5) (#131)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 29s
CI & Build / TypeScript typecheck (push) Successful in 36s
CI & Build / Python tests (push) Successful in 1m5s
CI & Build / Build & push image (push) Successful in 15s
Steps 1-5 of milestone 307. Design in note 3026; the step-6 true-up proposal is note 3061 and needs this deployed first. Behaviour-neutral by construction: tier defaults to always_on, so every rule this instance already has keeps binding exactly as it did. That guarantee is the first case in tests/test_integration_rule_surfacing.py, against real Postgres. Migrations 0087-0089 are additive. First boot backfills rule embeddings in the background. Plugin manifest at 0.1.47. |
||
|
|
02c1e37620 |
feat(rules): the write path can notice a standing rule it was never given (#3031, milestone 307 step 5, hook arm)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Successful in 24s
A conditional rule is not resident, so a session can be about to violate one it was never handed. This arm notices: when what is being written resembles a rule's trigger, the hint names it and says to read it before deciding it does not apply. A SUGGESTION, and the plan was wrong about why it could be more. It claimed the hook "already resolves a path to an area" — it does not, and nothing in Scribe maps a path to a System or a canonical area (build_write_path_hint resolves paths against snippet LOCATIONS, a different index; the learned-alias idea belongs to another project). Correction logged on the task. Rather than invent path→area inference to make a stale claim true, the arm does what D7 already decided and what this surface already IS: tags bind at enter_project, meaning suggests here. The header of the hook says NEVER BLOCKS; dressing a hint up as binding would have been the actual mistake. CONDITIONAL RULES ONLY. An always-on rule is already in the session, so re-offering it is noise — and noise on a hint that fires on every write is how a hint gets ignored. Telemetry goes to retrieval_logs, NOT note_usage_events, and that is a correctness call rather than a preference: note_usage ids are REMAPPED on a backup restore, so a rule id written there would come back attached to whatever note took that number — silently corrupting the evidence the next true-up is supposed to read. retrieval_logs is never restored and `source` already separates surfaces. record_retrieval's `results` type widened to match what it actually needs (an `.id`), instead of passing a Rule to something annotated Note. The rule dedup gets its OWN state file and query parameter, like the three channels before it — #2708's lesson was that one shared channel lets a hint of one class silence a different class that had never been shown. Plugin version bumped: a hook change clients cannot see did not ship (#1040). The stub is autouse in conftest rather than added to forty-odd call sites: the arm loads an embedding model, and every existing test that stubs the NOTES search would otherwise pull a real model in through the one arm it had no way to know about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4585cda3ff |
fix(rules): import or_ in rulebooks — the tier gate used it unimported (#3031)
CI & Build / Python lint (push) Successful in 7s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m9s
CI & Build / Build & push image (push) Successful in 24s
NameError at runtime: the tier/area clause and co_surfaced_partners both build an or_(), which rulebooks.py never imported. py_compile passes on this (it is a name error, not a syntax error) and so did the whole unit suite. Where it surfaced is the useful part. The unit tests mock the session, so the project-areas query returns empty, `reachable` stays None, and the or_ branch is never taken — they exercised the path that avoids the bug. Only the integration lane, with a real project and real rows, went down the branch that needed the name. Six integration tests caught it, including the inception one that merely calls get_applicable_rules in passing. A reminder about which lane proves what: mocking the thing that selects the branch means the branch is untested by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cd9aa87aa4 |
feat(rules): tier 1 preloads, tier 2 arrives by area — and a split rule can no longer be read half-way (#3031, milestone 307 step 5)
CI & Build / Python lint (push) Failing after 6s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Failing after 26s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Skipped
The payoff step: a rule stops having to be resident to be honoured. - list_always_on_rules returns the ALWAYS-ON tier only. It is the session-start call, made before any project is in scope, so there is no area vocabulary to match a conditional rule against yet. - get_applicable_rules carries a conditional rule when the project works in an area the rule is tagged to — resolved through systems.canonical_id, so the project's own NAME for the area is irrelevant, which is the entire reason the catalog exists. The gate is applied IN SQL, so `limit` counts rules that will actually surface rather than rules about to be dropped. - Bindingness is a deterministic TAG match, never a similarity score (D7). The vector channel stays a suggestion, in search. co_surfaced_partners is the fix that rule 144 never had. It was split off rule 46 and folded back the same day because "either rule could surface without the other and miss exposing a project to what the entire shape is intended to be" — correct, and merging was the only remedy available. Now a partner ARRIVES with its other half even when nothing else selected it, tagged `via: co_surfaces` so the payload says why. Two limits, both deliberate: only rules the caller owns, because an edge is not a back door into someone else's rulebook; and a project's suppressions are passed as exclusions, because an explicit mute is a decision and an edge does not outrank it. COMPATIBILITY, asserted first in the integration test rather than reasoned about: a rule with no tier, no areas and no edges binds exactly as it did before any of this existed. `tier` defaults to always_on, so an install upgrades and every rule it already had keeps arriving. Getting that backwards would silently stop enforcing rules people rely on, which is worse than any amount of payload bloat. Four unit tests were coupled to the ORDER of a mocked session's execute() calls, so a new query broke them. Rather than pad the sequence and deepen that coupling, the three post-query lookups are stubbed by name — they have their own coverage, and the real wiring is proven against Postgres. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6ada97bb0b |
fix(rules): resolve the Rule forward ref for ruff and readers (#3030)
CI & Build / Python lint (push) Successful in 6s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Successful in 33s
CI & Build / TypeScript typecheck (push) Successful in 44s
CI & Build / Python tests (push) Successful in 1m11s
CI & Build / Build & push image (push) Successful in 23s
F821: the quoted `"Rule"` in semantic_search_rules' return annotation evaluates fine at runtime — a string inside a subscript is a value, not a name lookup — but it points at nothing a reader or a type checker can follow, which is what ruff is objecting to and it is right to. A TYPE_CHECKING import resolves it at zero runtime cost, while the real import stays inside the function so this module still does not pull in the rulebook models. Placed after the import block, where isort wants the guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
95a37318fc |
feat(rules): rules become findable by meaning (#3030, milestone 307 step 4)
CI & Build / Python lint (push) Failing after 9s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 44s
CI & Build / integration (push) Successful in 45s
CI & Build / Python tests (push) Successful in 1m26s
CI & Build / Build & push image (push) Skipped
Rules were the only major record type with no vector, so `search` could never return one and a rule could arrive only by being preloaded. That single fact is what made every rule compete for one always-on budget. THE DECISION THE TASK ASKED FOR, made explicitly: a sibling rule_embeddings table, not a polymorphic embedding row. The ROW could have been generalised; the SEARCH could not. semantic_search_notes is Note-specific scoping end to end — the visibility clause, the supersession penalty, note_type/task_kind/system filters — and a rule shares none of it, scoping instead by rulebook ownership or project. Generalising the row while still needing two searches is the worst of both: a key with referential integrity to neither table, on the path every session start runs, to share four columns. What is genuinely common is BEHAVIOUR — get_embedding, chunk_document, embedding_text, CHUNKER_VERSION — and those are reused as-is. Sharing them is the DRY win; sharing the table would have been the DRY costume. The document shape is measured, not chosen (note 2485). That pass found the snippet was the only discriminative record in the corpus — a 0.153 top-to-second gap against 0.010-0.023 — and that the cause was its SHAPE: purpose stated twice in a short single-topic document. rule_document reproduces it: the trigger in the title AND as the body's first line. And it excludes `why`, which matters more than any of it. `why` is dated incident narrative — rule 46's runs to 4,300 characters — and long multi-topic prose is exactly what made sixteen dev-logs mutually indistinguishable. Adding it would not give the vector more to work with; it would give every rule the SAME thing to work with. rule_document takes no `why` parameter at all, so a well-meaning caller cannot pass one. A rule with no trigger degrades to title + statement — findable, less sharp. That is an argument for backfilling triggers (step 6), not for padding the document with whatever text is nearby. search(content_type="rule") returns the rule WITH its why and how_to_apply: they are its operational half, the session payload never carries them, and a caller who went looking should not have to re-fetch. Writes re-index fire-and-forget like notes; startup backfills in its own try block so neither backfill can skip the other. rule_embeddings is derived, so it joins note_embeddings in the backup's explicitly-NOT-included list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
682bea5257 |
fix(rules): the third row literal — fetchRules builds a list row too (#3029)
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / integration (push) Successful in 20s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Python tests (push) Successful in 1m3s
CI & Build / Build & push image (push) Successful in 40s
vue-tsc caught what I missed: there were THREE places hand-building a rule list row, not two. fetchRules mapped full rules down to the same four fields in a spot far from the other two, so consolidating the pair I could see left this one behind — which is precisely how the server side ended up with three divergent trim dicts in the first place. All three now go through toHeader. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8b60d552d2 |
feat(rules): the rule editor asks when it applies, and the list shows its age (#3029, milestone 307 step 3, UI)
CI & Build / Python lint (push) Successful in 6s
CI & Build / integration (push) Successful in 38s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Failing after 27s
CI & Build / Python tests (push) Successful in 1m27s
CI & Build / Build & push image (push) Skipped
Rule 27 — the schema and both doors shipped with no human surface, so step 3 was not shippable until this. RuleEditorSlideOver gains the trigger, the tier, the areas, and a read-only view of the rule's edges. The tier is a radio pair carrying the test itself rather than a bare toggle: can you name the trigger WITHOUT naming a system, an artifact type or a moment? If the honest answer is "whenever you are working", it is always on. It also says why conditional is not a demotion — it costs nothing when irrelevant, which is what lets a rule be as long as it needs to be. The relations block states the rule the whole milestone turns on: rules that FAIL TOGETHER are linked, never merged. RuleListPane shows the trigger and the LAST-CHANGED DATE on every row, and marks conditional only — always_on is the default and badging every row would say nothing. The date is the cheap triage the FabledCurator case wanted: a rule whose age predates the capability it duplicates is visible at a glance instead of needing a get_rule to find out. ProjectRulesTab's inline create form gains the same two fields, because a project rule bloats exactly the way a family one does — FabledCurator has 23 of them. Two type fixes the new shapes forced, both worth keeping: - toHeader() in the store: a list row is the server's rule_brief, so patching a list locally has to mirror every field it carries or the two disagree. There were two hand-built four-field literals doing that job. - ApplicableRules.rules / .project_rules are now described AS RuleHeader rather than as two more hand-written shapes — the same builder produces them, so the same type should describe them. groupByRulebookAndTopic skips a null-topic rule rather than widening TopicGroup to accept one: a rule carries topic_id XOR project_id, so a null topic in that list means something is wrong upstream, and a widened type would hide it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ffb7a0fe38 |
feat(rules): both doors carry the trigger, the tier, the areas and the edges (#3029, milestone 307 step 3, surfaces)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 21s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m7s
CI & Build / Build & push image (push) Successful in 24s
MCP and REST both gain when_to_apply / tier / system_ids / arose_from_id on create and update, plus relate_rules / unrelate_rules for the typed edges, and get_rule now returns a rule's areas and relations alongside it. rule_detail() is a SERVICE function, not one per door. It started as a copy in each — identical, and the prior-art hook flagged it immediately, which is the same lesson rules_payload (#2858) already recorded: a second copy drifts. Both doors call the one seam, so create, update and get cannot disagree about what a rule looks like coming back. The authoring guidance lands in create_rule's docstring rather than in a rule, per rule 119 as the operator described it: this is behaviour every instance should inherit, not one operator's preference. It states the test — ONE RULE = ONE THING YOU COULD VIOLATE. Rules that FAIL TOGETHER get linked with relate_rules(kind="co_surfaces"), never merged into one row. — and names why merging loses: a merged rule cannot be cited, surfaced or suppressed a clause at a time, and it grows without limit because adding to it is always cheaper than adding a rule. create_project_rule says the same about "overrides", which is what FabledCurator's 85/86 should have been instead of near-copies that drift from their parent. The tier arg carries the test itself: can you name the trigger WITHOUT naming a system, an artifact type or a moment? If the honest answer is "whenever you are working", it is always_on. Tests: the applicable-rules cases fabricated raw tuples matching the old column lists, so they move to the entity shape via fake_rule; new cases pin rule_brief (a DATE not a stamp, the depth left to get_rule, no null keys) and that an unknown tier falls back to BINDING. fake_rule gains when_to_apply / tier / arose_from_id for the note-2109 reason the helper exists: unnamed, they would be truthy MagicMocks. The tool tests stub the new rule_detail seam — they are about argument forwarding and have no database. The module header's "Sixteen tools" had been wrong for two milestones; the registration count test is what actually catches that, so the header now says so instead of carrying a number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6ddb8bf859 |
feat(rules): a rule can say when it applies, which area it is about, and what it belongs with (#3029, milestone 307 step 3, schema)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 25s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Failing after 47s
CI & Build / Build & push image (push) Skipped
A rule could not state its trigger, its area, or its siblings, so all three were being written as prose instead: a System's charter restating rule text, a `why` naming the note that caused it, and two halves of one shape merged into a single row because either could surface without the other. Migration 0088 adds the four fields those workarounds stood in for: - `when_to_apply` — the trigger. Nullable in the DB and required at the service layer: existing rules have none and a migration cannot invent one. - `tier` — always_on | conditional, defaulting to always_on. This migration therefore changes NOTHING about which rules bind; an install upgrades and every rule keeps arriving exactly as before. Getting that backwards is the one failure this milestone exists to prevent, so _valid_tier falls back to always_on rather than silently un-binding a rule with a typo'd tier. - `arose_from_id` — the record that caused the rule, the edge notes and tasks already have. SET NULL: trashing the source does not repeal the rule. - `rule_systems` / `rule_relations` — the canon tag and the typed edges (co_surfaces / overrides / elaborates), each earned from a workaround its absence forced. rule_brief() replaces the THREE hand-written trim dicts that had already diverged — two carried topic_id, one didn't, and none carried the timestamps the model has held all along. That omission is why a rule written before the capability it duplicates was indistinguishable at read time from one still doing work. It now carries updated_at as a DATE: the question is "how old is this", and a full stamp across the always-on set is ~2k characters for precision nobody reads. The two callers select the ENTITY rather than a column list, so rule_brief stays the single place deciding what a surfaced rule says. Backup: both new tables carried, area tags by canonical SLUG (ids are per-install). The rule-relation restore runs after ALL rules exist and after the catalog, because an edge names two rules and a tag names a global row — sections renumbered so the file reads in dependency order. A pre-0088 payload restores with tier=always_on, i.e. binding exactly as when it was taken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
67874268bb |
test(systems): move the name-gate cases to the service the gate moved into (#3028)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 10s
CI & Build / integration (push) Successful in 22s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Successful in 35s
Two tests further down test_mcp_tool_systems.py still drove the gate through the tool — `svc.list_systems` stubbed, the tool doing the normalising — so the new `await systems_svc.assess_system_name(...)` hit an unstubbed MagicMock. Stubbing them at the tool would have kept testing the wrong layer. The normalisation cases belong with the logic, so they move to tests/test_services_systems.py as real coverage of assess_system_name: case and whitespace folding, exact-beats-overlap (and that an exact hit short-circuits the lesser lookup), no invented match, fail-open on both arms, and silence for a nameless system. What stays the tool's job — rendering a duplicate, applying an exact area, offering an overlap — is already covered at the top of that file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c58529718b |
feat(systems): the catalog reaches the moment a name is minted, and gets a face (#3028, milestone 307 step 2)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 48s
CI & Build / integration (push) Successful in 38s
CI & Build / Python tests (push) Failing after 57s
CI & Build / Build & push image (push) Skipped
Step 1 found the reason the standard names never held, and it is sharper than "prose doesn't fire": the list WAS real and it WAS seeded — but only on the inception path, for a project with zero Systems. Ad-hoc create_system never consulted it, which is how Forge minted "CI and Release" and Portal minted "CI & release" after the constant already existed. This wires the vocabulary to the moment that mints a name. - services/systems.assess_system_name: the local duplicate gate AND the catalog lookup, in ONE service function both doors call. The gate lived only in the MCP tool, which is exactly how the web UI shipped without a check the agent surface enforced (#2482). REST now answers 409 with the System that already covers the area. - An `exact` catalog hit is APPLIED (mechanical — the names differ only in spelling). An `overlap` is only OFFERED, on both doors: applying a judgment call silently is how a cross-project rule surfaces in the wrong project. - canonical_systems.best_overlap is the ONE scorer behind the create-time offer and the review sweep, so the two surfaces can never name different areas for one System. It also takes the catalog the caller already holds, so the review is not an N+1. UI (folded in from step 1 — rule 27, that step shipped with no human surface): - SystemsSection: a Shared area picker on create and edit, the area on each card, and a collapsed review of proposals that appears only when there is something to decide. `exact` and `overlap` never share a style — one is mechanical, the other is the reviewer's judgment, and presenting them alike is how a wrong mapping gets waved through. - Settings → Admin → Areas: the catalog itself, showing each entry's slug, because the slug is what decides whether two names are the same area and a rename moves it. - A picker rather than a live matcher: reproducing the slug rule in TypeScript would give this feature two matchers to keep in step — the exact drift the catalog exists to end. The server stays authoritative. tests/helpers.fake_system gains canonical_id=None: an unnamed attribute is an auto-MagicMock and therefore truthy, which is the trap that helper exists for (note 2109) and a nullable FK walks straight into it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
879ef3053e |
fix(systems): the bootstrap ask reads the catalog, so its test must supply one (#3027)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 26s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m20s
CI & Build / Build & push image (push) Successful in 38s
CI caught the seam the promotion opened: the standard names now come from an async catalog read, and the unit test patches systems_svc wholesale — so the read raised, the fail-open swallowed it, and the ask shipped without the names it is supposed to carry. Stub standard_systems in that test, and assert the wiring rather than the vocabulary: THAT the seeded set is these eight is migration 0087's business and belongs in the inception integration test, against a real database. Adds the case the promotion actually created — an unreachable or empty catalog must still produce the ask. The names are an aid to the question, not the question; degrading to a weaker nudge is fine, going silent is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a97547fbc6 |
feat(systems): the area vocabulary becomes a global table so a rule can point at one (#3027, milestone 307 step 1)
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 39s
CI & Build / Python lint (push) Successful in 4s
CI & Build / integration (push) Successful in 30s
CI & Build / Python tests (push) Failing after 56s
CI & Build / Build & push image (push) Skipped
The eight standard area names already existed — as STANDARD_SYSTEMS, a tuple in
services/systems.py that milestone 297 seeds at inception. A constant cannot be
a foreign key, so nothing outside a project could reference an area: systems.
project_id is NOT NULL, and a rule that spans projects would have to chain
itself to one project's row. And because the list only ever applied on the
inception-seed path, three spellings of one area reached this instance anyway
(CI & runners / CI and Release / CI & release).
- canonical_systems: global, no user_id — a shared project inherits the
vocabulary instead of re-earning it. Migration 0087 seeds the same eight.
- systems.canonical_id: nullable, SET NULL. Association only — no System is
renamed and record_systems is untouched, so no record's tags move.
- canonical_slug folds &/and, case and punctuation, so spelling variants map
mechanically and a real difference ("CI & runners") becomes a proposal a
human confirms. propose_mappings reports; set_system_canonical is the only
writer.
- seed_standard_systems now reads the catalog and maps as it mints, so a
project born standard never needs a reconciliation pass.
- Catalog writes are admin-only; reads are open — a global list anyone can
extend stops being shared.
- backup: carried by SLUG, not id (ids are per-install). Restore reuses the
target's own rows and only creates entries an admin added on the source; an
unknown slug restores unmapped rather than failing.
Rule 22: STANDARD_SYSTEMS is removed, not deprecated. Rule 115: nothing seeded
names an app, repo or house convention. Design in note 3026.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
6b1f5e8031 |
Merge pull request 'Three fixes where the capability already existed and only the door was missing (#2782, #2975, #2278)' (#130) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 10s
CI & Build / integration (push) Successful in 25s
CI & Build / Python tests (push) Successful in 1m4s
CI & Build / Build & push image (push) Successful in 16s
|
||
|
|
a8f35e465e |
refactor(plugin+tests): the last two parallel-family gaps — pageable list tools, one config preamble (#2278)
CI & Build / Python lint (push) Successful in 4s
CI & Build / integration (push) Successful in 1m34s
CI & Build / Python tests (push) Successful in 2m21s
CI & Build / Plugin hooks (push) Successful in 27s
CI & Build / TypeScript typecheck (push) Successful in 36s
CI & Build / Build & push image (push) Successful in 25s
DRY pass 3's remainder. Both halves start from enumeration, because the task's
candidate list was hypotheses and the process requires counting before
proposing — and counting changed the answer twice.
## The list_* family: a limit with no offset
Enumerated all 19 `list_*` MCP tools first. They are genuinely heterogeneous —
8 take `project_id`, 6 take `limit`, six take no arguments at all — so a
common-parameter guard would invent a convention the API does not have, which
is the over-DRY trap (§5). One contract IS real: a `limit` without an `offset`
is a truncation with no continuation. The caller is told there are 250 results,
handed 50, and given no way to ask for the rest.
Two tools had it, and both were capped over a service that already accepted an
offset: `snippets_svc.list_snippets(offset=0)` was simply not exposed, and
`list_processes` passed a hardcoded `offset=0` into `query_knowledge`. The
capability existed one layer down in both; only the door was missing — the
missing-sibling shape exactly. Both now expose it.
`tests/test_mcp_list_family.py` guards it, with `list_tags` exempted for a
stated reason (a ranked top-N over a bounded vocabulary has no "rest" to page
into). Candidates derived, decision explicit, same design as test_mcp_auth —
plus the reverse checks: a stale exemption, and an offset with no limit, which
would page through an unbounded result set. Verified non-vacuous by running the
sweep against the pre-fix tree, where it fails naming both tools.
## The verb pairs: no finding, which is the finding
`preview`/`apply` and `dry_run`/`commit` do not exist anywhere in the 102
tools — those were guesses about a shape Scribe never adopted. `count_*` does
not exist either. Of the create/delete stems only `project_rule` lacks a
`delete_X`, and deliberately: a project rule IS a rule, `delete_rule` removes
it, and the docstring says so. `force` sits on 6 of 7 duplicate-gated creates;
the exception is `create_system`, whose gate is an exact normalized-NAME match
rather than a semantic near-match — forcing it would split one area's records
across two piles, which its own message explains. No guard added: it would
need a seven-entry exemption list to defend against a hypothetical. Recorded
on the leave-alone list instead, which the process asks for by name.
## The hook config preamble
Not 3 of 6 hooks as recorded — all FIVE carried their own copy, and of four
lines rather than two. The extra two are a guard treating an unexpanded
`${...}` placeholder as unset, so it is never sent as a garbage Bearer token:
precisely the correctness detail a sixth hook would omit with nothing failing
loudly. Now `scribe_config` in scribe_defs.sh, which also declares the two
names it owns. It sets globals rather than echoing, so a token never passes
through a subshell's output where xtrace or a log could catch it, and returns
a status so a caller can bail (`|| exit 0`) or continue degraded — the
session-context hook still owes its static floor when Scribe is unconfigured.
`check_plugin.py` now runs shellcheck with `-x`. Without it the shared helpers
were invisible: every variable they set read as unassigned and every bug inside
them went unlinted at the call site, which is the opposite of what sharing them
was for. All twelve fail-open scenarios still pass, and all five hooks were
probed live against the instance — prior_art and after_write both still name
canon, autoinject returns context, session_context serves 11k chars of rules,
sync_processes stays silent. Plugin 0.1.46 (#2209).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
64bfa5725f |
feat(telemetry): a read surface over retrieval_logs — the tuning loop had no read half (#2975)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / integration (push) Successful in 25s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m3s
CI & Build / Build & push image (push) Successful in 20s
`retrieval_logs` was write-only. `record_retrieval` inserted rows and nothing in the tree ever selected from them: the only `select()` over RetrievalLog lived in a test. So #1038's gate — "build the reranker once telemetry shows precision is the bottleneck" — was unsatisfiable by construction, and the one real tuning decision on record (the 0.68 write-path threshold, #2223) had to be reached by hand-probing the live instance with eight payloads. This adds the half that was missing. `retrieval_summary(user_id, days=30)` returns two aggregates side by side, each read from the table built for it — NOT a join. NoteUsageEvent's docstring is explicit that the two are complements ("RetrievalLog tunes the threshold, this tunes the corpus") and that RetrievalLog's JSONB `result_ids` cannot be indexed at the per-note grain, so correlating through it would be both slower and less honest than reading each source directly. That corrects the approach sketched on the task. - `sources`, per surface: calls, zero_result_calls, cleared_threshold (how often the best hit beat the threshold in force for THAT call), the top_score spread as p10/p50/p90/min/max, avg_result_count, p90 duration. Zero-result calls are counted apart from low-scoring ones — they are a different failure and averaging them together would hide both. - `usage`, from note_usage_events: ranked surfacings, ambient surfacings, and pulls split into `pulled_by_agent` / `pulled_by_human`. That split is not decoration. NoteUsageEvent's own comment says the mcp_/rest_ prefix is load-bearing and names #1038 while saying so: "is this dead weight?" is answered by any pull, "was that injected line useful?" only by an agent pull. `pull_through` exists to answer the second, so it counts agent pulls over ranked surfacings; both halves ship so the first stays answerable. Two things the code made me get right rather than guess: - Distinct-note counts get their own queries. `count(distinct note_id)` per (event, source) group cannot be summed across groups — a note surfaced by two sources is one distinct note and would be counted twice. A wrong number labelled "distinct" is worse than no number. - No CASE in the GROUP BY. #2663 is the bug where a second case() rendered its own expanding bind names, Postgres rejected the query, a broad except swallowed it, and every counter read zero in production while mocked tests passed. Grouping on raw `source` and classifying in Python cannot fail that way. For the same reason the readout distinguishes `read_failed` from an empty window, and its tests are integration against real Postgres — percentile_cont ... WITHIN GROUP only proves it parses against a database. Exposed as the `retrieval_telemetry` MCP tool, added to `_READ_ONLY_TOOLS`: it mutates nothing, but its name carries no read prefix, so the completeness test cannot derive it and it would otherwise have failed closed for read-only keys in silence — the same reason `enter_project` is spelled out there. Docs updated to name both exceptions rather than leave the rule looking derivable. Scoped to the caller's own telemetry: a retrieval log records what one user's agent asked for, query text included, and is not a shared record kind — the owner filter is the whole access rule, not a shortcut past access.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |