vrr/ouroboros
2
0
Fork 0
mirror of https://github.com/razzant/ouroboros.git synced 2026-10-03 04:07:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix: attribute and complete post-task learning inputs

Browse source 
A root's post-task reflection received `recent_advisory_runs` belonging to
ANOTHER task on the same checkout with their identity stripped, and narrated
those failures as its own. The Pattern Register updater then got only
`reflection[:500]`: the clip cut the exculpatory clause mid-word, so the
register recorded the inverse conclusion (fail closed on REVIEW_REQUIRED,
contrary to an Advisory-enforcement install) and bumped one row twice from two
roots of a single owner request.

Commit 1b7f94973 already un-clipped the sibling argument with the rationale
that a prefix can never authorize a whole-document rewrite. This completes its
missed half; learning input stays a projection over records that already exist,
with no new taxonomy, no live-memory rewrite, no retry and no second reviewer.

- review_evidence: advisory runs are split by ROW IDENTITY into this task's own
  rows (plus legacy rows with no recorded owner, which stay unknown rather than
  being re-attributed) and `foreign_advisory_runs`. Repository readiness
  (current_repo, obligations, debts, the exact-snapshot match) stays
  repository-scoped, and `has_evidence` stays true when only foreign rows exist.
  The split cannot key on the repository key, because an empty one widens the
  candidate list to every advisory run on the drive. Each row now carries its
  owning task, attempt, phase and complete snapshot hash (additive keys), and
  the prompt renders the foreign group last, under a heading naming the owning
  task ids, inside the same character budget.
- reflection: the Pattern Register call receives the WHOLE reflection and the
  exact goal. `goal_exact` is stored beside the bounded `goal` display field
  that logs and UI read, so neither replaces the other. The transaction is
  unchanged: full old-register read, LLM call outside the knowledge lock,
  re-read and exact compare under it, history before atomic replacement, one
  index rebuild. A losing compare-and-swap now warns WHICH task's learning was
  dropped, and the two swallowed exception sites warn with the exception text.
- knowledge tools: `patterns` joins `improvement-backlog` and `overview` as a
  global-only topic (owner decision Q15). The register's writer and every
  reader address the canonical drive, so a project room could otherwise mint a
  second register nobody opens.

Co-authored-by: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com>
This commit is contained in:
Ouroboros 2026-09-15 16:48:37 +03:00
parent c306b40d88
commit ddd5af5e5f
7 changed files with 372 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

6
docs/ARCHITECTURE.md
View file

