Merge pull request 'Lessons: the accretion reflex, and telemetry that can see across projects (#4195, #4196, #3735)' (#170) from dev into main
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 1m0s
CI & Build / Python tests (push) Successful in 1m43s
CI & Build / Build & push image (push) Successful in 15s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 1m0s
CI & Build / Python tests (push) Successful in 1m43s
CI & Build / Build & push image (push) Successful in 15s
This commit was merged in pull request #170.
This commit is contained in:
@@ -0,0 +1,62 @@
|
|||||||
|
"""note_usage_events records WHICH project the reader was in (#4196, #3735)
|
||||||
|
|
||||||
|
Revision ID: 0105
|
||||||
|
Revises: 0104
|
||||||
|
Create Date: 2026-09-20
|
||||||
|
|
||||||
|
Milestone 385. `RetrievalLog` has carried `project_id` from the start, so
|
||||||
|
"this record was SURFACED on project B" has always been answerable. The usage
|
||||||
|
table never had one, so "this record was OPENED on project B" never was — and
|
||||||
|
the two cannot be joined to recover it, because there is no session identity
|
||||||
|
server-side (see the NoteUsageEvent docstring, which rules that out on
|
||||||
|
purpose).
|
||||||
|
|
||||||
|
That gap sits exactly on the question milestone 385 exists to answer. A
|
||||||
|
lesson's whole claim is that it reaches a session on a project it was not
|
||||||
|
written on; step 8's acceptance is "retrieved on a different project, at the
|
||||||
|
moment it applies, AND opened". Surfacing cross-project: answerable. Opening:
|
||||||
|
answerable. Both at once: not, until this column.
|
||||||
|
|
||||||
|
WHICH PROJECT THIS IS, because the name is ambiguous and the wrong reading
|
||||||
|
makes the column useless. It is the project the READER was working in at the
|
||||||
|
moment of the event — NOT the project the record belongs to. The record's own
|
||||||
|
project is already on `notes.project_id`; copying it here would answer a
|
||||||
|
question nobody asked and quietly look like it answered this one.
|
||||||
|
|
||||||
|
NULLABLE, and not backfilled. Every existing row was written by a reader who
|
||||||
|
was somewhere, but naming it would be inventing a fact. Null means "not
|
||||||
|
reported", which is a different answer from a project id that might be wrong —
|
||||||
|
the same reasoning 0104 applied to its calibration stamps.
|
||||||
|
|
||||||
|
NULL IS ALSO THE ONGOING DEGRADED CASE, not only a historical one. A surfacing
|
||||||
|
arm always knows the project it searched; a getter only knows what its caller
|
||||||
|
passed. A pull with no project is still a pull and still counts toward "is this
|
||||||
|
dead weight" — it simply cannot answer "was it opened away from home".
|
||||||
|
|
||||||
|
THE INDEX carries `note_id` first, matching `ix_note_usage_note_event`: every
|
||||||
|
readout starts from a set of note ids and narrows, never from a project.
|
||||||
|
"""
|
||||||
|
import sqlalchemy as sa
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
revision = "0105"
|
||||||
|
down_revision = "0104"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column(
|
||||||
|
"note_usage_events",
|
||||||
|
sa.Column("project_id", sa.Integer(), nullable=True),
|
||||||
|
)
|
||||||
|
op.create_index(
|
||||||
|
"ix_note_usage_note_project",
|
||||||
|
"note_usage_events",
|
||||||
|
["note_id", "project_id"],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_index("ix_note_usage_note_project", table_name="note_usage_events")
|
||||||
|
op.drop_column("note_usage_events", "project_id")
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "scribe",
|
"name": "scribe",
|
||||||
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
||||||
"version": "2026.09.19.1614",
|
"version": "2026.09.20.0317",
|
||||||
"author": {
|
"author": {
|
||||||
"name": "Bryan Van Deusen"
|
"name": "Bryan Van Deusen"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -110,15 +110,27 @@ Two constraints on *how* that's achieved:
|
|||||||
silently, because it sometimes answers that what you are holding was never
|
silently, because it sometimes answers that what you are holding was never
|
||||||
a rule — and routing an observation is not losing it.
|
a rule — and routing an observation is not losing it.
|
||||||
|
|
||||||
**A preference is the one record you keep current yourself.** When the
|
**A preference is yours to keep current.** When the operator corrects you,
|
||||||
operator corrects you, or the preference on file no longer matches how they
|
or the preference on file no longer matches how they actually want
|
||||||
actually want something done, `update_preference` — that is expected, not a
|
something done, `update_preference` — that is expected, not a liberty, and
|
||||||
liberty, and updating one mid-work is the normal case rather than an
|
updating one mid-work is the normal case rather than an interruption of it.
|
||||||
interruption of it. It wants the task or note that taught the change. Say in the
|
It wants the task or note that taught the change. Say in the same turn that
|
||||||
same turn that you did it, so they can disagree while it is in front of
|
you did it, so they can disagree while it is in front of them. A rule waits
|
||||||
them. A rule waits for the operator instead: `create_rule` proposes and
|
for the operator instead: `create_rule` proposes and asks. If what you
|
||||||
asks. If what you learned is that something MUST be done a certain way,
|
learned is that something MUST be done a certain way, that is a rule to
|
||||||
that is a rule to propose, not a preference to harden in place.
|
propose, not a preference to harden in place.
|
||||||
|
|
||||||
|
**A lesson grows each time it proves itself.** When one arrives and the
|
||||||
|
situation it names is the one you are actually in, you are the single
|
||||||
|
reader placed to tell whether its trigger is keyed right and whether its
|
||||||
|
claim covers what you are seeing. `update_lesson` takes what you now know:
|
||||||
|
another incident added to what taught it, the claim stated more exactly,
|
||||||
|
or — the edit worth most — a trigger re-keyed to the situation that really
|
||||||
|
fired. A lesson nobody reaches is seldom wrong; far more often it is
|
||||||
|
waiting in a situation nobody is in. One claim that has met the same
|
||||||
|
failure four times is worth more than four claims that each met it once,
|
||||||
|
so when a near-duplicate create hands back an existing id, that is the
|
||||||
|
record to grow.
|
||||||
|
|
||||||
**Preferences shape how work is done, never what gets recorded.** They
|
**Preferences shape how work is done, never what gets recorded.** They
|
||||||
govern your conduct — how you report, how carefully you pace, which form
|
govern your conduct — how you report, how carefully you pace, which form
|
||||||
|
|||||||
@@ -177,8 +177,22 @@ async def create_lesson(
|
|||||||
return data
|
return data
|
||||||
|
|
||||||
|
|
||||||
async def get_lesson(lesson_id: int) -> dict:
|
async def get_lesson(lesson_id: int, project_id: int = 0) -> dict:
|
||||||
"""Fetch one lesson by id, with its trigger and sources read back out."""
|
"""Fetch one lesson by id, with its trigger and sources read back out.
|
||||||
|
|
||||||
|
IF THIS LESSON JUST PROVED ITSELF, IT IS WORTH MORE THAN IT SAYS. You are
|
||||||
|
reading it inside the situation it names, which makes you the one reader
|
||||||
|
who can tell whether its trigger is keyed to what actually fired and
|
||||||
|
whether its claim covers what you are seeing. `update_lesson` takes
|
||||||
|
another incident into `learned_from`, a claim stated more exactly, or a
|
||||||
|
re-keyed trigger — and the trigger is the edit that pays most, because a
|
||||||
|
lesson keyed to a situation nobody is in looks exactly like one nobody
|
||||||
|
needed.
|
||||||
|
|
||||||
|
`project_id` is the project you are WORKING IN, not this record's own.
|
||||||
|
Passing the active project is what makes "opened away from where it was
|
||||||
|
written" answerable; 0 leaves it unreported and the pull still counts.
|
||||||
|
"""
|
||||||
uid = current_user_id()
|
uid = current_user_id()
|
||||||
note = await lessons_svc.get_lesson(uid, lesson_id)
|
note = await lessons_svc.get_lesson(uid, lesson_id)
|
||||||
if note is None:
|
if note is None:
|
||||||
@@ -189,7 +203,10 @@ async def get_lesson(lesson_id: int) -> dict:
|
|||||||
# retrieval as any other note, so a getter that records nothing would leave
|
# retrieval as any other note, so a getter that records nothing would leave
|
||||||
# the kind permanently at zero pulls — reading as dead weight beside kinds
|
# the kind permanently at zero pulls — reading as dead weight beside kinds
|
||||||
# that merely had a counter (#2476, the repeat of #2245).
|
# that merely had a counter (#2476, the repeat of #2245).
|
||||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_lesson")
|
record_pulled(
|
||||||
|
user_id=uid, note_id=int(note.id),
|
||||||
|
source="mcp_get_lesson", project_id=project_id,
|
||||||
|
)
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -67,7 +67,7 @@ async def list_notes(
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
async def get_note(note_id: int) -> dict:
|
async def get_note(note_id: int, project_id: int = 0) -> dict:
|
||||||
"""Fetch the full content of a single Scribe note by its ID.
|
"""Fetch the full content of a single Scribe note by its ID.
|
||||||
|
|
||||||
Returns id, title, body (markdown), tags, project_id, created_at,
|
Returns id, title, body (markdown), tags, project_id, created_at,
|
||||||
@@ -80,6 +80,10 @@ async def get_note(note_id: int) -> dict:
|
|||||||
this one up to date. It is still here and still readable — supersession
|
this one up to date. It is still here and still readable — supersession
|
||||||
demotes, it never hides — but read it as what was true when written, and
|
demotes, it never hides — but read it as what was true when written, and
|
||||||
open the newer note before acting on it.
|
open the newer note before acting on it.
|
||||||
|
|
||||||
|
`project_id` is the project you are WORKING IN, not this record's own.
|
||||||
|
Passing the active project is what makes "opened away from where it was
|
||||||
|
written" answerable; 0 leaves it unreported and the pull still counts.
|
||||||
"""
|
"""
|
||||||
uid = current_user_id()
|
uid = current_user_id()
|
||||||
loaded = await notes_svc.get_note_for_user(uid, note_id)
|
loaded = await notes_svc.get_note_for_user(uid, note_id)
|
||||||
@@ -92,7 +96,10 @@ async def get_note(note_id: int) -> dict:
|
|||||||
# menu surfaces notes, tasks and processes too, so restricting this to
|
# menu surfaces notes, tasks and processes too, so restricting this to
|
||||||
# snippets would leave those permanently at zero pulls and make them look
|
# snippets would leave those permanently at zero pulls and make them look
|
||||||
# like dead weight next to snippets that merely had a counter (#2085).
|
# like dead weight next to snippets that merely had a counter (#2085).
|
||||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_note")
|
record_pulled(
|
||||||
|
user_id=uid, note_id=int(note.id),
|
||||||
|
source="mcp_get_note", project_id=project_id,
|
||||||
|
)
|
||||||
await supersession_svc.attach_relations(uid, note_id, out, hint=True)
|
await supersession_svc.attach_relations(uid, note_id, out, hint=True)
|
||||||
await systems_tools.attach_systems(
|
await systems_tools.attach_systems(
|
||||||
uid, getattr(note, "user_id", uid) or uid, out, note.id, note.project_id
|
uid, getattr(note, "user_id", uid) or uid, out, note.id, note.project_id
|
||||||
|
|||||||
@@ -102,7 +102,7 @@ async def create_process(
|
|||||||
return note.to_dict()
|
return note.to_dict()
|
||||||
|
|
||||||
|
|
||||||
async def get_process(name_or_id: str) -> dict:
|
async def get_process(name_or_id: str, project_id: int = 0) -> dict:
|
||||||
"""Fetch a stored process by name or id and return its full prompt — the
|
"""Fetch a stored process by name or id and return its full prompt — the
|
||||||
fire mechanism. The operator says "run the <name> process"; call this and
|
fire mechanism. The operator says "run the <name> process"; call this and
|
||||||
follow the returned body (including any 'clarify first' steps it contains).
|
follow the returned body (including any 'clarify first' steps it contains).
|
||||||
@@ -127,6 +127,10 @@ async def get_process(name_or_id: str) -> dict:
|
|||||||
follow-it-as-written contract above applies only to the operator's own
|
follow-it-as-written contract above applies only to the operator's own
|
||||||
processes — a shared one is a proposal, and running it unasked would put
|
processes — a shared one is a proposal, and running it unasked would put
|
||||||
someone else's judgement in charge of this session.
|
someone else's judgement in charge of this session.
|
||||||
|
|
||||||
|
`project_id` is the project you are WORKING IN, not this record's own.
|
||||||
|
Passing the active project is what makes "opened away from where it was
|
||||||
|
written" answerable; 0 leaves it unreported and the pull still counts.
|
||||||
"""
|
"""
|
||||||
uid = current_user_id()
|
uid = current_user_id()
|
||||||
note, candidates = await notes_svc.resolve_process(uid, name_or_id)
|
note, candidates = await notes_svc.resolve_process(uid, name_or_id)
|
||||||
@@ -141,7 +145,10 @@ async def get_process(name_or_id: str) -> dict:
|
|||||||
# this, the getter the product points at is the one getter that records
|
# this, the getter the product points at is the one getter that records
|
||||||
# nothing, and every process sits permanently at zero pulls looking like dead
|
# nothing, and every process sits permanently at zero pulls looking like dead
|
||||||
# weight beside kinds that merely had a counter (#2476, the repeat of #2245).
|
# weight beside kinds that merely had a counter (#2476, the repeat of #2245).
|
||||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_process")
|
record_pulled(
|
||||||
|
user_id=uid, note_id=int(note.id),
|
||||||
|
source="mcp_get_process", project_id=project_id,
|
||||||
|
)
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -214,6 +214,7 @@ async def enter_project(project_id: int) -> dict:
|
|||||||
user_id=uid,
|
user_id=uid,
|
||||||
note_ids=[int(t.id) for t in open_tasks],
|
note_ids=[int(t.id) for t in open_tasks],
|
||||||
source="enter_project",
|
source="enter_project",
|
||||||
|
project_id=project_id,
|
||||||
)
|
)
|
||||||
# A project need not have one, and most installs won't — null is ordinary
|
# A project need not have one, and most installs won't — null is ordinary
|
||||||
# here, not a missing prerequisite. Summary only: the guidance is ~9k of
|
# here, not a missing prerequisite. Summary only: the guidance is ~9k of
|
||||||
|
|||||||
@@ -209,7 +209,7 @@ async def create_snippet(
|
|||||||
return data
|
return data
|
||||||
|
|
||||||
|
|
||||||
async def get_snippet(snippet_id: int) -> dict:
|
async def get_snippet(snippet_id: int, project_id: int = 0) -> dict:
|
||||||
"""Fetch a snippet by id — the full record: code, signature, location, and a
|
"""Fetch a snippet by id — the full record: code, signature, location, and a
|
||||||
parsed `snippet` field of its structured parts.
|
parsed `snippet` field of its structured parts.
|
||||||
|
|
||||||
@@ -236,6 +236,10 @@ async def get_snippet(snippet_id: int) -> dict:
|
|||||||
`owner` and your `permission`. Read that as ONE PERSON'S SUGGESTION, not as
|
`owner` and your `permission`. Read that as ONE PERSON'S SUGGESTION, not as
|
||||||
established practice here: judge it on its merits, say whose it is when you
|
established practice here: judge it on its merits, say whose it is when you
|
||||||
reference it, and don't adopt it as the house pattern without checking.
|
reference it, and don't adopt it as the house pattern without checking.
|
||||||
|
|
||||||
|
`project_id` is the project you are WORKING IN, not this record's own.
|
||||||
|
Passing the active project is what makes "opened away from where it was
|
||||||
|
written" answerable; 0 leaves it unreported and the pull still counts.
|
||||||
"""
|
"""
|
||||||
uid = current_user_id()
|
uid = current_user_id()
|
||||||
note = await snippets_svc.get_snippet(uid, snippet_id)
|
note = await snippets_svc.get_snippet(uid, snippet_id)
|
||||||
@@ -250,7 +254,10 @@ async def get_snippet(snippet_id: int) -> dict:
|
|||||||
# snippets_svc.get_snippet — the service is also reached by update/merge
|
# snippets_svc.get_snippet — the service is also reached by update/merge
|
||||||
# paths, and counting those would inflate exactly the number that is
|
# paths, and counting those would inflate exactly the number that is
|
||||||
# supposed to mean "someone chose to look at this" (#2085).
|
# supposed to mean "someone chose to look at this" (#2085).
|
||||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_snippet")
|
record_pulled(
|
||||||
|
user_id=uid, note_id=int(note.id),
|
||||||
|
source="mcp_get_snippet", project_id=project_id,
|
||||||
|
)
|
||||||
await systems_tools.attach_systems(
|
await systems_tools.attach_systems(
|
||||||
uid, note.user_id, data, note.id, note.project_id
|
uid, note.user_id, data, note.id, note.project_id
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ async def list_tasks(
|
|||||||
return {"tasks": [notes_svc.brief_row(n, titles) for n in rows], "total": total}
|
return {"tasks": [notes_svc.brief_row(n, titles) for n in rows], "total": total}
|
||||||
|
|
||||||
|
|
||||||
async def get_task(task_id: int) -> dict:
|
async def get_task(task_id: int, project_id: int = 0) -> dict:
|
||||||
"""Fetch a single Scribe task by ID.
|
"""Fetch a single Scribe task by ID.
|
||||||
|
|
||||||
Returns id, title, body, status, priority, tags, project_id, milestone_id,
|
Returns id, title, body, status, priority, tags, project_id, milestone_id,
|
||||||
@@ -91,6 +91,10 @@ async def get_task(task_id: int) -> dict:
|
|||||||
|
|
||||||
A task another user shared with you also carries `shared`, `owner` and
|
A task another user shared with you also carries `shared`, `owner` and
|
||||||
`permission` — it's their work item, not one you took on.
|
`permission` — it's their work item, not one you took on.
|
||||||
|
|
||||||
|
`project_id` is the project you are WORKING IN, not this record's own.
|
||||||
|
Passing the active project is what makes "opened away from where it was
|
||||||
|
written" answerable; 0 leaves it unreported and the pull still counts.
|
||||||
"""
|
"""
|
||||||
uid = current_user_id()
|
uid = current_user_id()
|
||||||
loaded = await notes_svc.get_note_for_user(uid, task_id)
|
loaded = await notes_svc.get_note_for_user(uid, task_id)
|
||||||
@@ -123,7 +127,10 @@ async def get_task(task_id: int) -> dict:
|
|||||||
await systems_tools.attach_systems(
|
await systems_tools.attach_systems(
|
||||||
uid, getattr(note, "user_id", uid) or uid, data, note.id, note.project_id
|
uid, getattr(note, "user_id", uid) or uid, data, note.id, note.project_id
|
||||||
)
|
)
|
||||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_task")
|
record_pulled(
|
||||||
|
user_id=uid, note_id=int(note.id),
|
||||||
|
source="mcp_get_task", project_id=project_id,
|
||||||
|
)
|
||||||
return data
|
return data
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -66,12 +66,29 @@ class NoteUsageEvent(Base, CreatedAtMixin):
|
|||||||
# so never aggregate across the prefix without saying why (#1038, #2085).
|
# so never aggregate across the prefix without saying why (#1038, #2085).
|
||||||
source: Mapped[str] = mapped_column(Text, nullable=False)
|
source: Mapped[str] = mapped_column(Text, nullable=False)
|
||||||
|
|
||||||
|
# The project the READER was in when this happened — not the project the
|
||||||
|
# record belongs to, which is already on `notes.project_id`. The whole
|
||||||
|
# point is the comparison between the two: a record opened somewhere other
|
||||||
|
# than where it was written is the evidence that it TRANSFERRED, which is
|
||||||
|
# the claim the lesson kind rests on (milestone 385) and the one thing
|
||||||
|
# RetrievalLog could half-answer and this table could not answer at all.
|
||||||
|
#
|
||||||
|
# Nullable, and null is ordinary rather than historical. A surfacing arm
|
||||||
|
# always knows the project it searched; a getter knows only what its caller
|
||||||
|
# passed, and a caller that passed nothing is reporting "unknown", not
|
||||||
|
# "none". Treat a null as unreported: it still counts toward "is this dead
|
||||||
|
# weight", and it cannot speak to "was it opened away from home".
|
||||||
|
project_id: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
||||||
|
|
||||||
__table_args__ = (
|
__table_args__ = (
|
||||||
# The readout is always "these note ids, split by event" — a covering
|
# The readout is always "these note ids, split by event" — a covering
|
||||||
# composite beats separate single-column indexes for it.
|
# composite beats separate single-column indexes for it.
|
||||||
Index("ix_note_usage_note_event", "note_id", "event"),
|
Index("ix_note_usage_note_event", "note_id", "event"),
|
||||||
Index("ix_note_usage_created_at", "created_at"),
|
Index("ix_note_usage_created_at", "created_at"),
|
||||||
Index("ix_note_usage_user_id", "user_id"),
|
Index("ix_note_usage_user_id", "user_id"),
|
||||||
|
# note_id first, like ix_note_usage_note_event: every readout starts
|
||||||
|
# from a set of note ids and narrows, never from a project.
|
||||||
|
Index("ix_note_usage_note_project", "note_id", "project_id"),
|
||||||
)
|
)
|
||||||
|
|
||||||
def to_dict(self) -> dict:
|
def to_dict(self) -> dict:
|
||||||
|
|||||||
@@ -333,7 +333,7 @@ def _usage_event_rows(rows) -> list[dict]:
|
|||||||
return [
|
return [
|
||||||
{
|
{
|
||||||
"user_id": r.user_id, "note_id": r.note_id, "event": r.event,
|
"user_id": r.user_id, "note_id": r.note_id, "event": r.event,
|
||||||
"source": r.source,
|
"source": r.source, "project_id": r.project_id,
|
||||||
"created_at": r.created_at.isoformat() if r.created_at else None,
|
"created_at": r.created_at.isoformat() if r.created_at else None,
|
||||||
}
|
}
|
||||||
for r in rows
|
for r in rows
|
||||||
@@ -1548,6 +1548,12 @@ async def _restore_v2(data: dict) -> dict:
|
|||||||
note_id=mapped_nid,
|
note_id=mapped_nid,
|
||||||
event=ev.get("event", ""),
|
event=ev.get("event", ""),
|
||||||
source=ev.get("source", ""),
|
source=ev.get("source", ""),
|
||||||
|
# Degrades to None rather than dropping the row: unlike a shape
|
||||||
|
# event, a usage event without a project is still a real pull,
|
||||||
|
# and discarding it would deflate the pull-through this table
|
||||||
|
# exists to report. Null already means "not reported".
|
||||||
|
project_id=(project_id_map.get(ev["project_id"])
|
||||||
|
if ev.get("project_id") else None),
|
||||||
created_at=_dt(ev.get("created_at")),
|
created_at=_dt(ev.get("created_at")),
|
||||||
))
|
))
|
||||||
stats["note_usage_events"] += 1
|
stats["note_usage_events"] += 1
|
||||||
|
|||||||
@@ -85,13 +85,40 @@ def _schedule(rows: list[dict]) -> None:
|
|||||||
task.add_done_callback(_pending.discard)
|
task.add_done_callback(_pending.discard)
|
||||||
|
|
||||||
|
|
||||||
|
def _project_or_none(project_id: int | None) -> int | None:
|
||||||
|
"""0 and None both mean "no project reported" — store one of them.
|
||||||
|
|
||||||
|
Callers reach this from two conventions at once: the MCP tools spell "no
|
||||||
|
project" as `0` (it is an int parameter with an int default), while the
|
||||||
|
column is nullable. Folding them here keeps every call site from having to
|
||||||
|
remember which one this function wants, and stops a row claiming it was
|
||||||
|
read on project #0.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
pid = int(project_id or 0)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
return None
|
||||||
|
return pid or None
|
||||||
|
|
||||||
|
|
||||||
def record_surfaced(
|
def record_surfaced(
|
||||||
*, user_id: int | None, note_ids: list[int] | set[int], source: str
|
*,
|
||||||
|
user_id: int | None,
|
||||||
|
note_ids: list[int] | set[int],
|
||||||
|
source: str,
|
||||||
|
project_id: int | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Fire-and-forget: record that these notes were shown to the agent.
|
"""Fire-and-forget: record that these notes were shown to the agent.
|
||||||
|
|
||||||
Takes the whole menu at once — one insert per surfacing event, not per note
|
Takes the whole menu at once — one insert per surfacing event, not per note
|
||||||
— because a menu is a single decision and its rows should land together.
|
— because a menu is a single decision and its rows should land together.
|
||||||
|
|
||||||
|
`project_id` is where the READER was, not where the record lives. Every
|
||||||
|
surfacing arm knows it — it is the scope it just searched — so pass it;
|
||||||
|
it is what makes "surfaced away from home" answerable without joining
|
||||||
|
RetrievalLog. 0 is normalised to None: a project id of zero means "no
|
||||||
|
project" everywhere else in this codebase, and storing it would read as
|
||||||
|
project #0.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
rows = [
|
rows = [
|
||||||
@@ -100,6 +127,7 @@ def record_surfaced(
|
|||||||
"note_id": int(nid),
|
"note_id": int(nid),
|
||||||
"event": SURFACED,
|
"event": SURFACED,
|
||||||
"source": source,
|
"source": source,
|
||||||
|
"project_id": _project_or_none(project_id),
|
||||||
}
|
}
|
||||||
for nid in note_ids
|
for nid in note_ids
|
||||||
]
|
]
|
||||||
@@ -109,8 +137,25 @@ def record_surfaced(
|
|||||||
_schedule(rows)
|
_schedule(rows)
|
||||||
|
|
||||||
|
|
||||||
def record_pulled(*, user_id: int | None, note_id: int, source: str) -> None:
|
def record_pulled(
|
||||||
"""Fire-and-forget: record that a note was opened in full."""
|
*,
|
||||||
|
user_id: int | None,
|
||||||
|
note_id: int,
|
||||||
|
source: str,
|
||||||
|
project_id: int | None = None,
|
||||||
|
) -> None:
|
||||||
|
"""Fire-and-forget: record that a note was opened in full.
|
||||||
|
|
||||||
|
`project_id` is where the READER was — the caller's active project, never
|
||||||
|
the record's own. Compared against the record's `project_id`, it answers
|
||||||
|
whether this was opened somewhere other than where it was written, which
|
||||||
|
is the evidence that a record TRANSFERRED (milestone 385, #3735).
|
||||||
|
|
||||||
|
Unlike a surfacing arm, a getter only knows what it was handed, so this
|
||||||
|
stays optional and null is an ordinary answer meaning "not reported". A
|
||||||
|
pull with no project is still a pull: it counts toward dead-weight
|
||||||
|
detection and simply cannot speak to transfer.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
rows = [
|
rows = [
|
||||||
{
|
{
|
||||||
@@ -118,6 +163,7 @@ def record_pulled(*, user_id: int | None, note_id: int, source: str) -> None:
|
|||||||
"note_id": int(note_id),
|
"note_id": int(note_id),
|
||||||
"event": PULLED,
|
"event": PULLED,
|
||||||
"source": source,
|
"source": source,
|
||||||
|
"project_id": _project_or_none(project_id),
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
except Exception:
|
except Exception:
|
||||||
|
|||||||
@@ -872,6 +872,7 @@ async def _reserve_slot_for_lesson(
|
|||||||
if slot_id not in already:
|
if slot_id not in already:
|
||||||
record_surfaced(
|
record_surfaced(
|
||||||
user_id=user_id, note_ids=[slot_id], source="lesson_slot",
|
user_id=user_id, note_ids=[slot_id], source="lesson_slot",
|
||||||
|
project_id=project_id,
|
||||||
)
|
)
|
||||||
return kept + slot, slot_id
|
return kept + slot, slot_id
|
||||||
|
|
||||||
@@ -1076,6 +1077,7 @@ async def build_autoinject_hint(
|
|||||||
if i not in already and i != lesson_slot_id
|
if i not in already and i != lesson_slot_id
|
||||||
],
|
],
|
||||||
source="auto_inject",
|
source="auto_inject",
|
||||||
|
project_id=project_id,
|
||||||
)
|
)
|
||||||
|
|
||||||
return {"context": "\n".join(lines), "note_ids": note_ids, "config": cfg}
|
return {"context": "\n".join(lines), "note_ids": note_ids, "config": cfg}
|
||||||
@@ -2178,7 +2180,9 @@ async def build_write_path_hint(
|
|||||||
else "write_path_semantic")
|
else "write_path_semantic")
|
||||||
by_arm.setdefault(arm, []).append(int(item["id"]))
|
by_arm.setdefault(arm, []).append(int(item["id"]))
|
||||||
for arm, ids in by_arm.items():
|
for arm, ids in by_arm.items():
|
||||||
record_surfaced(user_id=user_id, note_ids=ids, source=arm)
|
record_surfaced(
|
||||||
|
user_id=user_id, note_ids=ids, source=arm, project_id=project_id,
|
||||||
|
)
|
||||||
|
|
||||||
# ── Standing rules that may apply here (milestone 307) ──────────────
|
# ── Standing rules that may apply here (milestone 307) ──────────────
|
||||||
#
|
#
|
||||||
|
|||||||
@@ -118,7 +118,8 @@ TOPICS: tuple[Topic, ...] = (
|
|||||||
"an empty session is not evidence of an empty rulebook",
|
"an empty session is not evidence of an empty rulebook",
|
||||||
index=('content_type="rule"', "nothing matched")),
|
index=('content_type="rule"', "nothing matched")),
|
||||||
Topic("rules bind, preferences guide and are kept current", U, ("preference", "update_preference"),
|
Topic("rules bind, preferences guide and are kept current", U, ("preference", "update_preference"),
|
||||||
"a preference is the one record you keep current yourself", index=("preferences guide",)),
|
"updating one mid-work is the normal case rather than an interruption of it",
|
||||||
|
index=("preferences guide",)),
|
||||||
Topic("kind is force: a rule binds, a preference guides, a lesson informs", U,
|
Topic("kind is force: a rule binds, a preference guides, a lesson informs", U,
|
||||||
("lesson", "costs time", "costs consistency"),
|
("lesson", "costs time", "costs consistency"),
|
||||||
"what happens if someone doesn't do this",
|
"what happens if someone doesn't do this",
|
||||||
@@ -129,6 +130,14 @@ TOPICS: tuple[Topic, ...] = (
|
|||||||
# mis-routing #3733 recorded happens, and the caller there has not
|
# mis-routing #3733 recorded happens, and the caller there has not
|
||||||
# necessarily read the skill.
|
# necessarily read the skill.
|
||||||
shared_with=("docstrings",)),
|
shared_with=("docstrings",)),
|
||||||
|
Topic("a lesson grows each time it proves itself", U,
|
||||||
|
("update_lesson", "re-keyed", "grows each time"),
|
||||||
|
"one claim that has met the same failure four times is worth more "
|
||||||
|
"than four claims that each met it once",
|
||||||
|
# No index marker. The index names session-START reflexes, and this
|
||||||
|
# one fires mid-work, the moment a lesson arrives in a situation it
|
||||||
|
# actually names. `lessons inform` already points at the kind.
|
||||||
|
shared_with=("docstrings",)),
|
||||||
Topic("preferences shape how work is done, never what is recorded", U,
|
Topic("preferences shape how work is done, never what is recorded", U,
|
||||||
("never what gets recorded",),
|
("never what gets recorded",),
|
||||||
"a preference never makes a task into a note"),
|
"a preference never makes a task into a note"),
|
||||||
|
|||||||
@@ -41,7 +41,8 @@ async def test_record_pulled_writes_a_single_row():
|
|||||||
record_pulled(user_id=7, note_id=11, source="mcp_get_snippet")
|
record_pulled(user_id=7, note_id=11, source="mcp_get_snippet")
|
||||||
rows = sched.call_args[0][0]
|
rows = sched.call_args[0][0]
|
||||||
assert rows == [
|
assert rows == [
|
||||||
{"user_id": 7, "note_id": 11, "event": "pulled", "source": "mcp_get_snippet"}
|
{"user_id": 7, "note_id": 11, "event": "pulled",
|
||||||
|
"source": "mcp_get_snippet", "project_id": None}
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -201,3 +201,110 @@ def test_every_named_loader_still_exists():
|
|||||||
f"renamed or moved. Update the list, or the pull check silently stops "
|
f"renamed or moved. Update the list, or the pull check silently stops "
|
||||||
f"covering whatever used them."
|
f"covering whatever used them."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── the reading project travels with the event (#4196, #3735) ────────────────
|
||||||
|
#
|
||||||
|
# The same sibling-drift shape as above, one dimension over. `RetrievalLog` has
|
||||||
|
# always carried `project_id`, so "surfaced on project B" was answerable; the
|
||||||
|
# usage table had none, so "opened on project B" was not, and the two cannot be
|
||||||
|
# joined (no session identity server-side — NoteUsageEvent's own docstring).
|
||||||
|
# That gap sat exactly on milestone 385's acceptance: a lesson's claim is that
|
||||||
|
# it reaches a session on a project it was NOT written on.
|
||||||
|
#
|
||||||
|
# A getter that takes the project and forgets to pass it fails the same way a
|
||||||
|
# missing record_pulled did: silently, changing no return value, and showing up
|
||||||
|
# only as a column that is mysteriously always null.
|
||||||
|
|
||||||
|
|
||||||
|
def _pulling_getters():
|
||||||
|
"""(module, name, source) for every getter that records a pull."""
|
||||||
|
for module, name, body in _getters():
|
||||||
|
if "record_pulled" in body:
|
||||||
|
yield module, name, body
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_getter_that_pulls_takes_the_reading_project():
|
||||||
|
"""Asserted on structure (rule 167): a behavioural test cannot see a
|
||||||
|
parameter that was never threaded through."""
|
||||||
|
missing = [
|
||||||
|
f"{module}.{name}"
|
||||||
|
for module, name, body in _pulling_getters()
|
||||||
|
if "project_id: int = 0" not in body.split("\n")[0]
|
||||||
|
]
|
||||||
|
assert not missing, (
|
||||||
|
f"these getters record a pull but cannot say where the reader was: "
|
||||||
|
f"{missing}. Add `project_id: int = 0` to the signature — see "
|
||||||
|
f"mcp/tools/notes.py:get_note."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_getter_that_pulls_passes_the_project_through():
|
||||||
|
"""Taking the argument and dropping it is worse than not taking it: the
|
||||||
|
signature advertises a dimension the column never receives."""
|
||||||
|
checked, missing = [], []
|
||||||
|
for module, name, body in _pulling_getters():
|
||||||
|
checked.append(f"{module}.{name}")
|
||||||
|
if "project_id=project_id" not in body:
|
||||||
|
missing.append(f"{module}.{name}")
|
||||||
|
assert checked, "found no pulling getters; the detection must have moved"
|
||||||
|
assert not missing, (
|
||||||
|
f"these getters accept a project and do not hand it to record_pulled: "
|
||||||
|
f"{missing}."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# The arms are the other half: unlike a getter, a surfacing arm ALWAYS knows
|
||||||
|
# the project — it is the scope it just searched — so there is no excuse for a
|
||||||
|
# null there, and `lesson_slot` is the one milestone 385 is measured through.
|
||||||
|
#
|
||||||
|
# `process_skill_sync` is exempt and named here rather than pattern-matched: it
|
||||||
|
# installs every Process the operator can reach, which is not a project-scoped
|
||||||
|
# question, so a project on that row would be a fiction.
|
||||||
|
SURFACING_ARMS_WITHOUT_A_PROJECT = {"process_skill_sync"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_project_scoped_surfacing_arm_records_its_project():
|
||||||
|
import ast as _ast
|
||||||
|
|
||||||
|
src = (_SRC / "services" / "plugin_context.py").read_text()
|
||||||
|
missing = []
|
||||||
|
for node in _ast.walk(_ast.parse(src)):
|
||||||
|
if not (isinstance(node, _ast.Call)
|
||||||
|
and getattr(node.func, "id", "") == "record_surfaced"):
|
||||||
|
continue
|
||||||
|
kwargs = {k.arg: k.value for k in node.keywords}
|
||||||
|
source = kwargs.get("source")
|
||||||
|
named = (source.value
|
||||||
|
if isinstance(source, _ast.Constant) else "<computed>")
|
||||||
|
if named in SURFACING_ARMS_WITHOUT_A_PROJECT:
|
||||||
|
continue
|
||||||
|
if "project_id" not in kwargs:
|
||||||
|
missing.append(f"{named} (line {node.lineno})")
|
||||||
|
assert not missing, (
|
||||||
|
f"these surfacing arms know the project they searched and do not "
|
||||||
|
f"record it: {missing}. Pass project_id=project_id, or add the arm to "
|
||||||
|
f"SURFACING_ARMS_WITHOUT_A_PROJECT with the reason it has none."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reading_project_guards_can_fail():
|
||||||
|
"""Rule 167: shown turning red once, so a guard that has quietly stopped
|
||||||
|
matching anything is distinguishable from one that passes."""
|
||||||
|
assert list(_pulling_getters()), "the pulling-getter detection matches nothing"
|
||||||
|
fake = "async def get_thing(thing_id: int) -> dict:\n record_pulled(x)"
|
||||||
|
assert "project_id: int = 0" not in fake.split("\n")[0]
|
||||||
|
assert "project_id=project_id" not in fake
|
||||||
|
|
||||||
|
|
||||||
|
def test_zero_and_none_both_mean_no_project_reported():
|
||||||
|
"""0 is how the MCP tools spell "no project" (int parameter, int default);
|
||||||
|
the column is nullable. A row claiming project #0 would be a fiction."""
|
||||||
|
from scribe.services.note_usage import _project_or_none
|
||||||
|
|
||||||
|
assert _project_or_none(0) is None
|
||||||
|
assert _project_or_none(None) is None
|
||||||
|
assert _project_or_none("") is None
|
||||||
|
assert _project_or_none("nonsense") is None
|
||||||
|
assert _project_or_none(2) == 2
|
||||||
|
assert _project_or_none("2") == 2
|
||||||
|
|||||||
@@ -245,6 +245,42 @@ def test_every_column_is_exported_or_declared_excluded(table):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_usage_importer_restores_the_reading_project():
|
||||||
|
"""The column guard above checks the EXPORT side only.
|
||||||
|
|
||||||
|
A column can be exported faithfully and then dropped on the way back in,
|
||||||
|
which restores a backup that reports success and has quietly lost a
|
||||||
|
dimension — #3182's failure mode, one direction over. There is no general
|
||||||
|
import-side guard yet; this covers the column #4196 added, by source
|
||||||
|
inspection, because the behavioural path needs Postgres.
|
||||||
|
|
||||||
|
It also pins the DEGRADE. `code_shape_events` skips a row whose project
|
||||||
|
will not map, because a shape event without its project says nothing. A
|
||||||
|
usage event is not like that: the project is optional by design and null
|
||||||
|
already means "not reported", so an unmappable one must restore as
|
||||||
|
unreported rather than vanish — dropping it would lose a real pull and
|
||||||
|
deflate the very pull-through this table exists to report.
|
||||||
|
"""
|
||||||
|
import inspect
|
||||||
|
|
||||||
|
src = inspect.getsource(backup._restore_v2)
|
||||||
|
marker = 'for ev in data.get("note_usage_events", []):'
|
||||||
|
assert marker in src, "the usage import loop moved; this guard is blind"
|
||||||
|
block = src[src.index(marker):][:1200]
|
||||||
|
|
||||||
|
assert 'ev.get("project_id")' in block, (
|
||||||
|
"the usage importer drops project_id — a restore would report success "
|
||||||
|
"and come back without the reading project"
|
||||||
|
)
|
||||||
|
assert "project_id_map" in block, (
|
||||||
|
"project_id must be re-mapped; a raw id points at whatever project "
|
||||||
|
"happens to hold that number in the destination install"
|
||||||
|
)
|
||||||
|
assert "continue" not in block.split('project_id=')[1][:200], (
|
||||||
|
"an unmappable project must degrade to None, not skip the row"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_the_column_guard_covers_every_table_with_a_row_helper():
|
def test_the_column_guard_covers_every_table_with_a_row_helper():
|
||||||
"""The guard is only as good as its registry — a table added to _BACKED_UP
|
"""The guard is only as good as its registry — a table added to _BACKED_UP
|
||||||
with a new helper, and not to the registry, would be unguarded and look
|
with a new helper, and not to the registry, would be unguarded and look
|
||||||
|
|||||||
Reference in New Issue
Block a user