From 09b95960cd8f60e57c8c5e302cf920d9442b960f Mon Sep 17 00:00:00 2001 From: xuke Date: Thu, 17 Sep 2026 15:38:50 +0800 Subject: [PATCH] =?UTF-8?q?feat(genesis):=20skill=20extraction=20and=20rea?= =?UTF-8?q?d-only=20investigation=20=E2=80=94=20two=20missing=20loops?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SkillExtractor (--extract-skills): At the root episode boundary, distil an accepted contribution into a .agents/skills/ file that the rest of the run inherits. The reference role fires after a PR merges; on this engine a post-merge version is one no later episode of the same run ever sees, so the literal trigger produces a write half that nobody inherits. Running inside the episode puts the skill in the proposal the next episode branches from. Also carries skill bodies in the brief (LocalWorld.skill_bodies), because the reference puts names and relies on skill_read, and this port has no read tools. Without the bodies "read one before using it" is an instruction none of the agents can follow and the loop is decorative. Investigator (--investigate): A manager may mark a delegation read-only (Delegation.readonly). Instead of the recursion the delegation runs an Investigator that reads the node files and records what it finds under ## Findings in that node CONTEXT.md, which enters the version. The fan-out to subagent_investigator is deliberately not ported — that is what a manager with read tools of its own is for, and giving the manager read tools is the session work. Both off by default (skills=None, investigator=None), zero behaviour change. 36 new offline tests. Full suite exit 0. --- CHANGELOG.md | 47 +++ docs/algo-genesis.md | 43 ++- examples/genesis/_delegation.py | 150 ++++++++- examples/genesis/_investigator.py | 187 +++++++++++ examples/genesis/_skills.py | 295 +++++++++++++++++ examples/genesis/_suite.py | 10 +- examples/genesis/_world.py | 35 +- examples/genesis/genesis_recursive_worlds.py | 49 ++- tests/test_genesis_investigator.py | 205 ++++++++++++ tests/test_genesis_skills.py | 316 +++++++++++++++++++ 10 files changed, 1307 insertions(+), 30 deletions(-) create mode 100644 examples/genesis/_investigator.py create mode 100644 examples/genesis/_skills.py create mode 100644 tests/test_genesis_investigator.py create mode 100644 tests/test_genesis_skills.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c20641c..ae12e016 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,53 @@ All notable changes to AgentDescent are documented here. The format follows ### Added +- **`Investigator` — a read-only delegation whose findings persist into the record.** + The Genesis reference implementation has an `:read` investigator role + (`agents/investigator.ex`) that a manager hands a question and that may write only + `CONTEXT.md`. `--investigate` ports it: a manager marks a delegation `"investigate": + true`, the delegation runs `Investigator` (`examples/genesis/_investigator.py`) instead + of the recursion, and what it finds is folded into the node's `CONTEXT.md` under + `## Findings` — so it enters the version and the next agent inherits it. + + It exists because of a blind spot in the port, not to complete a roster of roles: + `_MANAGER_PROMPT` carries the `CONTEXT.md` chain and a *listing* of the files at a + node, never their contents, since this port's agents have no read tool. A manager + deciding from the code is deciding blind, and the investigator is the one actor + allowed to read a node and leave a note about it. + + Deliberately narrow. The role's fan-out to `subagent_investigator` is **not** ported, + because fanning out read-only sub-agents is what a manager with read tools of its own + is for, and that is the port's session work. The one write it may make is the record + at the node it was sent to; source edits and writes to an ancestor's record are + refused and counted. + + 14 tests in `tests/test_genesis_investigator.py`, offline and deterministic. + +- **`SkillExtractor` — a skill is distilled from an accepted contribution, and the run + inherits it.** `LocalWorld.skills()` already collected `.agents/skills/` along the node + chain — `hierarchical_skill_names/2`, and what the paper lists among what an + accepted version carries (§3.1) — the read half and nothing else. + `--extract-skills` adds the write half: `SkillExtractor` (`examples/genesis/_skills.py`) + distils the contribution a root just accepted into a skill file, checks the skills that + already exist so it does not duplicate one, and places it at the node where it is + relevant. + + It runs at the **root episode boundary**, not after the merge. A post-merge version + is one no later episode of the run ever sees, so the loop would still be open; inside + the episode the skill rides in the same proposal and the next agent inherits it. + + The flag also turns on `LocalWorld.skill_bodies`, which puts inherited skill *bodies* + in the brief. Without it the feature is decorative — the brief otherwise carries only + names, and fetching a body needs a `skill_read` tool this port's agents do not have, so + "read one before using it" was an instruction none of them could follow. + + `skill_add`'s frontmatter is rendered here rather than emitted by the model — the + format is the tool's, not the model's. Skills get their own trust-region bucket + (`max_skill_edits=2` beside `max_edits=4`), because a lesson trimmed to make room for + the file it distils is self-defeating. `skills=None` is the old behaviour exactly. + + 22 tests in `tests/test_genesis_skills.py`, offline and deterministic. + - **`evolve(max_tokens=...)`: a budget in the unit that maps to the bill.** `max_calls` and `max_rollouts` count invocations, and a reasoning model can spend 40k tokens on hidden thinking in a single one -- so a 20-round run with diff --git a/docs/algo-genesis.md b/docs/algo-genesis.md index a250655f..db378559 100644 --- a/docs/algo-genesis.md +++ b/docs/algo-genesis.md @@ -965,13 +965,23 @@ so would a reader of this page without this paragraph. propose concurrently against one accepted version and `OctopusConflict` does the three-way merge in the parent. The observable consequence is the same — two children editing one file both survive — and the measurement of that is in the table above. -* **Skills are inherited but never extracted.** `LocalWorld.skills()` collects - `.agents/skills/` along the node chain and puts the *names* in the brief, which - is what `hierarchical_skill_names/2` does upstream and what the paper means by - listing "reusable skills" among what an accepted version carries (§3.1). What - is missing is the other end: upstream's `SkillExtractor` distils a completed - contribution into a new skill, and nothing here does. The domain ships one - human-written skill so the inheritance path is live rather than decorative. +* **Skills are inherited, and now extracted, with `--extract-skills`.** `LocalWorld.skills()` + collects `.agents/skills/` along the node chain, which is what + `hierarchical_skill_names/2` does upstream and what the paper means by listing + "reusable skills" among what an accepted version carries (§3.1). This port had only + that half. The other half is upstream's `SkillExtractor` (`agents/skill_extractor.ex`, + a `:read_write`, `delegation_level :low` role): it distils a completed contribution + into new skills, checks what already exists so it does not duplicate one, places each + at the Context Tree node where it is relevant, and is explicitly allowed to find + nothing. `SkillExtractor` in `examples/genesis/_skills.py` is that role, run at the + **root episode boundary** on the contribution the episode accepted -- upstream fires + it after the PR merges, but on this engine a post-merge version is one no later + episode of the same run sees, so the loop would still not close. Two things are + adapted rather than ported: the model returns the skill's node and body and this + side renders `skill_add`'s frontmatter, and the *bodies* travel in the brief once the + flag is on. The second one is the whole point -- `skill_read` is a tool, this port's + agents have none, and a name line saying "read one before using it" is an instruction + nobody can follow, which is why inheritance was inert before. * **An episode here is one model call; upstream it is a session.** The paper is explicit — "the agent can execute multiple model–tool turns during one supervised episode" — and the runs are configured at **2 048 root turns and 128 turns per @@ -984,10 +994,21 @@ so would a reader of this page without this paragraph. rather than another turn. It is the largest single distance between this port and the system it ports, and it is a property of the harness rather than of the algorithm: `evolve()` proposes with one completion per rollout. -* **Only two roles are ported.** The released code has ten agent modules; the - paper's appendix §1.3 says the model has two — manager and leaf executor — and - that "codebase lead / investigator / task scheduler are implementation labels, - not additional roles". Porting ten would be porting an implementation. +* **Only two roles are ported, plus one capability the paper calls a label.** The + released code has ten agent modules; the paper's appendix §1.3 says the model has two + — manager and leaf executor — and that "codebase lead / investigator / task scheduler + are implementation labels, not additional roles". Porting ten would be porting an + implementation. The one exception is the **investigator**, and it is ported as a + capability rather than a role: `--investigate` lets a manager mark a delegation + read-only, and `Investigator` (`examples/genesis/_investigator.py`) reads that node's + files and records what it finds under `## Findings` in the node's `CONTEXT.md`. The + reason it earns its place is the port's own blind spot — `_MANAGER_PROMPT` carries the + `CONTEXT.md` chain and a listing of files but never their contents, because this + port's agents have no read tool, so the investigator is the one actor that can read a + node's code and leave a note about it for the next agent. It is deliberately **not** + the upstream role in full: there is no fan-out to `subagent_investigator`, because + fanning out read-only sub-agents is what a manager with read tools of its own is for, + and that is the port's session work. ## Run it diff --git a/examples/genesis/_delegation.py b/examples/genesis/_delegation.py index aefcdbb3..87733a4a 100644 --- a/examples/genesis/_delegation.py +++ b/examples/genesis/_delegation.py @@ -42,6 +42,8 @@ from ._octopus import three_way +from ._skills import SkillExtractor + from ._world import (CONTEXT_FILE, KNOWN_ISSUES, EpisodeRecord, LocalWorld, WorldLog, directly_at, looks_like_file, normalise, owns, resolve_edit_path, routing_entry, shadowed_by_module, @@ -52,10 +54,18 @@ @dataclass(frozen=True) class Delegation: - """``(v, p) ⇝ (v, q)``: where a child is situated, and what it is for.""" + """``(v, p) ⇝ (v, q)``: where a child is situated, and what it is for. + + ``readonly`` marks the delegation as an **investigation** rather than work: + upstream's ``subagent_investigator`` is handed a question and answers it, and the + only thing it may write is a ``CONTEXT.md``. A manager asks for one by putting + ``investigate: true`` on a delegation -- see ``llm_manager`` -- and the answer is + persisted at the node so a later agent inherits it. + """ path: str objective: str + readonly: bool = False @dataclass(frozen=True) @@ -79,7 +89,7 @@ class Edit: owner: str path: str content: Optional[str] - kind: str = "work" # "record" | "work" | "context" + kind: str = "work" # "record" | "work" | "context" | "skill" @dataclass(frozen=True) @@ -182,6 +192,21 @@ class RecursiveDelegation: #: history at all -- not the same thing as the acceptance gate, which sees #: only what the whole episode returned. ``None`` runs the scope check alone. review: Optional[Review] = None + #: Upstream's ``SkillExtractor``, run at the root episode boundary on the + #: contribution the episode just accepted -- see :mod:`examples.genesis._skills`. + #: ``None`` is the old behaviour: skills are inherited and never written. + skills: Optional[SkillExtractor] = None + #: How many skill files may ride in one proposal *on top of* :attr:`max_edits`. + #: They are a separate bucket because they are not competing with the work: the + #: extractor exists so a later agent inherits what this one learned, and a skill + #: trimmed to make room for the work it distils is self-defeating. The caller + #: must size ``SpatialContract.max_files_per_diff`` to cover both, or the whole + #: diff is dropped rather than truncated. + max_skill_edits: int = 2 + #: Upstream's ``Investigator``: a read-only delegation a manager asked for, + #: answered at that node and persisted into its ``CONTEXT.md``. ``None`` means + #: every delegation is work. See :mod:`examples.genesis._investigator`. + investigator: Optional[Callable[["Brief"], Sequence[Edit]]] = None #: The version the most recent rollout branched from -- the latest accepted state, #: as the engine handed it over. Read by `CompletionJudge`, which has to look at #: the codebase to answer upstream's question and is not given the reward. @@ -221,6 +246,14 @@ class RecursiveDelegation: requests_raised: int = 0 adopted_requests: int = 0 unmet_requests: int = 0 + #: Skill files a root episode distilled from its own contribution, and the edits + #: dropped from the return because there was no room -- see :meth:`_bound`. + skills_written: int = 0 + #: Read-only investigations run, the finding records they left, and writes an + #: investigator offered that were not a ``CONTEXT.md`` at its own node. + investigations: int = 0 + finding_edits: int = 0 + investigation_refused: int = 0 # -- the ProposalPolicy protocol --------------------------------------- @@ -228,7 +261,11 @@ def propose(self, ctx) -> Sequence[str]: state = _state_of(ctx.rendered) self.last_state = dict(state) root = LocalWorld(version=int(ctx.base_version or 0), path=self.root_path, - readonly=tuple(self.readonly or self.contracts)) + readonly=tuple(self.readonly or self.contracts), + # Bodies travel exactly when there is something writing + # them: with the extractor off, a name the agent cannot open + # is what upstream sends, and it costs a run nothing. + skill_bodies=self.skills is not None) # The workspace of a transient agent tree: created for this rollout, gone # with it. `None` when worktrees are off, and every use of it is guarded. rollout = self.rollout_factory() if self.rollout_factory else None @@ -250,9 +287,19 @@ def _rollout(self, ctx, root: LocalWorld, state: Mapping[str, str], rollout=rollout) rationale = f"rework at {pending[0] or './'}" else: - edits, unmet, _ = self._episode(root, self._objective(ctx), state, ctx, + objective = self._objective(ctx) + edits, unmet, _ = self._episode(root, objective, state, ctx, depth=0, rollout=rollout) rationale = f"episode at {root.path or './'}" + # Upstream's SkillExtractor, at the root episode boundary: the + # contribution this episode accepted is the "completed PR" it distils. + # It runs after the episode returns rather than inside it because the + # upstream extractor is a **separate agent** -- its writes are its own, + # not the manager's -- and because here that also covers a root that had + # no children and did the work itself. It is not run for a rework + # rollout, which is the same contribution being retried, not a new one. + if self.skills is not None: + edits = self._distil(root, objective, state, edits, ctx, 0) # A need that reached the top of this episode's chain and still fell # outside its root's authority. Counted rather than dropped quietly: it # means the decomposition put work where nobody could do it. @@ -353,6 +400,20 @@ def _episode(self, world: LocalWorld, objective: str, state: Mapping[str, str], findings: List[Tuple[str, str, str]] = [] for delegation in delegations: child = world.delegate(delegation.path) + if delegation.readonly and self.investigator is not None: + # Upstream's `subagent_investigator`: handed a question, read-only + # over the code, its one write a `CONTEXT.md`. Nothing to judge and + # nothing to rework -- the record it leaves is the deliverable. + records = self._investigate(child, delegation.objective, state, ctx, + depth + 1) + if records: + self._fold(held, owner_of, records, child, state) + self.finding_edits += len(records) + children.append(self.log.record(EpisodeRecord( + agent_id=self.log.next_id("investigator"), role="investigator", + path=child.path, depth=depth + 1, version=child.version, + objective=delegation.objective, n_edits=len(records)))) + continue returned, asked, child_record = self._episode( child, delegation.objective, state, ctx, depth=depth + 1, rollout=rollout, base=child_base) @@ -432,6 +493,62 @@ def _episode(self, world: LocalWorld, objective: str, state: Mapping[str, str], child_base, kept) or record.commit return merged, pending, record + def _distil(self, world: LocalWorld, objective: str, state: Mapping[str, str], + merged: Sequence[Edit], ctx, depth: int) -> List[Edit]: + """Upstream's ``SkillExtractor`` on this episode's accepted contribution. + + Only the **work** is offered for distillation. A routing entry or a refusal + note is bookkeeping *about* the contribution, not the contribution; upstream + extracts from the PR diff, and the diff is the work. + + The extractor is handed a brief over the **post-merge** tree, so a skill about + what was just written can read what was just written. Its own failure is + swallowed: a dead model call must cost a skill, never the contribution the + episode already produced. + """ + contributed = [e for e in merged if e.kind == "work" and e.content is not None] + if not contributed: + return list(merged) + post = apply_edits(state, merged) + brief = Brief(world=world, objective=objective, + context=world.situate(post, contracts=self.contracts), + state=post, task=ctx.task, output=ctx.output, + reward=ctx.reward, depth=depth) + try: + written = list(self.skills(brief, contributed) or ()) + except Exception: # noqa: BLE001 - extraction must not cost the contribution + return list(merged) + self.skills_written += len(written) + return list(merged) + written + + def _investigate(self, child: LocalWorld, objective: str, + state: Mapping[str, str], ctx, depth: int) -> List[Edit]: + """One read-only investigation at ``child``; one record may come back. + + The investigator is shown the node's own files because this port's agents + have no read tool -- that is the whole reason the role is worth porting here. + A returned edit that is not a ``CONTEXT.md`` at the investigated node is + refused and counted: source edits are not an investigation, and a write to an + ancestor's record is the ancestor's own business. + """ + brief = Brief(world=child, objective=objective, + context=child.situate(state, contracts=self.contracts), + state=state, task=ctx.task, output=ctx.output, + reward=ctx.reward, depth=depth) + try: + offered = list(self.investigator(brief) or ()) + except Exception: # noqa: BLE001 - a dead investigator is not a failed run + return [] + self.investigations += 1 + key = f"{child.path}/{CONTEXT_FILE}" if child.path else CONTEXT_FILE + out: List[Edit] = [] + for edit in offered: + if normalise(edit.path) == normalise(key): + out.append(replace(edit, owner=child.path)) + else: + self.investigation_refused += 1 + return out + def _accountability_pass(self, world: LocalWorld, objective: str, state: Mapping[str, str], held: Mapping[str, Edit], ctx, depth: int, @@ -664,17 +781,26 @@ def _bound(self, edits: Sequence[Edit]) -> List[Edit]: it loses a source file -- and a record that brings a node into existence before either, because nothing else in the accepted version says the node exists. See :class:`Edit`. + + Skills are a bucket of their own (:attr:`max_skill_edits`), because they are + not bookkeeping for this work and they are not the work: they are what a + later agent inherits. Letting them compete with the contribution they were + distilled from would let a trimmed episode keep the source file and throw + away the lesson, which is the one ordering this mechanism cannot have. """ by_path: Dict[str, Edit] = {} for edit in edits: by_path[edit.path] = edit + skills = [by_path[p] for p in sorted(by_path) + if by_path[p].kind == "skill"][:self.max_skill_edits] + rest = {p: e for p, e in by_path.items() if e.kind != "skill"} order = {"record": 0, "work": 1} - ordered = [by_path[p] for p in - sorted(by_path, key=lambda p: (order.get(by_path[p].kind, 2), p))] + ordered = [rest[p] for p in + sorted(rest, key=lambda p: (order.get(rest[p].kind, 2), p))] if len(ordered) > self.max_edits: self.truncated += len(ordered) - self.max_edits ordered = ordered[:self.max_edits] - return ordered + return ordered + skills def _objective(self, ctx) -> str: return (f"validation case {getattr(ctx.task, 'id', '?')} scored " @@ -693,7 +819,15 @@ def stats(self) -> str: f"node_relative_paths={self.resolved_relative} " f"accountability={self.accountability_edits}/" f"{self.accountability_declined} " - f"findings_carried={self.accountability_findings}") + f"findings_carried={self.accountability_findings}" + + ("" if self.skills is None else + f" skills_written={self.skills_written} " + f"skill_extraction=[{self.skills.stats()}]") + + ("" if self.investigator is None else + f" investigations={self.investigations} " + f"finding_edits={self.finding_edits}/" + f"{self.investigation_refused} refused " + f"[{self.investigator.stats()}]")) def _with_findings(objective: str, path: str, diff --git a/examples/genesis/_investigator.py b/examples/genesis/_investigator.py new file mode 100644 index 00000000..942f19db --- /dev/null +++ b/examples/genesis/_investigator.py @@ -0,0 +1,187 @@ +"""The investigator: a read-only delegation whose findings persist into the record. + +A manager hands it a question and it answers from the node's code; the answer is written +into the node's ``CONTEXT.md`` so the next agent inherits it instead of re-discovering +it. Ports the ``investigator`` role (``agents/investigator.ex``), whose one permitted +write is a ``CONTEXT.md``. + +The gap it fills is concrete. ``_MANAGER_PROMPT`` carries the ``CONTEXT.md`` chain and a +listing of the files at the node; it does **not** carry the files' contents, because this +port's agents have no read tool. So a manager that must decide *from the code* is +deciding blind, and the investigator is the one actor allowed to read a node's code and +leave a note about it. + +The role's fan-out to ``subagent_investigator`` is deliberately not ported -- a manager +with read tools of its own is what that is for, and giving the manager read tools is the +port's session work. What is left is the part nothing else here does. + +The findings go under a ``## Findings`` heading of the node's ``CONTEXT.md``, through the +same :func:`~examples.genesis._world.under_heading` the rest of the port uses: findings +belong in a named section, because the next agent reads the section, not the whole file. +""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING, List, Mapping, Optional, Sequence + +from agentdescent.filetree import match_any + +from ._world import CONTEXT_FILE, child_paths, directly_at, under_heading + +if TYPE_CHECKING: # `_delegation` imports this module + from ._delegation import Brief, Edit + +__all__ = ["FINDINGS_HEADING", "INVESTIGATOR_PROMPT", "Investigator"] + +#: The section a finding is recorded under. ``KNOWN_ISSUES`` is for problems, this is +#: for knowledge, and a finding can be either. +FINDINGS_HEADING = "## Findings" + + +INVESTIGATOR_PROMPT = """You are a read-only investigator in a recursive software \ +world, situated at the repository path `{path}`. You answer one question about what is \ +here. You do not change code and you do not design anything. + +{context} + +The question you were sent to answer: + +{objective} + +The files at this node, in full: + +{sources} + +Answer only from what is in front of you, and say so when the answer is "nothing here" \ +rather than guessing. Then record what a future agent would otherwise have to \ +re-discover: a dependency between two files, a name that resolves somewhere unexpected, \ +a gotcha, a test gap, an external interface's real shape, a fact that contradicts the \ +record above. One markdown list item each, each one sentence. + +Every finding you do not record is a finding the next agent will have to re-discover. \ +If nothing here is worth persisting, record nothing -- an empty record is the honest \ +answer, not a failure. + +Reply with ONE JSON object and nothing else: +{{"findings": "", + "answer": ""}}""" + + +class Investigator: + """``(brief) -> [Edit]`` -- one read-only investigation of one node. + + Installs on :class:`~examples.genesis._delegation.RecursiveDelegation` as + ``investigator=``, and runs for a delegation the manager marked read-only (see + ``llm_manager``). ``None`` there is the old behaviour: every delegation is work. + + The one write it may make is a ``CONTEXT.md`` **at the node it was sent to**. A + delegation that returned source edits would not be an investigation, and a write to + an ancestor's record is the ancestor's own business -- both are refused and counted + rather than silently applied. + """ + + def __init__(self, complete, *, contracts: Sequence[str] = (), + frozen: Sequence[str] = (), per_file_chars: int = 4_000, + max_chars: int = 12_000, max_files: int = 12): + self._complete = complete + self._contracts = tuple(contracts) + self._frozen = tuple(frozen) + self._per_file_chars = per_file_chars + self._max_chars = max_chars + self._max_files = max_files + #: Calls made, findings recorded, the two ways one was refused, and the two + #: "nothing happened" cases that must not look alike. + self.calls = 0 + self.recorded = 0 + self.empty = 0 + self.unparsed = 0 + self.refused = 0 + + def __call__(self, brief: "Brief") -> List["Edit"]: + from ._delegation import Edit # circular at import time, not at call time + + node = brief.world.path + sources = _sources(brief.state, node, self._per_file_chars, + self._max_chars, self._max_files) + if sources is None: + # Nothing at the node to read: there is nothing to investigate, and a + # call spent on an empty directory is a call spent on nothing. + self.empty += 1 + return [] + self.calls += 1 + try: + reply = self._complete(INVESTIGATOR_PROMPT.format( + path=node or "./", context=brief.context, objective=brief.objective, + sources=sources)) or "" + except Exception: # noqa: BLE001 - a dead call costs a finding, not the run + self.unparsed += 1 + return [] + findings = _findings_of(reply) + if findings is None: + self.unparsed += 1 + return [] + if not findings.strip(): + self.empty += 1 + return [] + + key = f"{node}/{CONTEXT_FILE}" if node else CONTEXT_FILE + if self._frozen and match_any(key, self._frozen): + self.refused += 1 + return [] + body = under_heading(brief.state.get(key, f"# {node or './'}\n"), + FINDINGS_HEADING, + "\n".join(f"- {line.strip().lstrip('- ').strip()}" + for line in findings.splitlines() + if line.strip())) + self.recorded += 1 + # `context` when the record was already there and this adds to it, `record` + # when it is the write that brings the node's record into existence -- the + # same distinction the rest of the port makes, for the same reason. + return [Edit(owner=node, path=key, content=body, + kind="context" if key in brief.state else "record")] + + def stats(self) -> str: + return (f"investigations={self.calls} findings={self.recorded} " + f"empty={self.empty} refused={self.refused} " + f"unparsed={self.unparsed}") + + +def _sources(state: Mapping[str, str], node: str, per_file: int, budget: int, + max_files: int) -> Optional[str]: + """The node's own files, in full; ``None`` when the node owns none. + + Its own files, not its subtree's: an investigator's scope is the node it stands on, + and a deeper level is what sending it deeper is for. Children are listed by name so + the reader knows the tree continues. + """ + here = sorted(k for k in state if directly_at(node, k)) + if not here: + return None + out: List[str] = [] + for key in here[:max_files]: + body = state[key] + if per_file and len(body) > per_file: + body = body[:per_file] + "\n... [truncated] ..." + out.append(f"# {key}\n```\n{body}\n```") + children = child_paths(node, list(state)) + if children: + out.append("subdirectories in the repository here: " + + ", ".join(f"{c}/" for c in children)) + text = "\n\n".join(out) + return text if len(text) <= budget else text[:budget] + "\n... [truncated] ..." + + +def _findings_of(reply: str) -> Optional[str]: + """``findings`` from a reply, or ``None`` when the reply is not a JSON object.""" + start = reply.find("{") + if start < 0: + return None + try: + data = json.loads(reply[start:reply.rfind("}") + 1]) + except Exception: # noqa: BLE001 - malformed model output, not a bug + return None + if not isinstance(data, Mapping) or "findings" not in data: + return None + findings = data["findings"] + return findings if isinstance(findings, str) else None diff --git a/examples/genesis/_skills.py b/examples/genesis/_skills.py new file mode 100644 index 00000000..df48b982 --- /dev/null +++ b/examples/genesis/_skills.py @@ -0,0 +1,295 @@ +"""The write half of the skill loop: distilling an accepted contribution into a skill. + +``LocalWorld.skills()`` already collected ``.agents/skills/`` along the node chain and +put the *names* in the brief -- the read half, and nothing else: a skill a person wrote, +inherited, never written back. This module is the other half. It ports the +``skill_extractor`` role (``agents/skill_extractor.ex``): it checks the skills that +already exist so it does not duplicate one, places each at the node where it is +relevant, and is allowed to find nothing. + +**Placement is the one difference that matters, and it is deliberate.** The extractor +runs *inside the episode that produced the contribution*, so the skill it writes is part +of the same proposal and the next agent in the run inherits it. The reference role runs +after the PR merges; on this engine that is a version no later episode of the same run +ever sees, so a literal trigger would produce the write half and still leave the loop +open. The root episode boundary is the cheapest honest reading of "extract from a +completed contribution". + +**The tools are not ported, and the node field is why they do not have to be.** +``skill_add``/``skill_edit`` write a file, ``skill_enable`` places it at the Context Tree +node "where it's most relevant", and ``skill_read`` fetches a body on demand. The +extractor here returns the file contents and the node, the spatial contract enforces +that the node is one the root may write, and the brief carries the bodies because the +port's agents have no read tool to fetch them with -- see :meth:`LocalWorld.situate`. + +The frontmatter is rendered here rather than by the model: the format is the tool's +(``name``, ``description``, ``parameters``), and a model that has to emit YAML is a +model that can emit YAML that will not parse. +""" + +from __future__ import annotations + +import json +import re +from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Sequence, Tuple + +from agentdescent.filetree import match_any + +from ._world import SKILLS_DIR, normalise + +if TYPE_CHECKING: # `_delegation` imports this module + from ._delegation import Brief, Edit + +__all__ = ["SKILL_PROMPT", "SkillExtractor", "is_skill_path", "skill_name", "skill_path"] + +#: Skill names are kebab-case and become a path component, so the rule is enforced +#: rather than trusted to the model. +_NAME_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") + + +SKILL_PROMPT = """You are the skill-extraction agent of a persistent software world. \ +A contribution was just accepted; your job is to distil the part of it that a later \ +agent would otherwise have to re-discover into a **skill** -- a markdown file under \ +`.agents/skills/` that a future agent working here will be handed. + +{context} + +The contribution was made against this objective: + +{objective} + +These are the files it wrote, in full: + +{work} + +The skills that already exist in this world: + +{existing} + +Answer only from what is in front of you. A skill earns its place only if it is \ +**complex, important and reusable** -- a procedure, a convention, a gotcha, an \ +invariant that a later agent working here would get wrong without being told. Do \ +**not** write one for: + +- anything a competent developer would already know; +- a one-off fix with no reusable pattern; +- something the files above already state plainly; +- a duplicate of an existing skill -- do not rewrite it, skip it. + +It is a correct and expected answer to find nothing worth extracting. + +For each skill, choose the **node** -- the directory a later agent would be standing \ +in when the advice is relevant; `""` is the repository root -- and give it a \ +`kebab-case` name, a one-line `description`, and the `body` in markdown. List inputs \ +under `parameters` only if the skill really takes any. + +Reply with ONE JSON object and nothing else: +{{"skills": [{{"node": "", + "name": "", + "description": "", + "body": "", + "parameters": [{{"name": "", "type": "", + "description": "", + "required": true}}]}}], + "reason": ""}}""" + + +def skill_path(node: str, name: str) -> str: + """``/.agents/skills/.md``, at the root when ``node`` is empty.""" + node = normalise(node) + return f"{node}/{SKILLS_DIR}/{name}.md" if node else f"{SKILLS_DIR}/{name}.md" + + +def is_skill_path(path: str) -> bool: + """Is this a skill file rather than work? The same test ``_suite.cold_start`` uses.""" + return f"/{SKILLS_DIR}/" in f"/{normalise(path)}" + + +def skill_name(path: str) -> Optional[str]: + """The skill's name -- its basename without ``.md`` -- or ``None``.""" + if not is_skill_path(path): + return None + tail = path.rsplit("/", 1)[-1] + return tail[:-3] if tail.endswith(".md") else tail + + +class SkillExtractor: + """``(parent, work) -> [Edit]`` -- skills distilled from an accepted contribution. + + Installs on :class:`~examples.genesis._delegation.RecursiveDelegation` as + ``skills=``. ``None`` (the default there) is the old behaviour: the world inherits + skills and never writes one. + + The counters are the point of the counters: a run whose extractor was never called, + a run that was called and always found nothing, and a run that tried to write a + duplicate look identical in the artifact and need opposite fixes. + """ + + def __init__(self, complete, *, contracts: Sequence[str] = (), + frozen: Sequence[str] = (), max_skills: int = 2, + max_chars: int = 8_000, per_file_chars: int = 4_000, + max_work_files: int = 8): + self._complete = complete + self._contracts = tuple(contracts) + #: Globs a skill may never be written under -- the frozen specification is a + #: contract, not a place the world is allowed to grow a skill into. + self._frozen = tuple(frozen) + self._max_skills = max_skills + self._max_chars = max_chars + self._per_file_chars = per_file_chars + self._max_work_files = max_work_files + #: Model calls made, skills written, and the three ways one was refused. + self.calls = 0 + self.extracted = 0 + self.deduped = 0 + self.refused = 0 + self.unparsed = 0 + + def __call__(self, parent: "Brief", work: Sequence["Edit"]) -> List["Edit"]: + from ._delegation import Edit # circular at import time, not at call time + + contributed = [e for e in work if e.kind == "work" and e.content] + if not contributed: + # Nothing but bookkeeping came back: there is no contribution to learn + # from, and a call spent on the routing table would be a call spent on + # nothing. Not counted -- this is the common case, not a failure. + return [] + existing = [k for k in parent.state if is_skill_path(k)] + self.calls += 1 + prompt = SKILL_PROMPT.format( + context=parent.context, objective=parent.objective, + work=_render(contributed[:self._max_work_files], self._per_file_chars, + self._max_chars), + existing="\n".join(f" {k}" for k in sorted(existing)) or " (none yet)") + try: + reply = self._complete(prompt) or "" + except Exception: # noqa: BLE001 - a dead call must not cost the contribution + self.unparsed += 1 + return [] + items, usable = _parse(reply) + if not usable: + self.unparsed += 1 + return [] + + dirs = _directories(parent.state) + taken = {n for n in (skill_name(k) for k in existing) if n} + out: List["Edit"] = [] + for item in items: + if len(out) >= self._max_skills: + break + node = normalise(str(item.get("node", ""))) + name = str(item.get("name", "")).strip() + body = str(item.get("body", "")).strip() + path = skill_path(node, name) + if not name or not body or not _NAME_RE.match(name) or len(name) > 64: + self.refused += 1 + continue + if not _is_node(node, parent.state, dirs): + self.refused += 1 + continue + if self._frozen and match_any(path, self._frozen): + self.refused += 1 + continue + if path in parent.state or name in taken: + # The extractor is required to look before it writes: a skill that + # already exists is not improved by a second copy of it. + self.deduped += 1 + continue + taken.add(name) + out.append(Edit( + owner=parent.world.path, path=path, kind="skill", + content=skill_file(name, str(item.get("description", "")), + item.get("parameters"), body))) + self.extracted += len(out) + return out + + def stats(self) -> str: + return (f"skills={self.extracted} written/{self.deduped} dupe/" + f"{self.refused} refused/{self.unparsed} unparsed " + f"over {self.calls} calls") + + +def skill_file(name: str, description: str, parameters: Any, body: str) -> str: + """One skill file: the frontmatter, then the body. + + Rendered rather than model-emitted for the reason the module docstring gives: the + format is the tool's, not the model's. + """ + description = " ".join(str(description or "").split())[:160] or name + lines = ["---", f"name: {name}", + f'description: "{description.replace(chr(34), chr(39))}"'] + params = parameters if isinstance(parameters, list) else [] + if params: + lines.append("parameters:") + for p in params: + if not isinstance(p, Mapping): + continue + lines.append(f" - name: {str(p.get('name', '')).strip()}") + lines.append(f" type: {str(p.get('type', 'string')).strip() or 'string'}") + lines.append(" description: \"" + + " ".join(str(p.get("description", "")).split())[:160] + .replace(chr(34), chr(39)) + "\"") + lines.append(" required: " + + str(bool(p.get("required", False))).lower()) + lines.append("---") + return "\n".join(lines) + "\n\n" + body.rstrip() + "\n" + + +def _render(work: Sequence["Edit"], per_file: int, budget: int) -> str: + """Whole files, clipped per file, then trimmed from the end within ``budget``.""" + out: List[str] = [] + for edit in work: + body = edit.content or "" + if per_file and len(body) > per_file: + body = body[:per_file] + "\n... [truncated] ..." + out.append(f"# {edit.path}\n```\n{body}\n```") + text = "\n\n".join(out) + return text if len(text) <= budget else text[:budget] + "\n... [truncated] ..." + + +def _directories(state: Mapping[str, str]) -> set: + """Every directory the state implies, root included.""" + out = {""} + for key in state: + parts = normalise(key).split("/")[:-1] + acc = "" + for part in parts: + acc = f"{acc}/{part}" if acc else part + out.add(acc) + return out + + +def _is_node(node: str, state: Mapping[str, str], dirs: Mapping[str, Any]) -> bool: + """Is ``node`` a directory this world has, and not a file of the same name?""" + if node == "": + return True + if node in state: # a file, not a node + return False + return node in dirs + + +def _parse(reply: str) -> Tuple[List[Dict[str, Any]], bool]: + """``(items, usable)``. ``usable`` is False when the reply is not a JSON object. + + A reply that parses but carries an empty ``skills`` list is **usable**: finding + nothing is an explicitly valid answer, and counting it as a parse failure would make + "the model looked and found nothing" indistinguishable from "the model broke", which + are opposite problems. + """ + start = reply.find("{") + if start < 0: + return [], False + try: + data = json.loads(reply[start:reply.rfind("}") + 1]) + except Exception: # noqa: BLE001 - malformed model output, not a bug + return [], False + if not isinstance(data, Mapping): + return [], False + if "skills" not in data or data["skills"] is None: + # The key absent is the model saying "nothing" without spelling it out; the + # key present but the wrong type is a broken protocol. Different problems. + return [], True + items = data["skills"] + if not isinstance(items, list): + return [], False + return [i for i in items if isinstance(i, Mapping)], True diff --git a/examples/genesis/_suite.py b/examples/genesis/_suite.py index a7840ffe..d5f431ea 100644 --- a/examples/genesis/_suite.py +++ b/examples/genesis/_suite.py @@ -290,12 +290,17 @@ def review(parent, returned): your own subtree, or to handle this yourself. Reply with ONE JSON object and nothing else: -{{"delegations": [{{"path": "", "objective": ""}}]}} +{{"delegations": [{{"path": "", "objective": "", + "investigate": false}}]}} Rules: - You are ACCOUNTABLE for all code under `{path}`, and delegating does not \ discharge that. The files AT `{path}` itself are nobody else's to write: after \ your children return you get one more turn to write them. +- Set `"investigate": true` when you need **to know something about a child subtree \ +before deciding**, rather than work done in it. A read-only investigator reads the \ +files there and records what it finds in that node's record; it writes no code. Use \ +one delegation per question. - A node is a **directory**, never a file. `src/frontend` is a node; \ `src/frontend/lexer.py` is a file that belongs to the agent situated at \ `src/frontend`, and delegating to it is refused. @@ -375,7 +380,8 @@ def manager(brief: Brief) -> Sequence[Delegation]: for item in items: if isinstance(item, dict) and item.get("path"): out.append(Delegation(normalise(str(item["path"])), - str(item.get("objective", brief.objective)))) + str(item.get("objective", brief.objective)), + readonly=bool(item.get("investigate")))) return out return manager diff --git a/examples/genesis/_world.py b/examples/genesis/_world.py index 75fe5bc1..6dfe15f1 100644 --- a/examples/genesis/_world.py +++ b/examples/genesis/_world.py @@ -344,6 +344,12 @@ class LocalWorld: #: the brief, and :meth:`routing` must not send a manager into a directory #: where nothing is writable. readonly: Sequence[str] = () + #: Hand the agent the inherited skills' **bodies**, not just their names. + #: Upstream the names go in the brief and a ``skill_read`` tool fetches a body + #: on demand; an agent here has no tool, so the name line is an instruction it + #: cannot follow and inheritance is real only when the body travels too. Off by + #: default, because a run that never writes a skill should not pay to carry one. + skill_bodies: bool = False def __post_init__(self) -> None: object.__setattr__(self, "path", normalise(self.path)) @@ -360,7 +366,8 @@ def delegate(self, q: str) -> "LocalWorld": raise ValueError( f"delegation must stay inside the parent's subtree: " f"{self.path or './'} cannot situate a child at {q or './'}") - return LocalWorld(version=self.version, path=q, readonly=self.readonly) + return LocalWorld(version=self.version, path=q, readonly=self.readonly, + skill_bodies=self.skill_bodies) def routing(self, state: Mapping[str, str]) -> List[str]: """Where this node says work may be delegated to. @@ -397,10 +404,16 @@ def _all_readonly(self, state: Mapping[str, str], child: str) -> bool: def skills(self, state: Mapping[str, str]) -> List[str]: """Skill files this node inherits, nearest ancestor last. - Names only reach the brief, not bodies: upstream hands an agent the skill - *names* available at its node and lets it read the ones it wants, because - pasting every ancestor's skills into every episode is how a context - window is spent on things nobody asked for. + Names only reach the brief by default, not bodies: upstream hands an agent + the skill *names* available at its node and lets it read the ones it wants + with ``skill_read``, because pasting every ancestor's skills into every + episode is how a context window is spent on things nobody asked for. + + That reasoning holds only where the agent *can* read one, and this port's + agents have no tools. So ``skill_bodies`` exists: with it set, the bodies + travel with the brief and inheritance stops being a list of names the agent + cannot open. It is off by default so that a run which writes no skill pays + nothing for the mechanism. """ out: List[str] = [] for node in self._chain(): @@ -483,8 +496,16 @@ def clip(body: str) -> str: parts: List[str] = [] skills = self.skills(state) if skills: - parts.append("--- skills available here (read one before using it) ---\n" - + "\n".join(f" {k}" for k in skills)) + if self.skill_bodies: + # A number of skills, not a list of names: no read tool exists here, + # so "read one before using it" is an instruction nobody can follow. + parts.append( + "--- skills inherited at this node (reusable knowledge -- follow " + "it where it applies) ---\n" + + "\n\n".join(f"# {k}\n{clip(state[k])}" for k in skills)) + else: + parts.append("--- skills available here (read one before using it) ---\n" + + "\n".join(f" {k}" for k in skills)) # Split "mine" from "my children's". An agent is bad at noticing an # absence inside a long list, and the absence is the actionable part: # a node that owns no file yet says so on its own line. diff --git a/examples/genesis/genesis_recursive_worlds.py b/examples/genesis/genesis_recursive_worlds.py index deb9c65c..25b1fbff 100644 --- a/examples/genesis/genesis_recursive_worlds.py +++ b/examples/genesis/genesis_recursive_worlds.py @@ -64,8 +64,14 @@ surface, known issues -- and the read-only roles exist to do nothing else. Here the two writes an agent makes on its own are the routing entry for a node it opened and the refusal note for a child it rejected; an LLM executor may write - more, and nothing requires it to. Skills (``.agents/skills/``) are not loaded - at all. + more, and nothing requires it to. Skills (``.agents/skills/``) are inherited by + name everywhere, and ``--extract-skills`` adds the other half -- an extractor + that distils an accepted contribution into one -- which also makes the brief + carry their *bodies*, because this port's agents have no read tool to fetch a + body with. ``--investigate`` adds the investigator for the same missing tool: a + manager may mark a delegation read-only, and what that investigator reads at the + node is recorded in that node's ``CONTEXT.md`` rather than returned only to a + parent that is one completion and cannot act on it until the next round. * **Multi-repository work, the desktop shell and the dashboard are out of scope.** Upstream's task-level accept is a human action on that dashboard (``EvoGit.Review.merge_branch/2,3``); here the task-level gate is the @@ -96,7 +102,9 @@ from ._claude_code import ClaudeCodeExecutor, claude_code_available from ._extract import ExtractPhase from ._judge import ParentJudge +from ._investigator import Investigator from ._review import CompletionJudge, ParentCodeReview, chain_reviews +from ._skills import SkillExtractor from ._suite import TEST_FAILURE from ._suite import cold_start, preflight from ._octopus import OctopusConflict, git_available @@ -281,6 +289,20 @@ def build_parser() -> argparse.ArgumentParser: help="skip the parent reading its child's diff (one model " "call per returned child). The tests alone cannot see " "a function that computes nothing") + parser.add_argument("--extract-skills", action="store_true", + help=("port upstream's SkillExtractor: when the root accepts " + "a contribution, distil it into a `.agents/skills/` file " + "the rest of the run inherits. Needs --model. The skill " + "bodies then travel in the brief, because this port's " + "agents have no read tool to fetch them with -- which is " + "why inheritance was inert before this flag existed")) + parser.add_argument("--investigate", action="store_true", + help=("port upstream's Investigator: a manager may mark a " + "delegation read-only, and a read-only investigator reads " + "that node's files and records what it finds under " + "`## Findings` in the node's CONTEXT.md. Needs --model. " + "No fan-out to sub-investigators -- that is the manager's " + "own read tools, which are the session work")) parser.add_argument("--no-parent-tests", action="store_true", help=("stop the parent running the suite on a child's " "work before accepting it; leaves only the scope " @@ -479,6 +501,22 @@ def main(argv=None) -> None: # that cost. Both now, tests first because they are free. code_review = (None if args.no_parent_review or complete is None else ParentCodeReview(complete, contracts=spec.CONTRACTS)) + if args.extract_skills and complete is None: + print("--extract-skills needs --model: distilling a contribution into reusable " + "knowledge is a judgement, and a rule-based one would be writing down " + "what it was handed") + return + skill_extractor = (None if not args.extract_skills else + SkillExtractor(complete, contracts=spec.CONTRACTS, + frozen=spec.FROZEN)) + if args.investigate and complete is None: + print("--investigate needs --model: reading code and saying what matters in it " + "is a judgement, and a rule-based reader would be describing what it was " + "handed") + return + investigator = (None if not args.investigate else + Investigator(complete, contracts=spec.CONTRACTS, + frozen=spec.FROZEN)) ledger = WorktreeLedger() if args.worktrees else None if ledger is not None and not git_worktrees_available(): print("Worktrees: git worktree is unavailable here -- running without it") @@ -501,6 +539,8 @@ def main(argv=None) -> None: readonly=spec.FROZEN, review=chain_reviews(None if args.no_parent_tests else spec.suite_review(tasks), code_review), + skills=skill_extractor, + investigator=investigator, accountability=not args.no_accountability) judge = ParentJudge(log=log, enabled=not args.engine_gate) octopus = None if args.keyed_union else OctopusConflict() @@ -614,6 +654,11 @@ def _superseded(rendered, task, output, reward_): f"context_updates={delegation.record_updates}") if ledger is not None: print(f"workspace : {ledger.summary()}") + if skill_extractor is not None: + print(f"skills : written={delegation.skills_written} " + f"{skill_extractor.stats()}") + if investigator is not None: + print(f"investigation : {investigator.stats()}") if sessions is not None: print(f"claude code : {sessions.summary()}") if code_review is not None: diff --git a/tests/test_genesis_investigator.py b/tests/test_genesis_investigator.py new file mode 100644 index 00000000..6e253370 --- /dev/null +++ b/tests/test_genesis_investigator.py @@ -0,0 +1,205 @@ +"""Upstream's ``Investigator``, narrow: a read-only investigation that persists. + +The narrow scope is deliberate. Upstream's investigator also fans out to +``subagent_investigator`` at child nodes, and that part is *not* ported here -- the +port's session work gives a manager read tools of its own, which is what fan-out is +for. What is left, and what nothing else in this port does, is the other half: a +read-only delegation whose findings are **persisted into the version**, so a later +agent inherits them instead of re-discovering them. + +The gap it fills is that ``_MANAGER_PROMPT`` carries the ``CONTEXT.md`` chain and a +listing of files, never their contents -- this port's agents have no read tool. An +investigator is the one role allowed to read a node's code and leave a note about it. +""" + +from __future__ import annotations + +import json + +from agentdescent.filetree import canonical +from agentdescent.evolution import Task +from agentdescent.policies import ProposalContext + +from examples.genesis._delegation import (Brief, Delegation, Edit, + RecursiveDelegation) +from examples.genesis._investigator import (FINDINGS_HEADING, Investigator) +from examples.genesis._suite import llm_manager +from examples.genesis._world import CONTEXT_FILE, LocalWorld, WorldLog + +_STATE = { + CONTEXT_FILE: "# root\n", + "src/" + CONTEXT_FILE: "# src\n", + "src/core.py": "from src.util import g\n\ndef f():\n return g()\n", + "src/util.py": "def g():\n return 42\n", + "src/deep/" + CONTEXT_FILE: "# deep\n", + "src/deep/leaf.py": "def h():\n return 0\n", +} + + +def _brief(state, path, objective="what does this node do?"): + world = LocalWorld(version=1, path=path) + return Brief(world=world, objective=objective, + context=world.situate(state), state=dict(state), + task=Task(id="t", prompt="x"), output="", reward=0.0, depth=1) + + +def _ctx(state, reward=0.0): + return ProposalContext(rendered=canonical(state), + task=Task(id="t", prompt="x"), output="", + reward=reward, base_version=1) + + +def _reply(findings="- `src/core.py` imports `g` from `src/util.py`.\n"): + return json.dumps({"findings": findings, "answer": "two modules."}) + + +# --------------------------------------------------------------------------- +# what it writes, and where +# --------------------------------------------------------------------------- + +def test_findings_are_recorded_under_the_nodes_own_record(): + investigator = Investigator(lambda p: _reply()) + (edit,) = investigator(_brief(_STATE, "src")) + assert edit.owner == "src" + assert edit.path == f"src/{CONTEXT_FILE}" + assert FINDINGS_HEADING in edit.content + assert "- `src/core.py` imports `g` from `src/util.py`." in edit.content + assert edit.content.startswith("# src\n"), "the record is extended, not replaced" + assert (investigator.calls, investigator.recorded) == (1, 1) + + +def test_an_existing_record_is_extended_and_a_missing_one_is_created(): + existing = Investigator(lambda p: _reply())(_brief(_STATE, "src"))[0] + assert existing.kind == "context" # src/CONTEXT.md was there + fresh_state = {CONTEXT_FILE: "# root\n", "new/core.py": "x = 1\n"} + fresh = Investigator(lambda p: _reply())(_brief(fresh_state, "new"))[0] + assert fresh.kind == "record" # the write that creates it + + +def test_the_investigator_is_shown_the_nodes_files_not_its_childrens(): + seen = [] + + def complete(prompt): + seen.append(prompt) + return _reply() + + Investigator(complete)(_brief(_STATE, "src")) + assert "return 42" in seen[0], "the node's own code must reach the reader" + assert "return 0" not in seen[0], "a child subtree is not this node's to read" + assert "src/deep/" in seen[0], "but the reader is told the tree continues" + + +def test_the_question_travels_into_the_prompt(): + seen = [] + Investigator(lambda p: seen.append(p) or _reply())( + _brief(_STATE, "src", objective="where is g defined?")) + assert "where is g defined?" in seen[0] + + +# --------------------------------------------------------------------------- +# the cases that must not be conflated +# --------------------------------------------------------------------------- + +def test_nothing_worth_recording_costs_nothing(): + investigator = Investigator(lambda p: json.dumps({"findings": "", "answer": "none"})) + assert investigator(_brief(_STATE, "src")) == [] + assert (investigator.calls, investigator.empty, investigator.unparsed) == (1, 1, 0) + + +def test_a_node_with_no_files_is_not_investigated(): + investigator = Investigator(lambda p: _reply()) + assert investigator(_brief({CONTEXT_FILE: "# root\n"}, "empty")) == [] + assert (investigator.calls, investigator.empty) == (0, 1) + + +def test_a_broken_reply_and_a_dead_call_are_counted_as_unparsed(): + for complete in (lambda p: "no json at all", + lambda p: json.dumps({"answer": "no findings key"}), + lambda p: (_ for _ in ()).throw(RuntimeError("down"))): + investigator = Investigator(complete) + assert investigator(_brief(_STATE, "src")) == [] + assert investigator.unparsed == 1 + + +def test_a_frozen_record_is_never_written(): + investigator = Investigator(lambda p: _reply(), frozen=("spec/**",)) + state = {"spec/" + CONTEXT_FILE: "# spec\n", "spec/x.md": "x\n"} + assert investigator(_brief(state, "spec")) == [] + assert investigator.refused == 1 + + +# --------------------------------------------------------------------------- +# the seam: a read-only delegation runs the investigator, not the recursion +# --------------------------------------------------------------------------- + +def _policy(investigator, log=None, **kw): + return RecursiveDelegation( + manager=lambda brief: ([Delegation("src", "what is here?", readonly=True)] + if brief.world.path == "" else []), + executor=lambda brief: [Edit(brief.world.path, "src/should_not_exist.py", "x\n")], + log=log or WorldLog(), max_depth=3, investigator=investigator, **kw) + + +def test_a_read_only_delegation_runs_the_investigator_and_folds_its_record(): + log = WorldLog() + policy = _policy(Investigator(lambda p: _reply()), log=log) + policy.propose(_ctx(_STATE, reward=1.0)) + assert policy.investigations == 1 and policy.finding_edits == 1 + roles = [e.role for e in log.episodes] + assert "investigator" in roles + assert "executor" not in roles, "a read-only delegation must not run the executor" + + +def test_the_finding_record_rides_in_the_proposal(): + policy = _policy(Investigator(lambda p: _reply())) + (proposal,) = policy.propose(_ctx(_STATE, reward=1.0)) + assert f"src/{CONTEXT_FILE}" in proposal + assert FINDINGS_HEADING in proposal + + +def test_a_read_only_delegation_without_an_investigator_is_ordinary_work(): + """A flag with nothing behind it must not silently drop the delegation.""" + log = WorldLog() + policy = RecursiveDelegation( + manager=lambda brief: ([Delegation("src", "do it", readonly=True)] + if brief.world.path == "" else []), + executor=lambda brief: [Edit(brief.world.path, "src/core.py", "x = 1\n")], + log=log, max_depth=3) # no investigator installed + policy.propose(_ctx(_STATE, reward=1.0)) + assert "executor" in [e.role for e in log.episodes] + assert policy.investigations == 0 + + +def test_a_write_that_is_not_the_nodes_record_is_refused(): + """Source edits are not an investigation, and an ancestor's record is not ours.""" + def wrong(brief): + return [Edit("src", "src/core.py", "hacked\n")] + + policy = _policy(wrong) + policy.propose(_ctx(_STATE, reward=1.0)) + assert policy.investigations == 1 + assert policy.finding_edits == 0 + assert policy.investigation_refused == 1 + + +# --------------------------------------------------------------------------- +# the manager's protocol +# --------------------------------------------------------------------------- + +def test_llm_manager_reads_the_investigate_flag(): + def complete(prompt): + return json.dumps({"delegations": [ + {"path": "src", "objective": "build it"}, + {"path": "src/deep", "objective": "what is in there?", "investigate": True}]}) + + delegations = llm_manager(complete)(_brief(_STATE, "")) + assert [(d.path, d.readonly) for d in delegations] == [ + ("src", False), ("src/deep", True)] + + +def test_llm_manager_still_accepts_a_reply_without_the_flag(): + def complete(prompt): + return json.dumps({"delegations": [{"path": "src", "objective": "build it"}]}) + + (delegation,) = llm_manager(complete)(_brief(_STATE, "")) + assert delegation.readonly is False diff --git a/tests/test_genesis_skills.py b/tests/test_genesis_skills.py new file mode 100644 index 00000000..ceca4bd9 --- /dev/null +++ b/tests/test_genesis_skills.py @@ -0,0 +1,316 @@ +"""Upstream's ``SkillExtractor``, and the read half that makes it matter. + +The port could already *inherit* a skill: ``LocalWorld.skills()`` collects +``.agents/skills/`` along the node chain, which is ``hierarchical_skill_names/2`` and +what the paper lists among what an accepted version carries (3.1). What it could not +do is write one, and -- because its agents have no read tool -- what it could not do +either is hand an inherited skill's *body* to anyone. A name line telling an agent to +"read one before using it" is an instruction it cannot follow. + +So there are two halves under test here and they are one feature: the extractor +(:mod:`examples.genesis._skills`) and ``LocalWorld.skill_bodies``, without which the +extractor's output is inert. +""" + +from __future__ import annotations + +import json + +from agentdescent.evolution import Task +from agentdescent.filetree import canonical +from agentdescent.policies import ProposalContext + +from examples.genesis._delegation import (Brief, Delegation, Edit, + RecursiveDelegation) +from examples.genesis._skills import (SkillExtractor, is_skill_path, skill_name, + skill_path) +from examples.genesis._world import CONTEXT_FILE, SKILLS_DIR, LocalWorld, WorldLog + + +# --------------------------------------------------------------------------- +# helpers +# --------------------------------------------------------------------------- + +_STATE = { + CONTEXT_FILE: "# root\n", + "src/" + CONTEXT_FILE: "# src\n", + "src/core.py": "def f():\n return 1\n", + "src/util.py": "def g():\n return 2\n", +} + + +def _brief(state, path="", objective="grow the repository"): + world = LocalWorld(version=1, path=path, skill_bodies=True) + return Brief(world=world, objective=objective, + context=world.situate(state), state=dict(state), + task=Task(id="t", prompt="x"), output="", reward=0.0, depth=0) + + +def _ctx(state, reward=0.0): + return ProposalContext(rendered=canonical(state), + task=Task(id="t", prompt="x"), output="", + reward=reward, base_version=1) + + +def _work(*paths): + return [Edit(owner=p.rsplit("/", 1)[0] if "/" in p else "", path=p, content="x = 1\n") + for p in paths] + + +def _one_skill(node="src", name="core-first", body="# core first\n", **kw): + item = {"node": node, "name": name, "description": "import core first", + "body": body} + item.update(kw) + return json.dumps({"skills": [item], "reason": "a gotcha"}) + + +# --------------------------------------------------------------------------- +# paths and names +# --------------------------------------------------------------------------- + +def test_skill_paths_are_node_scoped_under_the_agents_directory(): + assert skill_path("", "deploy") == f"{SKILLS_DIR}/deploy.md" + assert skill_path("src", "deploy") == f"src/{SKILLS_DIR}/deploy.md" + assert skill_path("./src/", "deploy") == f"src/{SKILLS_DIR}/deploy.md" + assert is_skill_path("src/.agents/skills/x.md") + assert not is_skill_path("src/x.py") + assert not is_skill_path("src/.agents/readme.md") + assert skill_name("src/.agents/skills/x.md") == "x" + assert skill_name("src/x.py") is None + + +# --------------------------------------------------------------------------- +# the extractor: what it writes +# --------------------------------------------------------------------------- + +def test_extractor_writes_a_skill_with_upstreams_frontmatter(): + """`skill_add`'s format -- name/description/parameters -- not the model's YAML.""" + extractor = SkillExtractor(lambda p: _one_skill(parameters=[ + {"name": "mod", "type": "str", "description": "the module", "required": True}])) + edits = extractor(_brief(_STATE), _work("src/core.py")) + assert len(edits) == 1 + edit = edits[0] + assert (edit.kind, edit.owner, edit.path) == ( + "skill", "", f"src/{SKILLS_DIR}/core-first.md") + assert edit.content.startswith("---\nname: core-first\n") + assert 'description: "import core first"' in edit.content + assert "parameters:\n - name: mod\n type: str\n" in edit.content + assert " required: true\n" in edit.content + assert edit.content.rstrip().endswith("# core first") + assert (extractor.calls, extractor.extracted, extractor.unparsed) == (1, 1, 0) + + +def test_a_skill_can_be_placed_at_the_root(): + extractor = SkillExtractor(lambda p: _one_skill(node="")) + (edit,) = extractor(_brief(_STATE), _work("src/core.py")) + assert edit.path == f"{SKILLS_DIR}/core-first.md" + + +def test_the_root_brief_ignores_which_node_the_work_came_from(): + """The extractor is a root-level agent: it distils work from anywhere and picks + the node, which is `skill_enable`'s "where it is most relevant".""" + extractor = SkillExtractor(lambda p: _one_skill(node="src")) + (edit,) = extractor(_brief(_STATE), _work("src/core.py", "src/util.py")) + assert edit.path.startswith("src/") + + +# --------------------------------------------------------------------------- +# the extractor: what it refuses, and why each refusal is counted separately +# --------------------------------------------------------------------------- + +def test_a_duplicate_of_an_existing_skill_is_not_written(): + """Upstream: "use `skill_list` ... Avoid duplicating existing skills." """ + state = dict(_STATE) + state[f"src/{SKILLS_DIR}/core-first.md"] = "---\nname: core-first\n---\nold\n" + extractor = SkillExtractor(lambda p: _one_skill()) + assert extractor(_brief(state), _work("src/core.py")) == [] + assert (extractor.deduped, extractor.extracted) == (1, 0) + + +def test_a_name_used_elsewhere_is_a_duplicate_even_at_another_node(): + state = dict(_STATE) + state[f"{SKILLS_DIR}/core-first.md"] = "---\nname: core-first\n---\nold\n" + extractor = SkillExtractor(lambda p: _one_skill(node="src")) + assert extractor(_brief(state), _work("src/core.py")) == [] + assert extractor.deduped == 1 + + +def test_the_written_skill_shows_in_the_extractors_next_prompt(): + """The whole point of `skill_list` before `skill_add`: the second call sees the + first call's skill and can refuse to duplicate it.""" + seen = [] + + def complete(prompt): + seen.append(prompt) + return _one_skill() + + state = dict(_STATE) + extractor = SkillExtractor(complete) + first = extractor(_brief(state), _work("src/core.py")) + for edit in first: + state[edit.path] = edit.content + second = extractor(_brief(state), _work("src/util.py")) + assert second == [] + assert f"src/{SKILLS_DIR}/core-first.md" in seen[1] + + +def test_malformed_names_nodes_and_bodies_are_refused(): + for kw in ({"name": "Not Kebab"}, {"name": "core_first"}, {"name": ""}, + {"body": " "}, {"node": "src/nope"}, {"node": "src/core.py"}): + extractor = SkillExtractor(lambda p, kw=kw: _one_skill(**kw)) + assert extractor(_brief(_STATE), _work("src/core.py")) == [], kw + assert extractor.refused == 1, kw + + +def test_a_skill_may_not_be_written_into_the_frozen_contract(): + extractor = SkillExtractor(lambda p: _one_skill(node="spec"), + frozen=("spec/**",)) + state = dict(_STATE, **{"spec/" + CONTEXT_FILE: "# spec\n"}) + assert extractor(_brief(state), _work("src/core.py")) == [] + assert extractor.refused == 1 + + +def test_at_most_max_skills_are_written_per_contribution(): + reply = json.dumps({"skills": [ + {"node": "src", "name": f"s{i}", "description": "d", "body": "# body\n"} + for i in range(5)]}) + extractor = SkillExtractor(lambda p: reply, max_skills=2) + assert len(extractor(_brief(_STATE), _work("src/core.py"))) == 2 + assert extractor.extracted == 2 # the rest are capped, not refused + + +# --------------------------------------------------------------------------- +# the extractor: the three "nothing happened" cases that must not look alike +# --------------------------------------------------------------------------- + +def test_finding_nothing_is_a_valid_answer_and_not_a_parse_failure(): + """Upstream: "perfectly valid to find no skills worth extracting".""" + extractor = SkillExtractor(lambda p: json.dumps({"skills": [], "reason": "nothing"})) + assert extractor(_brief(_STATE), _work("src/core.py")) == [] + assert (extractor.calls, extractor.extracted, extractor.unparsed) == (1, 0, 0) + + +def test_an_unparseable_reply_is_counted(): + for reply in ("no json here", "{not json}", "[]", '{"skills": "nope"}'): + extractor = SkillExtractor(lambda p, reply=reply: reply) + assert extractor(_brief(_STATE), _work("src/core.py")) == [] + assert extractor.unparsed == 1, reply + + +def test_a_dead_call_costs_a_skill_and_not_the_contribution(): + def boom(prompt): + raise RuntimeError("endpoint down") + + extractor = SkillExtractor(boom) + assert extractor(_brief(_STATE), _work("src/core.py")) == [] + assert extractor.unparsed == 1 + + +def test_bookkeeping_alone_makes_no_call(): + extractor = SkillExtractor(lambda p: _one_skill()) + bookkeeping = [Edit("", CONTEXT_FILE, "# root\n", kind="record")] + assert extractor(_brief(_STATE), bookkeeping) == [] + assert extractor.calls == 0 + + +# --------------------------------------------------------------------------- +# the read half: without it the write half is inert +# --------------------------------------------------------------------------- + +def test_skill_bodies_reach_the_brief_only_when_asked_for(): + skill = f"src/{SKILLS_DIR}/core-first.md" + state = dict(_STATE, **{skill: "---\nname: core-first\n---\nimport core first\n"}) + named = LocalWorld(version=1, path="src").situate(state) + assert skill in named and "import core first" not in named + carried = LocalWorld(version=1, path="src", skill_bodies=True).situate(state) + assert skill in carried and "import core first" in carried + + +def test_skill_bodies_are_inherited_down_the_chain(): + skill = f"src/{SKILLS_DIR}/core-first.md" + state = dict(_STATE, **{skill: "---\nname: core-first\n---\nimport core first\n"}) + deep = LocalWorld(version=1, path="src/core", skill_bodies=True).situate(state) + assert "import core first" in deep + + +# --------------------------------------------------------------------------- +# the seam: a root episode distils what it just accepted +# --------------------------------------------------------------------------- + +def test_a_rework_rollout_does_not_redistil_the_same_contribution(): + """A rework is the same contribution retried, not a new one to learn from.""" + state = {CONTEXT_FILE: "# root\n", "src/" + CONTEXT_FILE: "# src\n"} + log = WorldLog() + policy = RecursiveDelegation( + manager=lambda brief: ([Delegation("src", "build it")] + if brief.world.path == "" else []), + executor=lambda brief: [Edit("src", "src/core.py", "def f():\n return 1\n")], + log=log, max_depth=2, + skills=SkillExtractor(lambda p: _one_skill(node="src"))) + log.request_rework("src", "not yet") + policy.propose(_ctx(state, reward=1.0)) + assert policy.skills_written == 0 + + +def test_a_skill_rides_in_the_proposal_the_engine_commits(): + state = {CONTEXT_FILE: "# root\n"} + policy = RecursiveDelegation( + manager=lambda brief: [], # root does the work itself + executor=lambda brief: [Edit("", "src/core.py", "def f():\n return 1\n")], + log=WorldLog(), max_depth=2, + skills=SkillExtractor(lambda p: _one_skill(node="src"))) + (proposal,) = policy.propose(_ctx(state, reward=1.0)) + assert f"src/{SKILLS_DIR}/core-first.md" in proposal + assert "def f():" in proposal + + +def test_a_skill_is_not_written_when_the_episode_produced_only_bookkeeping(): + state = {CONTEXT_FILE: "# root\n"} + policy = RecursiveDelegation( + manager=lambda brief: [], + executor=lambda brief: [Edit("", "src/core.py", "def f():\n return 1\n", + kind="record")], + log=WorldLog(), max_depth=2, + skills=SkillExtractor(lambda p: _one_skill())) + (proposal,) = policy.propose(_ctx(state, reward=1.0)) + assert SKILLS_DIR not in proposal + assert policy.skills_written == 0 + + +def test_a_managed_root_distils_too(): + """The contribution is what the episode returned, children included.""" + state = {CONTEXT_FILE: "# root\n", "src/" + CONTEXT_FILE: "# src\n"} + policy = RecursiveDelegation( + manager=lambda brief: ([Delegation("src", "build it")] + if brief.world.path == "" else []), + executor=lambda brief: [Edit("src", "src/core.py", "def f():\n return 1\n")], + log=WorldLog(), max_depth=2, + skills=SkillExtractor(lambda p: _one_skill(node="src"))) + (proposal,) = policy.propose(_ctx(state, reward=1.0)) + assert f"src/{SKILLS_DIR}/core-first.md" in proposal + assert policy.skills_written == 1 + + +# --------------------------------------------------------------------------- +# the trust region: skills do not compete with the work they distil +# --------------------------------------------------------------------------- + +def test_skills_are_bounded_separately_from_the_work(): + policy = RecursiveDelegation(manager=lambda b: [], executor=lambda b: [], + log=WorldLog(), max_edits=1, max_skill_edits=2) + work = _work("a.py", "b.py") # one survives, one is trimmed + skills = [Edit("", f"{SKILLS_DIR}/s{i}.md", "---\n---\n", kind="skill") + for i in range(3)] + bound = policy._bound(work + skills) + assert len([e for e in bound if e.kind == "skill"]) == 2 + assert len([e for e in bound if e.kind != "skill"]) == 1 + assert policy.truncated == 1 + + +def test_no_skills_means_the_bounds_are_unchanged(): + """`skills=None` must be the old behaviour exactly, counter and all.""" + policy = RecursiveDelegation(manager=lambda b: [], executor=lambda b: [], + log=WorldLog(), max_edits=1) + assert policy.max_skill_edits == 2 # present, but nothing produces one + bound = policy._bound(_work("a.py", "b.py", "c.py")) + assert len(bound) == 1 and policy.truncated == 2