@ -1792,10 +1792,14 @@ Rationale. The review ran without tools for as long as it did because its guaran
#### Post-task reflection
The root post-task checkpoint decides whether an error-bearing or non-trivial run warrants Experience Review. `reflection.generate_reflection` sends the Light route a bounded task goal, trace summary, tool-use profile, errors, review and child evidence, and the same frozen non-final cost snapshot the task summary uses; it runs outside the tool loop, records its own usage, and its failure never erases the delivered result or changes a review verdict. Admission to the Pattern Register is typed rather than a word scan: it opens on a call the loop recorded as errored (its stamped `tool_result_code`, or the recorded status for a legacy row), on a producer fact that names a failure the ok status cannot carry (a preserved commit whose post-commit tests failed publishes `post_commit_tests`), on the typed codes already stored with an entry, or on a genuinely FAILED child, whose classes reach the root through the child evidence the synthesis walk already collects. A child that was cancelled, soft-landed best-effort or ended degraded is not a failure and does not open it. Those same failed-child classes make the run error-bearing for both the trigger and the existing error-reflection prompt, which names the classes when root calls were clean, from that one walk: children do not reflect, so a short clean root that delegated the work is the only place its child's failure can be learned from at all. Deliberately not "a reason code exists", which would open the register on every terminal; children still contribute evidence and never reflect themselves.
The root post-task checkpoint decides whether an error-bearing or non-trivial run warrants Experience Review. `reflection.generate_reflection` sends the Light route the EXACT task goal — the request decides what the run was for, so it is never a prefix — beside bounded projections of the trace summary, tool-use profile, errors, and review and child evidence, and the same frozen non-final cost snapshot the task summary uses; it runs outside the tool loop, records its own usage, and its failure never erases the delivered result or changes a review verdict. Admission to the Pattern Register is typed rather than a word scan: it opens on a call the loop recorded as errored (its stamped `tool_result_code`, or the recorded status for a legacy row), on a producer fact that names a failure the ok status cannot carry (a preserved commit whose post-commit tests failed publishes `post_commit_tests`), on the typed codes already stored with an entry, or on a genuinely FAILED child, whose classes reach the root through the child evidence the synthesis walk already collects. A child that was cancelled, soft-landed best-effort or ended degraded is not a failure and does not open it. Those same failed-child classes make the run error-bearing for both the trigger and the existing error-reflection prompt, which names the classes when root calls were clean, from that one walk: children do not reflect, so a short clean root that delegated the work is the only place its child's failure can be learned from at all. Deliberately not "a reason code exists", which would open the register on every terminal; children still contribute evidence and never reflect themselves.
A reflection lands where it durably belongs: a non-project root appends the full entry to the canonical `logs/task_reflections.jsonl`; a project-scoped root appends the full entry to its project drive and the canonical log receives only a bounded pointer row — full project text never enters the canonical log, which feeds future global context. A project-bound task's context includes a bounded labeled tail of its own project's reflections; the headless mirror drive of a split root is never the reflection home, and the Pattern Register update stays canonical in both cases. Every entry carries task identity, evidence, lessons, backlog candidates, and validated memory actions. `MEMORY_ACTIONS_JSON` permits only `scratchpad_append`, `knowledge_write`, and `identity_update_candidate`, at bounded count and size. `apply_memory_actions` routes accepted actions through provenance-preserving memory and knowledge APIs. An `identity_update_candidate` is recorded in the scratchpad for review and is never auto-written to `identity.md`. For a project-scoped task, reflection applies knowledge actions only (the project store by default, explicit global allowed); its scratchpad and identity-candidate actions are skipped because this automatic Light pass lacks the conversation's full view — the conversation itself writes identity and scratchpad from any room through its own tools. Reflection may propose a future campaign or backlog item, but it cannot enqueue, review, commit, or enable one.
The Pattern Register writer (`reflection._update_patterns`) REPLACES the whole document, so every decision input it reads is complete: the full current register, the exact goal (`goal_exact`, stored beside the bounded `goal` display field that logs and UI have always shown), and the whole reflection text. A prefix of a decision input can never authorize the rewrite — a 500-character clip once cut an exculpatory clause mid-word and the register recorded the inverse of what the reflection concluded. `patterns` is a reserved global-only topic alongside `improvement-backlog` and `overview`: whichever room writes it, the register has ONE home on the canonical drive, the only one its writer and its readers (context assembly, deep self-review, the headless copy) address. The Light call stays outside the knowledge lock while the re-read, exact compare, history append and atomic replacement are one critical section; a register that moved under a losing writer is preserved and that task's learning is DROPPED with a warning naming the task, never retried against a source it did not decide from. Count provenance: a row's count is bumped once per observed episode, so two roots of one owner request bump it twice — the number counts episodes, not distinct requests.
The advisory rows a reflection or summary reads are ATTRIBUTED. Advisory runs are scoped by repository and several tasks legitimately review one checkout, so `collect_review_evidence` renders as `recent_advisory_runs` only the rows this task owns, plus legacy rows carrying no owner at all, which stay unknown rather than being re-attributed in either direction; another task's rows reach the prompt under their own heading naming the owning task ids, and every row now carries its owning task, attempt, phase and complete snapshot hash. Repository readiness (`current_repo`, open obligations, commit-readiness debt, the exact-snapshot match) stays repository-scoped, because that is what "can this checkout be committed" means. The split is keyed on ROW IDENTITY and never on a scope key: an empty repository key widens the candidate list to every advisory run on the drive, and the installation's history is not one task's record.
Only the root runs full post-task synthesis once (`root_phase_checkpoint` makes the paid phase at-most-once across restart); children contribute evidence, never a second global synthesis. The owner's final answer does not wait for blocking synthesis: after the durable result is stored, the final `send_message` is delivered immediately while the buffered-return copy is RETAINED (queue.put is not a delivery receipt); both copies carry one `delivery_id` and the supervisor suppresses the duplicate through the durable registry in `supervisor/terminal_delivery.py`. The same file holds a bounded PENDING outbox — ONE seam for the normal, cancel, and reap terminal paths: a terminal answer is recorded as owed BEFORE it is enqueued and cleared in the same write that marks it delivered, so a crash between settle and send replays it instead of losing it. Every root's answer enters this outbox at durable-result persistence time under the canonical `final:<tid>:<digest>` id. Replays back off and are bounded; an exhausted or capacity-evicted row is dropped LOUDLY (full text preserved on disk, typed `terminal_delivery_exhausted` event, chat notice) — external transports stay at-least-once and that residual is disclosed. `task_done` still goes last through the buffered return — an early `task_done` would release the queue slot and start child-drive cleanup while post-task still runs — so a worker reaped during a hung synthesis has already delivered the answer. Synthesis receives a sealed final package from the durable result: submitted final text, its artifact manifest, and completion_observations. The terminal writer preserves full redacted action observations in the canonical artifact store (`task.budget_drive_root or drive_root`) before compact publication. Completion sources use the existing write-once `source_handles/context_checkpoints` store with verified `task_source` refs; the published-ref closure includes completion observations before child cleanup. Sources stay outside deliverables and inferred readiness. Their native reader selector uses `get_task_result(include_completion_source=true)` with the source task id and its canonical drive. It first returns complete character length/hash, then explicit `source_start_char`/`source_end_char` ranges; `artifacts.text_source_range_projection` shares the unchanged work-order range contract. Source bytes, kind, path containment and SHA are checked before any excerpt. Packet-only summary/reflection receive per-send-tool counts, each family's latest recorded return, and task-related skill readiness with coverage; full-source references are for later readers, not evidence the synthesizer has read. Positive observed facts correct error-trace impressions, while tool success does not prove owner receipt, empty material does not prove absence, and skill readiness does not attribute an owner's action to the task. Recovery uses these stored observations; task-summary requests/responses use chat_observed.
Pooled workers retain their slot until root post-task synthesis settles, for API-only and subscription tasks alike; early final-answer delivery keeps the response independent from that queue timing. Ordinary native post-work stays on its registered actor thread without a pooled worker slot. Its existing `TaskModelWait` owner remains available through `POST_TASK_SYNTHESIS_INFLIGHT` after ordinary dialogue admission closes; detached server post-work binds its own live owner in the same registry. Temporary role overrides follow that owner, and the existing task mailbox remains available until the terminal post-task checkpoint. Gateway decisions use the phase owner independently of closed dialogue admission, and activity hydration enriches the existing direct row with its finalizing state and model waits. An open phase remains finalizing rather than appearing completed. Typed quota/auth waits resume only the unsettled call, while stop or unknown outcomes degrade the phase without repeating finished stages. Restart recovery still degrades an indeterminate `running` phase rather than replaying a possibly paid request.

