feat(telemetry): a usage event records which project the reader was in (#4196, #3735)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m5s
CI & Build / Python tests (push) Failing after 1m15s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m5s
CI & Build / Python tests (push) Failing after 1m15s
CI & Build / Build & push image (push) Skipped
`RetrievalLog` has carried `project_id` since it existed, so "this record was SURFACED on project B" was always answerable. `note_usage_events` had none, so "this record was OPENED on project B" was not — and the two cannot be joined to recover it, because there is deliberately no session identity server-side. NoteUsageEvent's own docstring rules that out. That gap sat 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, and step 8's acceptance is "retrieved on a different project AND opened". Each half was answerable; the conjunction was not. WHICH project, because the name is ambiguous and the wrong reading makes the column useless: it is the project the READER was in, never the one the record belongs to. The record's own project is already on the note; copying it here would answer a question nobody asked while looking like it answered this one. The surfacing half is free — every arm already holds the scope it just searched, so auto_inject, lesson_slot, the write-path arms and enter_project now record it. process_skill_sync does not and should not: it installs every Process the operator can reach, which is not a project-scoped question, so a project there would be a fiction. The pull half needs the caller, since a getter knows only what it was handed. The five single-record getters take `project_id: int = 0` and pass it through, following the convention `search` and `create_*` already set. Null stays an ordinary answer meaning "not reported" — a pull with no project is still a pull and still counts toward dead weight; it simply cannot speak to transfer. The four REST detail views report none for now: a human opening a record in a browser is a different event from an agent recalling one, and #2245 left that asymmetry deliberately undecided. Guarded the way #2245 and #2476 taught: by source inspection, because a parameter that was never threaded through changes no return value and shows up only as a column that is mysteriously always null. Three guards — the signature, the pass-through, and the arms — plus the can-fail test rule 167 asks for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -85,13 +85,40 @@ def _schedule(rows: list[dict]) -> None:
|
||||
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(
|
||||
*, 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:
|
||||
"""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
|
||||
— 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:
|
||||
rows = [
|
||||
@@ -100,6 +127,7 @@ def record_surfaced(
|
||||
"note_id": int(nid),
|
||||
"event": SURFACED,
|
||||
"source": source,
|
||||
"project_id": _project_or_none(project_id),
|
||||
}
|
||||
for nid in note_ids
|
||||
]
|
||||
@@ -109,8 +137,25 @@ def record_surfaced(
|
||||
_schedule(rows)
|
||||
|
||||
|
||||
def record_pulled(*, user_id: int | None, note_id: int, source: str) -> None:
|
||||
"""Fire-and-forget: record that a note was opened in full."""
|
||||
def record_pulled(
|
||||
*,
|
||||
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:
|
||||
rows = [
|
||||
{
|
||||
@@ -118,6 +163,7 @@ def record_pulled(*, user_id: int | None, note_id: int, source: str) -> None:
|
||||
"note_id": int(note_id),
|
||||
"event": PULLED,
|
||||
"source": source,
|
||||
"project_id": _project_or_none(project_id),
|
||||
}
|
||||
]
|
||||
except Exception:
|
||||
|
||||
@@ -872,6 +872,7 @@ async def _reserve_slot_for_lesson(
|
||||
if slot_id not in already:
|
||||
record_surfaced(
|
||||
user_id=user_id, note_ids=[slot_id], source="lesson_slot",
|
||||
project_id=project_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
|
||||
],
|
||||
source="auto_inject",
|
||||
project_id=project_id,
|
||||
)
|
||||
|
||||
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")
|
||||
by_arm.setdefault(arm, []).append(int(item["id"]))
|
||||
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) ──────────────
|
||||
#
|
||||
|
||||
Reference in New Issue
Block a user