42
ouroboros/reflection.py
View file

@ -516,7 +516,13 @@ def generate_reflection(
"ts": utc_now_iso(),
"task_id": task.get("id", ""),
"task_type": str(task.get("type", "")),
# Two goal fields with one owner each: ``goal`` is the bounded DISPLAY
# field every log/UI reader has always shown, ``goal_exact`` is the
# request as the owner wrote it. A destructive writer (the Pattern
# Register replaces its whole document) must decide from the exact text,
# not from a 200-char display prefix that can end mid-sentence.
"goal": goal,
"goal_exact": str(task.get("text") or ""),
"rounds": None if usage_dict.get("loop_evidence_unavailable") else int(usage_dict.get("rounds", 0)),
"cost_usd": (
round(float(usage_dict["cost"]), 4)
@ -638,8 +644,11 @@ def append_reflection(drive_root: pathlib.Path, entry: Dict[str, Any]) -> None:
if _admits_pattern_register(entry):
try:
_update_patterns(drive_root, entry)
except Exception:
log.debug("Pattern register update failed (non-critical)", exc_info=True)
except Exception as exc:
# Learning that silently fails to land is invisible erosion: the
# register simply never hears about this class again (P1).
log.warning("Pattern register update failed for task %s: %s",
entry.get("task_id", "?"), exc, exc_info=True)
def append_reflection_routed(env: Any, task: Dict[str, Any], entry: Dict[str, Any]) -> None:
@ -679,8 +688,9 @@ def append_reflection_routed(env: Any, task: Dict[str, Any], entry: Dict[str, An
if _admits_pattern_register(entry):
try:
_update_patterns(canonical, entry)
except Exception:
log.debug("Pattern register update failed (non-critical)", exc_info=True)
except Exception as exc:
log.warning("Pattern register update failed for task %s: %s",
entry.get("task_id", "?"), exc, exc_info=True)
try:
append_jsonl(canonical / "logs" / REFLECTIONS_FILENAME, {
"ts": str(entry.get("ts") or utc_now_iso()),
@ -741,13 +751,16 @@ def _update_patterns(drive_root: pathlib.Path, entry: Dict[str, Any]) -> None:
current = _PATTERNS_HEADER
prompt = _PATTERNS_PROMPT.format(
# This call replaces the whole file, so its decision input must be the
# complete current register. Provider overflow/error is handled by the
# caller as an abstention; a prefix can never authorize the rewrite.
# This call replaces the whole file, so EVERY decision input must be
# complete: the current register, the exact goal, and the whole
# reflection. Provider overflow/error is handled by the caller as an
# abstention; a prefix can never authorize the rewrite. A 500-char clip
# of the reflection once cut an exculpatory clause mid-word and the
# register recorded the inverse of what the reflection concluded.
current_patterns=current,
goal=_truncate_with_notice(entry.get("goal", "?"), 200),
goal=str(entry.get("goal_exact") or entry.get("goal") or "?"),
markers=", ".join(entry.get("key_markers", [])),
reflection=_truncate_with_notice(entry.get("reflection", ""), 500),
reflection=str(entry.get("reflection") or ""),
)
light_model = get_light_model()
@ -792,7 +805,16 @@ def _update_patterns(drive_root: pathlib.Path, entry: Dict[str, Any]) -> None:
log.warning("Pattern register source became unavailable; preserving it")
return
if latest != current:
log.info("Pattern register changed during update; preserving the newer source")
# The newer source wins and this rewrite is abandoned — so the
# learning it carried is DROPPED, not deferred. Name whose it was:
# a silent "preserving the newer source" hid which task's lesson the
# register never recorded. No retry (a second paid call would decide
# from a register that has moved again).
log.warning(
"Pattern register changed during this update; preserving the newer source. "
"Task %s learning was NOT recorded in the register (no retry).",
str(entry.get("task_id") or "?"),
)
return
if not append_jsonl(drive_root / "memory" / "knowledge" / "patterns_history.jsonl", {
"ts": utc_now_iso(),

108
ouroboros/review_evidence.py
View file

@ -655,6 +655,34 @@ def commit_review_evidence_section(evidence: dict, *, delivery: str, compact: bo
return header + "\nRecorded excerpt:\n" + truncate_within_limit(preview, room)
def _split_advisory_runs_by_owner(runs: List[Any], task_id: str) -> tuple[List[Any], List[Any]]:
"""Split repository advisory runs into this task's own rows and another task's.
Keyed on ROW IDENTITY, never on a scope key. Advisory runs are scoped by
repository, and several tasks legitimately review the same checkout, so a
repo-scoped list mixes owners: a root's post-task reflection was handed
another task's failed advisory commands with their identity stripped and
narrated them as its own failures. An empty ``repo_key`` widens the
candidate list to every advisory run on the drive, which is why the split
cannot key on that key either — the installation's history is not this
task's record.
A row carrying no ``task_id`` is legacy provenance: unknown, never
re-attributed in either direction, and left in the repository group it has
always been rendered in. A caller with no ``task_id`` (repository-readiness
surfaces) keeps whole-repository semantics, where nothing is foreign.
"""
current = str(task_id or "")
if not current:
return list(runs), []
own: List[Any] = []
foreign: List[Any] = []
for run in runs:
owner = str(getattr(run, "task_id", "") or "")
(foreign if owner and owner != current else own).append(run)
return own, foreign
def collect_review_evidence(
drive_root: Any,
*,
@ -665,6 +693,16 @@ def collect_review_evidence(
max_obligations: int | None = None,
max_continuations: int = 3,
) -> Dict[str, Any]:
"""Project this task's commit/advisory review record over the durable ledger.
Repository readiness (``current_repo``, obligations, debts, the exact
snapshot match) stays repository-scoped — that is what "can this checkout be
committed" means. The RUN LISTS are attributed: ``recent_advisory_runs``
holds only rows this task owns (plus legacy rows with no recorded owner),
while another task's rows on the same checkout are carried separately under
``foreign_advisory_runs`` so a reader cannot mistake them for this task's
own work.
"""
from ouroboros.review_state import (
_LEGACY_CURRENT_REPO_KEY,
advisory_commit_ready,
@ -687,6 +725,7 @@ def collect_review_evidence(
repo_runs = state.filter_advisory_runs(repo_key=repo_key)
else:
repo_runs = all_runs
own_runs, foreign_runs = _split_advisory_runs_by_owner(repo_runs, task_id)
if task_id:
scoped_attempts = state.filter_attempts(task_id=task_id)
@ -731,8 +770,10 @@ def collect_review_evidence(
},
"recent_attempts": [_attempt_to_dict(item) for item in (scoped_attempts[-max_attempts:] if max_attempts > 0 else [])],
"omitted_attempts": max(0, len(scoped_attempts) - max_attempts) if max_attempts > 0 else len(scoped_attempts),
"recent_advisory_runs": [_run_to_dict(item) for item in (repo_runs[-max_runs:] if max_runs > 0 else [])],
"omitted_advisory_runs": max(0, len(repo_runs) - max_runs) if max_runs > 0 else len(repo_runs),
"recent_advisory_runs": [_run_to_dict(item) for item in (own_runs[-max_runs:] if max_runs > 0 else [])],
"omitted_advisory_runs": max(0, len(own_runs) - max_runs) if max_runs > 0 else len(own_runs),
"foreign_advisory_runs": [_run_to_dict(item) for item in (foreign_runs[-max_runs:] if max_runs > 0 else [])],
"omitted_foreign_advisory_runs": max(0, len(foreign_runs) - max_runs) if max_runs > 0 else len(foreign_runs),
"open_obligations": [_obligation_to_dict(item) for item in (open_obligations[:max_obligations] if max_obligations is not None else open_obligations)],
"omitted_obligations": max(0, len(open_obligations) - max_obligations) if max_obligations is not None else 0,
"commit_readiness_debts": [_debt_to_dict(item) for item in open_debts],
@ -744,6 +785,10 @@ def collect_review_evidence(
evidence["has_evidence"] = any([
evidence["recent_attempts"],
evidence["recent_advisory_runs"],
# Another task's runs are still evidence about this checkout: the lens
# must not report "nothing recorded" when it is holding rows it simply
# may not attribute to this task.
evidence["foreign_advisory_runs"],
evidence["open_obligations"],
evidence["commit_readiness_debts"],
evidence["continuations"],
@ -752,6 +797,7 @@ def collect_review_evidence(
# Omission counters signal truncated evidence even when visible lists are empty
evidence["omitted_attempts"] > 0,
evidence["omitted_advisory_runs"] > 0,
evidence["omitted_foreign_advisory_runs"] > 0,
evidence["omitted_obligations"] > 0,
evidence["omitted_continuations"] > 0,
evidence["omitted_corrupt"] > 0,
@ -781,6 +827,33 @@ def _acceptance_panel_prompt_row(panel: Dict[str, Any]) -> Dict[str, Any]:
return row
_FOREIGN_ADVISORY_KEYS = ("foreign_advisory_runs", "omitted_foreign_advisory_runs")
def _foreign_advisory_section(evidence: Dict[str, Any]) -> str:
"""Render another task's advisory runs under a heading that says whose they are.
Rows split out by ``collect_review_evidence`` are repository context, so
they must reach the reader — but never inside the block a summariser or a
reflection reads as "my review record". The heading names the owning task
ids, because the identity is exactly what a reader needs to not adopt the
failures.
"""
rows = [row for row in (evidence.get("foreign_advisory_runs") or []) if isinstance(row, dict)]
omitted = int(evidence.get("omitted_foreign_advisory_runs") or 0)
if not rows and not omitted:
return ""
owners = sorted({str(row.get("task_id") or "") for row in rows} - {""})
return (
"ADVISORY RUNS OF OTHER TASKS (NOT this task's work — do not report them as my own):\n"
f"owning task_ids: {', '.join(owners) if owners else '(none recorded)'}; "
f"shown={len(rows)}; omitted={omitted}.\n"
"These ran on the same checkout for a different task. They are evidence about the\n"
"repository, never this task's errors, decisions or obligations.\n"
+ json.dumps(rows, ensure_ascii=False, indent=2)
)
def format_review_evidence_for_prompt(
evidence: Dict[str, Any],
*,
@ -798,7 +871,14 @@ def format_review_evidence_for_prompt(
``acceptance_panels`` leads with the task's OWN acceptance-panel projection.
The commit/advisory lens knows nothing about it, so its absence statement
names the lens it describes rather than claiming the task bought no review.
Another task's advisory runs leave the main JSON body entirely and are
rendered last, under their own attributing heading.
"""
evidence = evidence if isinstance(evidence, dict) else {}
foreign_section = _foreign_advisory_section(evidence)
if foreign_section:
evidence = {key: value for key, value in evidence.items() if key not in _FOREIGN_ADVISORY_KEYS}
rows = [
_acceptance_panel_prompt_row(panel)
for panel in (acceptance_panels if isinstance(acceptance_panels, list) else [])
@ -842,6 +922,9 @@ def format_review_evidence_for_prompt(
if evidence and evidence.get("has_evidence"):
rendered_evidence = json.dumps(evidence, ensure_ascii=False, indent=2)
prefix_chars = len(sections[0]) + 2 if sections else 0
# The attributing section is part of the same budget: it may shorten the
# body, never ride beyond the bound the caller asked for.
prefix_chars += len(foreign_section) + 2 if foreign_section else 0
limit = max_chars - prefix_chars if max_chars > 0 else 0
if source_ref and max_chars > 0 and len(rendered_evidence) > max(1, limit):
rendered_evidence = truncate_review_artifact(
@ -852,6 +935,12 @@ def format_review_evidence_for_prompt(
f"canonical source_ref={json.dumps(source_ref, ensure_ascii=False)}"
)
sections.append(rendered_evidence)
if foreign_section:
room = max_chars - sum(len(section) + 2 for section in sections) if max_chars > 0 else 0
sections.append(
truncate_review_artifact(foreign_section, limit=max(1, room))
if max_chars > 0 and len(foreign_section) > max(1, room) else foreign_section
)
if not sections:
return "(no commit/advisory review evidence recorded for this task)"
return "\n\n".join(sections)
@ -887,7 +976,16 @@ _RESPONDED_STATUSES = frozenset({"fresh", "stale"})
def _run_to_dict(item: Any) -> Dict[str, Any]:
"""Serialise AdvisoryRunRecord with responded/skipped/error status summary."""
"""Serialise AdvisoryRunRecord with responded/skipped/error status summary.
The row carries its own IDENTITY: the owning ``task_id`` ("" = a legacy row
written before the field existed — unknown, never re-attributed), the
complete ``snapshot_hash`` of the tree that was reviewed (a 12-char prefix
cannot be joined back to an exact snapshot), and the ``attempt``/``phase``
of the lifecycle that produced it. A stripped row is why one root narrated
another task's advisory failures as its own. The keys are additive; every
historical reader keeps working.
"""
valid_items = [entry for entry in list(getattr(item, "items", []) or []) if isinstance(entry, dict)]
fail_items = [
{
@ -914,6 +1012,10 @@ def _run_to_dict(item: Any) -> Dict[str, Any]:
return {
"ts": str(getattr(item, "ts", "") or ""),
"task_id": str(getattr(item, "task_id", "") or ""),
"attempt": int(getattr(item, "attempt", 0) or 0),
"phase": str(getattr(item, "phase", "") or ""),
"snapshot_hash": str(getattr(item, "snapshot_hash", "") or ""),
"status": status,
"status_summary": status_summary,
"repo_key": str(getattr(item, "repo_key", "") or ""),

14
ouroboros/tools/knowledge.py
View file

@ -16,11 +16,15 @@ from ouroboros.utils import append_jsonl, utc_now_iso
KNOWLEDGE_DIR = "memory/knowledge"
BACKLOG_TOPIC = "improvement-backlog"
PATTERNS_TOPIC = "patterns"
# Reserved shared topics with exactly one home. Whichever room asks for them,
# they resolve to the global shelf: the backlog is the P7 SSOT, and the overview
# is the shared orientation every context loads. A project copy of either would
# be a second source of truth nobody reads.
GLOBAL_ONLY_TOPICS = frozenset({BACKLOG_TOPIC, OVERVIEW_TOPIC})
# they resolve to the global shelf: the backlog is the P7 SSOT, the overview is
# the shared orientation every context loads, and the Pattern Register is
# cross-project cognition whose only writer (post-task reflection) and only
# readers (context, deep self-review, the headless copy) all use the canonical
# drive. A project copy of any of them would be a second source of truth nobody
# reads.
GLOBAL_ONLY_TOPICS = frozenset({BACKLOG_TOPIC, OVERVIEW_TOPIC, PATTERNS_TOPIC})
# Existing consolidator and Pattern Register imports share this exact lock.
_knowledge_write_lock = knowledge_store.knowledge_write_lock
@ -171,7 +175,7 @@ def _knowledge_list(ctx: ToolContext, scope: str = "") -> str:
def get_tools() -> List[ToolEntry]:
topic = {"type": "string", "description": "Shelf-relative topic path without .md; nested paths and Unicode names are supported; no scope prefixes (global/, project/)."}
scope = {"type": "string", "description": "global or project:<exact project id>. Omitted uses this task's project shelf, otherwise global. Global knowledge remains explicitly reachable from a project. Understanding of people and relationships, and anything that should outlive the project, belongs in global. Reserved topics (improvement-backlog, overview) always resolve to global."}
scope = {"type": "string", "description": "global or project:<exact project id>. Omitted uses this task's project shelf, otherwise global. Global knowledge remains explicitly reachable from a project. Understanding of people and relationships, and anything that should outlive the project, belongs in global. Reserved topics (improvement-backlog, overview, patterns) always resolve to global."}
return [
ToolEntry("knowledge_read", {
"name": "knowledge_read",

128
tests/test_collect_review_evidence.py
View file

@ -3,9 +3,137 @@
Split out of ``tests/test_agent_task_pipeline.py`` when that module was divided
by theme; every moved block is verbatim. Covers task-scoped recent attempts,
repo-scoped open obligations, and commit-readiness debt extraction.
Also covers advisory-run ATTRIBUTION. Repository readiness is repository-scoped
by design, but the run lists are not: a root's reflection received another
task's failed advisory rows with their identity stripped and narrated them as
its own failures. The split is a projection over existing records, so it is the
same under advisory and blocking review enforcement; only ``repo_commit_ready``
reads enforcement at all.
"""
def _repo_with_history(tmp_path, name="repo"):
"""A checkout with a tracked file, so ``compute_snapshot_hash`` is stable."""
repo_dir = tmp_path / name
repo_dir.mkdir(parents=True)
(repo_dir / ".git").mkdir()
(repo_dir / "tracked.py").write_text("print('hello')\n", encoding="utf-8")
return repo_dir
_FOREIGN_FAILURE = "deleted the release notes instead of editing them"
def _advisory_run(*, snapshot_hash, repo_key, task_id, status="fresh", failing=""):
from ouroboros.review_state import AdvisoryRunRecord
return AdvisoryRunRecord(
snapshot_hash=snapshot_hash,
commit_message=f"advisory for {task_id or '(legacy)'}",
status=status,
ts="2026-08-08T10:00:00+00:00",
repo_key=repo_key,
task_id=task_id,
attempt=1,
phase="advisory",
items=[{"verdict": "FAIL", "severity": "critical", "item": "destructive_edit",
"reason": failing}] if failing else [],
)
def test_another_tasks_advisory_failures_are_not_rendered_as_this_tasks_own(tmp_path):
"""The reproduced incident: A reflects, and B's FAIL rows are B's, labelled."""
from ouroboros.review_evidence import collect_review_evidence, format_review_evidence_for_prompt
from ouroboros.review_state import AdvisoryReviewState, make_repo_key, save_state
repo_dir = _repo_with_history(tmp_path)
repo_key = make_repo_key(repo_dir)
state = AdvisoryReviewState()
state.add_run(_advisory_run(snapshot_hash="b-snapshot", repo_key=repo_key,
task_id="task-b", failing=_FOREIGN_FAILURE))
save_state(tmp_path, state)
evidence = collect_review_evidence(tmp_path, task_id="task-a", repo_dir=repo_dir)
assert evidence["recent_advisory_runs"] == []
assert evidence["omitted_advisory_runs"] == 0
assert [row["task_id"] for row in evidence["foreign_advisory_runs"]] == ["task-b"]
assert evidence["foreign_advisory_runs"][0]["findings"][0]["reason"] == _FOREIGN_FAILURE
assert evidence["foreign_advisory_runs"][0]["snapshot_hash"] == "b-snapshot"
assert evidence["has_evidence"] is True
rendered = format_review_evidence_for_prompt(evidence, max_chars=8000)
heading = rendered.find("ADVISORY RUNS OF OTHER TASKS")
assert heading >= 0
assert "task-b" in rendered[heading:]
# The failure text exists ONLY after the attributing heading: nothing above
# it can be read as this task's own record.
assert rendered.find(_FOREIGN_FAILURE) > heading
assert _FOREIGN_FAILURE not in rendered[:heading]
def test_another_tasks_exact_snapshot_advisory_still_answers_repository_readiness(tmp_path):
"""Attribution is not scoping: the checkout is still covered by B's review."""
from ouroboros.review_evidence import collect_review_evidence
from ouroboros.review_state import (
AdvisoryReviewState, compute_snapshot_hash, make_repo_key, save_state,
)
repo_dir = _repo_with_history(tmp_path)
repo_key = make_repo_key(repo_dir)
state = AdvisoryReviewState()
state.add_run(_advisory_run(snapshot_hash=compute_snapshot_hash(repo_dir),
repo_key=repo_key, task_id="task-b"))
save_state(tmp_path, state)
evidence = collect_review_evidence(tmp_path, task_id="task-a", repo_dir=repo_dir)
assert evidence["current_repo"]["advisory_status"] == "fresh"
assert evidence["current_repo"]["repo_commit_ready"] is True
assert evidence["recent_advisory_runs"] == []
assert [row["task_id"] for row in evidence["foreign_advisory_runs"]] == ["task-b"]
def test_a_run_without_a_task_id_stays_unknown_and_is_never_re_attributed(tmp_path):
"""A legacy row predates the field; it is not evidence that it is mine."""
from ouroboros.review_evidence import collect_review_evidence
from ouroboros.review_state import AdvisoryReviewState, make_repo_key, save_state
repo_dir = _repo_with_history(tmp_path)
state = AdvisoryReviewState()
state.add_run(_advisory_run(snapshot_hash="legacy-snapshot",
repo_key=make_repo_key(repo_dir), task_id=""))
save_state(tmp_path, state)
evidence = collect_review_evidence(tmp_path, task_id="task-a", repo_dir=repo_dir)
assert evidence["foreign_advisory_runs"] == []
assert [row["task_id"] for row in evidence["recent_advisory_runs"]] == [""]
assert evidence["recent_advisory_runs"][0]["snapshot_hash"] == "legacy-snapshot"
def test_an_empty_repo_key_does_not_pull_another_tasks_runs_into_this_task(tmp_path):
"""No repo_dir widens the candidate list to the whole drive; identity still holds."""
from ouroboros.review_evidence import collect_review_evidence
from ouroboros.review_state import AdvisoryReviewState, make_repo_key, save_state
repo_a = _repo_with_history(tmp_path, "repo-a")
repo_b = _repo_with_history(tmp_path, "repo-b")
state = AdvisoryReviewState()
state.add_run(_advisory_run(snapshot_hash="a-snapshot", repo_key=make_repo_key(repo_a),
task_id="task-a"))
state.add_run(_advisory_run(snapshot_hash="b-snapshot", repo_key=make_repo_key(repo_b),
task_id="task-b", failing=_FOREIGN_FAILURE))
save_state(tmp_path, state)
evidence = collect_review_evidence(tmp_path, task_id="task-a", repo_dir=None)
assert evidence["repo_key"] == ""
assert [row["task_id"] for row in evidence["recent_advisory_runs"]] == ["task-a"]
assert [row["task_id"] for row in evidence["foreign_advisory_runs"]] == ["task-b"]
def test_collect_review_evidence_keeps_recent_attempts_task_scoped(tmp_path):
from ouroboros.review_evidence import collect_review_evidence
from ouroboros.review_state import AdvisoryReviewState, CommitAttemptRecord, make_repo_key, save_state

61
tests/test_durable_learning_completeness.py
View file

@ -34,6 +34,67 @@ def test_pattern_register_rewrite_receives_complete_tail(tmp_path, monkeypatch):
assert tail in path.read_text(encoding="utf-8")
def test_pattern_register_receives_the_whole_reflection_and_the_exact_goal(tmp_path, monkeypatch):
"""A correction past character 500 decides the register's row, so it must arrive.
The incident: the Pattern Register writer received ``reflection[:500]``. The
clip landed inside the exculpatory clause of a reflection whose EARLIER
sentence said the opposite, so the register recorded the inverse conclusion
and kept bumping its count. The goal was clipped the same way at 200 chars.
Both are decision inputs of a DESTRUCTIVE rewrite (this call replaces the
whole register), so both arrive complete. Asserted on the messages actually
composed for the Light model, and on the producer's own entry rather than a
hand-built one.
"""
from ouroboros import reflection
(tmp_path / "memory" / "knowledge").mkdir(parents=True)
goal_tail = "GOAL TAIL: the owner asked for the release notes, not a branch cleanup."
goal = "Ship the release. " + ("Background context sentence. " * 30) + goal_tail
early = "EARLY READING: the host should fail closed on REVIEW_REQUIRED."
correction = (
"DECISIVE CORRECTION: this install runs advisory enforcement, so failing closed "
"on REVIEW_REQUIRED would be the inverse of the configured rule."
)
reflection_body = early + " " + ("Padding sentence about the trace. " * 30) + correction
assert len(goal) > 200
assert reflection_body.index(correction) > 500
captured = {}
replies = [{"content": reflection_body
+ "\nMEMORY_ACTIONS_JSON: []\nBACKLOG_CANDIDATES_JSON: []"}]
def fake_chat(*args, **kwargs):
if kwargs.get("call_type") == "pattern_register_update":
captured["prompt"] = kwargs["messages"][0]["content"]
return ({"content": reflection._PATTERNS_HEADER
+ "| advisory misreading | 1 | clipped input | pass whole input | open |\n"}, {})
return (replies.pop(0), {})
monkeypatch.setattr("ouroboros.config.get_light_model", lambda: "light")
monkeypatch.setattr("ouroboros.llm.LLMClient", lambda: object())
monkeypatch.setattr("ouroboros.llm_observability.chat_observed", fake_chat)
entry = reflection.generate_reflection(
{"id": "task-learn", "text": goal, "drive_root": str(tmp_path)},
{"tool_calls": [{"tool": "write_file", "is_error": True, "status": "error",
"tool_result_code": "TOOL_REPORTED_FAILURE", "result": "boom"}]},
"trace", object(), {"rounds": 3, "cost": 0.0},
)
# The bounded display field survives beside the exact one; neither replaces the other.
assert entry["goal_exact"] == goal
assert entry["goal"].startswith("Ship the release.") and goal_tail not in entry["goal"]
assert entry["reflection"] == reflection_body
reflection.append_reflection(tmp_path, entry)
prompt = captured["prompt"]
assert reflection_body in prompt
assert prompt.index(correction) > prompt.index(early)
assert goal_tail in prompt
assert "OMISSION NOTE" not in prompt
def test_backlog_fingerprint_uses_unsanitized_canonical_fields(tmp_path, monkeypatch):
from ouroboros.improvement_backlog import append_backlog_items, load_backlog_items

39
tests/test_knowledge_global_only_topics.py
View file

@ -1,7 +1,9 @@
"""The two reserved shared topics resolve to the global shelf from any room.
"""The reserved shared topics resolve to the global shelf from any room.
`overview` is the orientation every context loads and `improvement-backlog` is
the P7 SSOT. Before this, a project room silently minted
`overview` is the orientation every context loads, `improvement-backlog` is the
P7 SSOT, and `patterns` is the Pattern Register, whose only writer (post-task
reflection) and whose readers (context, deep self-review, the headless copy) all
use the canonical drive. Before this, a project room silently minted
`projects/<id>/knowledge/overview.md`: the write succeeded, nothing read it, and
the resident orientation slot stayed empty. Ordinary topics keep following the
room they were written in.
@ -22,8 +24,31 @@ def project_ctx(tmp_path, project_id: str = "demo") -> ToolContext:
budget_drive_root=str(tmp_path), project_id=project_id, task_id="t1")
def test_reserved_set_is_exactly_the_two_shared_topics():
assert tools.GLOBAL_ONLY_TOPICS == frozenset({tools.BACKLOG_TOPIC, store.OVERVIEW_TOPIC})
def test_reserved_set_is_exactly_the_three_shared_topics():
assert tools.GLOBAL_ONLY_TOPICS == frozenset({
tools.BACKLOG_TOPIC, store.OVERVIEW_TOPIC, tools.PATTERNS_TOPIC})
def test_pattern_register_topic_reaches_the_drive_the_writer_and_readers_use(tmp_path):
"""The Pattern Register has one home, and it is the one everybody else uses.
The register's writer (`reflection._update_patterns`) and its readers
(`context.py`, `deep_self_review`, the headless copy) all address
`memory/knowledge/patterns.md` on the canonical drive. A project room that
could mint `projects/<id>/knowledge/patterns.md` through the tool would be
writing error-class learning into a file none of them ever open.
"""
ctx = project_ctx(tmp_path)
canonical = tmp_path / "memory" / "knowledge" / "patterns.md"
written = tools._knowledge_write(
ctx, topic=tools.PATTERNS_TOPIC,
content="# Pattern Register\n\n| Error class | Count |\n|---|---|\n| x | 1 |\n")
assert "✅" in written
assert canonical.exists()
assert not (tmp_path / "projects" / "demo" / "knowledge" / "patterns.md").exists()
assert "| x | 1 |" in tools._knowledge_read(ctx, tools.PATTERNS_TOPIC)
@pytest.mark.parametrize("topic", sorted(tools.GLOBAL_ONLY_TOPICS))
@ -37,7 +62,7 @@ def test_reserved_topics_resolve_globally_from_a_project_room(tmp_path, topic, s
def test_ordinary_topic_still_follows_the_room(tmp_path):
"""The narrowing is exactly two names — every other topic keeps both shelves."""
"""The narrowing is exactly three names — every other topic keeps both shelves."""
ctx = project_ctx(tmp_path)
room = tools._address(ctx, "deploy-recipes")
@ -125,7 +150,7 @@ def test_write_schema_states_the_rules_the_code_actually_enforces(tmp_path):
schemas = {entry.name: entry.schema for entry in tools.get_tools()}
props = schemas["knowledge_write"]["parameters"]["properties"]
assert "Reserved topics (improvement-backlog, overview) always resolve to global." in props["scope"]["description"]
assert "Reserved topics (improvement-backlog, overview, patterns) always resolve to global." in props["scope"]["description"]
assert "people and relationships" in props["scope"]["description"]
assert "no scope prefixes" in props["topic"]["description"]
assert "stays resident in the index" in schemas["knowledge_write"]["description"]