From e349521eee6577a1e4ee434f672488134dfde778 Mon Sep 17 00:00:00 2001 From: Konrad Rokicki Date: Tue, 13 Oct 2026 09:22:30 -0400 Subject: [PATCH 1/6] Readable names, type hints and plain-language docs across autodev and mia_data Rename one-letter and cryptic variables (self.s -> self.state, p -> pr, cw/vw -> crop_workspace/verify_workspace, _c -> safe_text, ...), add type hints for Config/GitHub/Mirror/jobs/client parameters via TYPE_CHECKING imports, and replace spec-reference comments with short descriptions of what each module, class and non-obvious method does. JobArgs fields ws/n/crop_ws become workspace/n_crops/crop_workspace (activity inputs, not replayed). VerifyResult.n keeps its name because it is an activity result present in recorded Temporal histories. Co-Authored-By: Claude Fable 5.1 --- autodev/activities.py | 276 +++++++------ autodev/agent.py | 91 +++-- autodev/auth.py | 90 +++-- autodev/cli.py | 139 ++++--- autodev/config.py | 91 +++-- autodev/decide.py | 23 +- autodev/github.py | 149 ++++--- autodev/gitops.py | 85 ++-- autodev/jobs.py | 108 +++-- autodev/models.py | 34 +- autodev/ops.py | 51 ++- autodev/review.py | 114 +++--- autodev/workflows.py | 160 ++++---- mia_data/activities.py | 758 ++++++++++++++++++++---------------- mia_data/binding.py | 103 +++-- mia_data/catalog.py | 79 ++-- mia_data/cli.py | 323 ++++++++------- mia_data/config.py | 110 +++--- mia_data/crop_test.py | 69 ++-- mia_data/gallery.py | 44 ++- mia_data/jobspec.py | 186 +++++---- mia_data/models.py | 31 +- mia_data/publish.py | 107 ++--- mia_data/seed.py | 114 ++++-- mia_data/status.py | 41 +- mia_data/workflows.py | 543 ++++++++++++++------------ tests/test_autodev_agent.py | 2 +- tests/test_mia_workflows.py | 32 +- 28 files changed, 2295 insertions(+), 1658 deletions(-) diff --git a/autodev/activities.py b/autodev/activities.py index b4f3528..e344a78 100644 --- a/autodev/activities.py +++ b/autodev/activities.py @@ -1,4 +1,9 @@ -"""Temporal activities: the only code that touches GitHub, git, csub and the filesystem.""" +"""The autodev Temporal activities: the only code that touches GitHub, git, csub and the filesystem. + +Each activity is small and idempotent where it matters (comments are posted once per marker, jobs are +submitted once per key), because Temporal may retry any of them. Anything the agent wrote is treated as +untrusted: read with bounds, never executed, and scrubbed of secrets and fake markers before it is posted. +""" from __future__ import annotations @@ -7,6 +12,7 @@ import logging import re from pathlib import Path +from typing import TYPE_CHECKING, Awaitable, Callable, TypeVar from temporalio import activity from temporalio.common import WorkflowIDConflictPolicy @@ -22,41 +28,56 @@ from .review import WRITE_PERMISSIONS, approved, feedback, handled_ids, latest_round_at from .workflows import IssueWorkflow +if TYPE_CHECKING: + from temporalio.client import Client + + from .agent import AgentJobs + from .config import Config + from .github import GitHub + from .gitops import Mirror + log = logging.getLogger(__name__) -PERMANENT_CSUB = {"policy_violation", "invalid_request"} # the broker or client rejected the spec; csub's -# ProtocolError and ConfigError carry invalid_request. transport_error, lsf_error etc. may pass, so they retry. +# csub error codes meaning the broker or client rejected the spec itself; csub's ProtocolError and ConfigError +# carry invalid_request. transport_error, lsf_error etc. may pass on retry, so they are left retryable. +PERMANENT_CSUB = {"policy_violation", "invalid_request"} PERMANENT_HTTP = {400, 403, 404, 410, 422} # rate-limit 403s are retried inside the GitHub client # ponytail: known secret shapes plus our own token (see neutralize); not a general secret scanner. SECRET_RE = re.compile(r"sk-ant-[\w-]+|gh[pousr]_\w+|github_pat_\w+|eyJ[\w-]+\.[\w-]+\.[\w-]+") +_Fn = TypeVar("_Fn", bound=Callable[..., Awaitable]) + def neutralize(text: str, token: str | None = None) -> str: - """Agent-controlled text must not forge our `` markers, nor publish a credential that a + """Make agent-written text safe to post on GitHub. + + It must not be able to forge our `` markers, nor leak a credential that a prompt-injected agent printed (e.g. from ~/.claude/.credentials.json).""" if token: text = text.replace(token, "[redacted]") return SECRET_RE.sub("[redacted]", text).replace("` markers count.""" handled_re = re.compile(re.escape(marker_prefix) + r" handled (\S+) -->") - out: set[str] = set() - for c in comments: - if _login(c) == bot: - out.update(handled_re.findall(c.get("body") or "")) - return out + handled: set[str] = set() + for comment in comments: + if _login(comment) == bot: + handled.update(handled_re.findall(comment.get("body") or "")) + return handled def combine_checks(check_runs: list[dict], statuses: list[dict], required: list[str] = ()) -> str: - states = ["pending" if r.get("status") != "completed" else - "failure" if r.get("conclusion") in _FAILED else "success" for r in check_runs] - states += [{"success": "success", "pending": "pending"}.get(s.get("state"), "failure") for s in statuses] - seen = {r.get("name") for r in check_runs} | {s.get("context") for s in statuses} + """Fold check runs and commit statuses into one state: failure beats pending beats success. + + "none" means nothing reported and nothing is required; a missing required check counts as pending.""" + states = ["pending" if run.get("status") != "completed" else + "failure" if run.get("conclusion") in _FAILED else "success" for run in check_runs] + states += [{"success": "success", "pending": "pending"}.get(status.get("state"), "failure") for status in statuses] + seen = {run.get("name") for run in check_runs} | {status.get("context") for status in statuses} if not states and not required: return "none" - out = next((s for s in ("failure", "pending") if s in states), "success") - return "pending" if out == "success" and not seen.issuperset(required) else out + worst = next((state for state in ("failure", "pending") if state in states), "success") + return "pending" if worst == "success" and not seen.issuperset(required) else worst diff --git a/autodev/workflows.py b/autodev/workflows.py index 15420b4..24cb1f0 100644 --- a/autodev/workflows.py +++ b/autodev/workflows.py @@ -1,8 +1,14 @@ -"""Temporal workflows. No I/O here; activities are called by name (see activities.py).""" +"""The Temporal workflows that drive one GitHub issue from `go` label to merged PR. + +Workflow code does no I/O of its own: every GitHub, git and cluster interaction is an activity, called by +name so this module never imports the activity implementations (see activities.py). The workflow keeps +its durable state in an `IssueInput` and hands that to continue_as_new when its history grows long. +""" from __future__ import annotations from datetime import timedelta +from typing import Any from temporalio import workflow from temporalio.common import RetryPolicy @@ -21,31 +27,40 @@ class RoundFailed(Exception): - pass + """An agent round ended without a usable result (job failed, or no commits were made).""" -def _err(e: BaseException) -> str: - while isinstance(e, ActivityError) and e.cause: - e = e.cause - return str(e) or type(e).__name__ +def error_message(error: BaseException) -> str: + """The message of the innermost cause of an activity failure, for posting to GitHub.""" + while isinstance(error, ActivityError) and error.cause: + error = error.cause + return str(error) or type(error).__name__ @workflow.defn class IssueWorkflow: + """One workflow per issue: prepare a workspace, run the agent, open a PR, then watch the PR until it + is merged, closed, or needs a human. Reviewer change requests start another agent round.""" + @workflow.init def __init__(self, inp: IssueInput) -> None: - self.s = inp - self.state = "STARTING" + self.state = inp # durable: carried into continue_as_new + self.phase = "STARTING" # for the status query only self.job_id: str | None = None self.last_error: str | None = None self.workspace: Workspace | None = None @workflow.query(name="status") def status(self) -> Status: - return Status(self.s.issue, self.state, self.s.round, self.s.pr, self.job_id, self.last_error) + state = self.state + return Status(state.issue, self.phase, state.round, state.pr, self.job_id, self.last_error) + + async def _act(self, name: str, arg: Any, seconds: int = 60, budget: timedelta = timedelta(hours=6), + result_type: type | None = None) -> Any: + """Run an activity with a per-attempt timeout and an overall retry budget. - async def _act(self, name: str, arg, seconds: int = 60, budget: timedelta = timedelta(hours=6), result_type=None): - # retry by time budget; activities raise non-retryable ApplicationError for permanent failures + Transient failures retry until the budget runs out; activities raise a non-retryable + ApplicationError for failures that a retry cannot fix.""" return await workflow.execute_activity( name, arg, start_to_close_timeout=timedelta(seconds=seconds), schedule_to_close_timeout=budget, retry_policy=RetryPolicy(maximum_interval=RETRY_MAX_INTERVAL), result_type=result_type) @@ -55,89 +70,104 @@ async def _comment(self, number: int, body: str, marker: str) -> None: @workflow.run async def execute(self, inp: IssueInput) -> str: - s = self.s - if not KEY_RE.fullmatch(s.key): # unset, or a bad value from a hand-started run: it names directories - s.key = workflow.info().run_id[:8] + state = self.state + if not KEY_RE.fullmatch(state.key): # unset, or a bad value from a hand-started run: it names directories + state.key = workflow.info().run_id[:8] try: - if s.pr is None: - s.pr = (await self._act("find_pr", s.issue, result_type=PrRef)).pr - if s.pr is None: - await self._act("set_stage", StageInput(s.issue, "working")) - res = await self._round(None) - if res.pr is None: + if state.pr is None: # a restarted workflow may already have a PR from an earlier attempt + state.pr = (await self._act("find_pr", state.issue, result_type=PrRef)).pr + if state.pr is None: + await self._act("set_stage", StageInput(state.issue, "working")) + result = await self._run_round(None) + if result.pr is None: raise RoundFailed("the agent made no commits; its summary is above") - s.pr = res.pr - await self._act("set_stage", StageInput(s.issue, "review")) - return await self._watch() - except (ActivityError, RoundFailed) as e: - self.state, self.last_error = "FAILED", _err(e) - await self._act("report_failure", FailureInput(s.issue, s.key, s.round, self.last_error, self.workspace), + state.pr = result.pr + await self._act("set_stage", StageInput(state.issue, "review")) + return await self._watch_pr() + except (ActivityError, RoundFailed) as error: + self.phase, self.last_error = "FAILED", error_message(error) + await self._act("report_failure", + FailureInput(state.issue, state.key, state.round, self.last_error, self.workspace), budget=timedelta(hours=24)) return "failed" - async def _round(self, fb: Feedback | None) -> PrRef: - s = self.s - s.round += 1 - self.state, self.job_id = "PREPARING", None - self.workspace = ws = await self._act( - "prepare_round", RoundInput(s.issue, s.key, s.round, s.pr, fb.text if fb else None), + async def _run_round(self, feedback: Feedback | None) -> PrRef: + """One agent round: prepare a workspace, submit the agent job, wait for it, publish its commits. + + With `feedback`, the round is a revision of the existing PR; otherwise it is the first attempt.""" + state = self.state + state.round += 1 + self.phase, self.job_id = "PREPARING", None + self.workspace = workspace = await self._act( + "prepare_round", + RoundInput(state.issue, state.key, state.round, state.pr, feedback.text if feedback else None), seconds=600, result_type=Workspace) - job = await self._act("submit_agent", SubmitInput(s.issue, s.key, s.round, ws), + job = await self._act("submit_agent", SubmitInput(state.issue, state.key, state.round, workspace), result_type=JobRef) - self.state, self.job_id = "RUNNING", job.job_id - await self._comment(s.pr or s.issue, f"Round {s.round} started: agent job `{job.job_id}`.", - f"started {s.key}-{s.round}") - out = await workflow.execute_activity( - "await_agent", AwaitInput(job.job_id, ws), result_type=JobOutcome, + self.phase, self.job_id = "RUNNING", job.job_id + await self._comment(state.pr or state.issue, f"Round {state.round} started: agent job `{job.job_id}`.", + f"started {state.key}-{state.round}") + # Waiting on the job runs on its own task queue so long waits don't starve the short activities. + outcome = await workflow.execute_activity( + "await_agent", AwaitInput(job.job_id, workspace), result_type=JobOutcome, task_queue=workflow.info().task_queue + "-await", start_to_close_timeout=timedelta(minutes=job.walltime_min) + QUEUE_SLACK, schedule_to_close_timeout=timedelta(minutes=job.walltime_min) + timedelta(hours=24), heartbeat_timeout=timedelta(minutes=3), retry_policy=RetryPolicy(maximum_interval=RETRY_MAX_INTERVAL)) - workflow.logger.info(ROUND_DONE_LOG, s.issue, s.round, out.job_id, out.state) - if out.state != "DONE" or out.exit_code not in (0, None): - raise RoundFailed(f"agent job {out.job_id} ended {out.state} (exit {out.exit_code})") - self.state = "PUBLISHING" - res = await self._act("publish_round", PublishInput(s.issue, s.key, s.round, ws, s.pr, fb.id if fb else None), - seconds=300, result_type=PrRef) - if res.head_sha: # pushed: a later observe/merge failure must not attach this round's log tail + workflow.logger.info(ROUND_DONE_LOG, state.issue, state.round, outcome.job_id, outcome.state) + if outcome.state != "DONE" or outcome.exit_code not in (0, None): + raise RoundFailed(f"agent job {outcome.job_id} ended {outcome.state} (exit {outcome.exit_code})") + self.phase = "PUBLISHING" + result = await self._act( + "publish_round", + PublishInput(state.issue, state.key, state.round, workspace, state.pr, feedback.id if feedback else None), + seconds=300, result_type=PrRef) + if result.head_sha: # pushed: a later observe/merge failure must not attach this round's log tail self.workspace = None - return res + return result + + async def _watch_pr(self) -> str: + """Poll the PR until it is merged or closed, running revision rounds and escalations as needed. - async def _watch(self) -> str: - s = self.s + Rolls the history over with continue_as_new when Temporal suggests it or the poll cap is hit.""" + state = self.state for _ in range(MAX_POLLS_PER_EXECUTION): - self.state = "REVIEW" - snap = await self._act("observe_pr", ObserveInput(s.pr), result_type=PrSnapshot) - action, detail = decide(snap, s.handled, s.revisions, s.max_revisions) + self.phase = "REVIEW" + snapshot = await self._act("observe_pr", ObserveInput(state.pr), result_type=PrSnapshot) + action, detail = decide(snapshot, state.handled, state.revisions, state.max_revisions) if action == DONE: - await self._act("set_stage", StageInput(s.issue, "done")) + await self._act("set_stage", StageInput(state.issue, "done")) return "merged" if action == CLOSED: - await self._act("set_stage", StageInput(s.issue, None)) + await self._act("set_stage", StageInput(state.issue, None)) return "closed" if action == MERGE: - r = await self._act("merge_pr", MergeInput(s.issue, s.pr, snap.head_sha), result_type=MergeResult) - if r.merged: - await self._act("set_stage", StageInput(s.issue, "done")) + merge = await self._act("merge_pr", MergeInput(state.issue, state.pr, snapshot.head_sha), + result_type=MergeResult) + if merge.merged: + await self._act("set_stage", StageInput(state.issue, "done")) return "merged" - await self._comment(s.pr, f"Approved, but merging failed: {r.message}", f"merge-failed {snap.head_sha}") + await self._comment(state.pr, f"Approved, but merging failed: {merge.message}", + f"merge-failed {snapshot.head_sha}") elif action == REVISE: - s.handled.append(detail.id) - s.revisions += 1 - await self._act("set_stage", StageInput(s.issue, "working")) - await self._round(detail) - await self._act("set_stage", StageInput(s.issue, "review")) + state.handled.append(detail.id) + state.revisions += 1 + await self._act("set_stage", StageInput(state.issue, "working")) + await self._run_round(detail) + await self._act("set_stage", StageInput(state.issue, "review")) continue elif action == ESCALATE: - await self._comment(s.pr, f"Needs a human: {detail}.", f"escalate {detail} {snap.head_sha}") - await workflow.sleep(timedelta(minutes=s.poll_minutes)) + await self._comment(state.pr, f"Needs a human: {detail}.", f"escalate {detail} {snapshot.head_sha}") + await workflow.sleep(timedelta(minutes=state.poll_minutes)) if workflow.info().is_continue_as_new_suggested(): break - workflow.continue_as_new(s) + workflow.continue_as_new(state) @workflow.defn class PollWorkflow: + """Run on a schedule: find issues labelled `go` and start an IssueWorkflow for each.""" + @workflow.run async def execute(self) -> list[int]: return await workflow.execute_activity( diff --git a/mia_data/activities.py b/mia_data/activities.py index c4eb7c1..cb8bfa6 100644 --- a/mia_data/activities.py +++ b/mia_data/activities.py @@ -1,6 +1,10 @@ -"""Temporal activities for mia-data: the only code that touches GitHub, csub and the filesystem (spec §7, §8, §9). +"""The mia-data Temporal activities: the only code that touches GitHub, csub and the filesystem. -Every file a job wrote is read with `read_untrusted` and parsed defensively; results are small dataclasses.""" +The worker keeps each round's trusted inputs (record, downloads entry, pins, checked bindings) under +state_dir/rounds/, which is never mounted into a job. Jobs get copies in their workspaces under +work_root. Every file a job wrote is read back with `read_untrusted` and parsed defensively, and the +results that enter workflow history are the small dataclasses in models.py. +""" from __future__ import annotations @@ -15,17 +19,18 @@ import shutil from datetime import datetime, timedelta, timezone from pathlib import Path +from typing import TYPE_CHECKING, Any, Awaitable, Callable, TypeVar import yaml from temporalio import activity from temporalio.exceptions import ApplicationError from autodev.activities import PERMANENT_CSUB, neutralize, permanent_errors -from autodev.jobs import read_untrusted +from autodev.jobs import JobSpec, read_untrusted from autodev.review import WRITE_PERMISSIONS from . import catalog -from . import publish as pub +from . import publish as crop_publish from .binding import check_binding, hosts, record_urls from .config import HUMAN_LABELS, STAGES from .gallery import gallery_files, publish_gallery @@ -37,9 +42,17 @@ from .seed import ID_RE, MARKER_RE from .status import render_binding, render_crops_table, render_links, render_stage +if TYPE_CHECKING: + from temporalio.client import Client + + from autodev.github import GitHub + from autodev.jobs import CsubJobs + + from .config import Config + log = logging.getLogger(__name__) -TEST_PR_RE = re.compile(r"i0-e(\d+)-[0-9a-f]{7}") # `mia-data test-pr`'s round: PR at a head, no data issue +TEST_PR_RE = re.compile(r"i0-e(\d+)-[0-9a-f]{7}") # `mia-data test-pr`'s round id: PR at a head, no data issue ROUND_RE = re.compile(rf"i\d+-e\d+|{TEST_PR_RE.pattern}") SNIPPET_ISSUE_RE = re.compile(r"") # open_snippet_issue's marker KIND_RE = re.compile(r"[a-z][a-z0-9-]{0,60}") # a job kind, optionally with a suffix (crop-) @@ -50,90 +63,114 @@ PERMANENT = (HostsNotAllowed, PinMoved, Conflict, ValueError, FileNotFoundError, yaml.YAMLError) # inputs that a retry can't fix +_Fn = TypeVar("_Fn", bound=Callable[..., Awaitable]) + -def mia_permanent_errors(fn): - """autodev's permanent_errors, plus our refusals: a moved pin, a host outside the policy, a publication - conflict, a bad input (malformed YAML too) or a missing state file go to needs-human instead of burning - retries (§8.2).""" +def mia_permanent_errors(fn: _Fn) -> _Fn: + """Decorator: autodev's permanent_errors, plus mia-data's own permanent failures. + + A moved pin, a host outside the policy, a publication conflict, a bad input (malformed YAML too) or a + missing state file send the round to needs-human instead of burning retries.""" inner = permanent_errors(fn) @functools.wraps(fn) - async def wrapper(*args, **kw): + async def wrapper(*args, **kwargs): try: - return await inner(*args, **kw) - except PERMANENT as e: - raise ApplicationError(f"{type(e).__name__}: {e}", type=type(e).__name__, non_retryable=True) from e + return await inner(*args, **kwargs) + except PERMANENT as error: + raise ApplicationError(f"{type(error).__name__}: {error}", type=type(error).__name__, + non_retryable=True) from error return wrapper -def _write(p: Path, data: str | bytes) -> None: - p.parent.mkdir(parents=True, exist_ok=True) - tmp = p.with_name(f".{p.name}.tmp") +def write_atomic(path: Path, data: str | bytes) -> None: + """Write via a temp file and rename, so a crash never leaves a half-written file.""" + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_name(f".{path.name}.tmp") tmp.write_bytes(data.encode() if isinstance(data, str) else data) - os.replace(tmp, p) + os.replace(tmp, path) -def _text(x, n: int = 2000) -> str: - return "" if x is None else str(x)[:n] +def _text(value: Any, limit: int = 2000) -> str: + """A job-reported value as a bounded string ("" for None).""" + return "" if value is None else str(value)[:limit] -def _texts(x, n: int = 20) -> list[str]: - return [_text(e, 500) for e in (x if isinstance(x, list) else [] if x is None else [x])[:n]] +def _texts(value: Any, limit: int = 20) -> list[str]: + """A job-reported value as a bounded list of bounded strings (a scalar becomes a one-item list).""" + items = value if isinstance(value, list) else [] if value is None else [value] + return [_text(item, 500) for item in items[:limit]] -def _small(x, limit: int = 16384) -> dict | None: +def _small_dict(value: Any, limit: int = 16384) -> dict | None: """Job-reported JSON goes into workflow history only as a small dict (the result fields are typed dict).""" try: - return x if isinstance(x, dict) and len(json.dumps(x)) <= limit else None + return value if isinstance(value, dict) and len(json.dumps(value)) <= limit else None except (TypeError, ValueError): return None -def _loads(text: str | bytes | None): +def _loads(text: str | bytes | None) -> Any: + """json.loads that returns None instead of raising.""" try: return json.loads(text or "null") except (ValueError, RecursionError): return None -def _json(ws: str | Path, rel: str): - return _loads(read_untrusted(Path(ws), rel, 1 << 20)) +def _job_json(workspace: str | Path, rel: str) -> Any: + """A JSON file a job wrote, or None if missing, unreadable or malformed.""" + return _loads(read_untrusted(Path(workspace), rel, 1 << 20)) def _fenced(text: str, lang: str = "") -> str: + """`text` in a markdown code fence longer than any backtick run inside it.""" fence = "`" * max(3, 1 + max(map(len, re.findall(r"`+", text)), default=0)) return f"{fence}{lang}\n{text.rstrip()}\n{fence}" -def _withdrawn(snap: Path) -> dict[str, str]: - ys = yaml.safe_load(read_untrusted(snap, "WITHDRAWN.yaml", 1 << 20) or "") or [] - return {e["name"]: _text(e.get("reason")) for e in ys if isinstance(e, dict) and isinstance(e.get("name"), str)} +def withdrawn_snippets(snapshot_dir: Path) -> dict[str, str]: + """snippet name -> reason, from the snapshot's WITHDRAWN.yaml.""" + entries = yaml.safe_load(read_untrusted(snapshot_dir, "WITHDRAWN.yaml", 1 << 20) or "") or [] + return {entry["name"]: _text(entry.get("reason")) for entry in entries + if isinstance(entry, dict) and isinstance(entry.get("name"), str)} -def _catalog(snap: Path) -> dict[str, str]: - """name -> snippet.yaml text, hiding withdrawn and superseded snippets (§7.2).""" - root = snap / "snippets" - out = {d.name: t for d in sorted(root.iterdir() if root.is_dir() else []) - if (t := read_untrusted(snap, f"snippets/{d.name}/snippet.yaml", 1 << 20)) is not None} - gone = set(_withdrawn(snap)) - for t in out.values(): - s = (y := yaml.safe_load(t)) and isinstance(y, dict) and y.get("supersedes") - gone |= {s} if isinstance(s, str) else {x for x in s if isinstance(x, str)} if isinstance(s, list) else set() - return {k: v for k, v in out.items() if k not in gone} +def visible_snippets(snapshot_dir: Path) -> dict[str, str]: + """snippet name -> its snippet.yaml text, hiding withdrawn snippets and ones another snippet supersedes.""" + root = snapshot_dir / "snippets" + catalog_text = {entry.name: text for entry in sorted(root.iterdir() if root.is_dir() else []) + if (text := read_untrusted(snapshot_dir, f"snippets/{entry.name}/snippet.yaml", 1 << 20)) is not None} + hidden = set(withdrawn_snippets(snapshot_dir)) + for text in catalog_text.values(): + parsed = yaml.safe_load(text) + superseded = parsed and isinstance(parsed, dict) and parsed.get("supersedes") + if isinstance(superseded, str): + hidden.add(superseded) + elif isinstance(superseded, list): + hidden |= {name for name in superseded if isinstance(name, str)} + return {name: text for name, text in catalog_text.items() if name not in hidden} class Activities: - def __init__(self, cfg, gh_data, gh_snippets, gh_catalog, jobs, bot: str, client=None, probe=None): + """The activity implementations, bound to the three GitHub clients (data, snippets and catalog repos), + the csub job runner and the Temporal client. + + `bot` is the GitHub login mia-data posts as. `probe` returns the csub policy's allowed_hosts patterns; + it is injectable for tests.""" + + def __init__(self, cfg: Config, gh_data: GitHub, gh_snippets: GitHub, gh_catalog: GitHub, jobs: CsubJobs, + bot: str, client: Client | None = None, probe: Callable[[], list[str]] | None = None): self.cfg, self.gh_data, self.gh_snippets, self.gh_catalog = cfg, gh_data, gh_snippets, gh_catalog self.jobs, self.bot, self.client = jobs, bot, client - self.probe = probe or (lambda: list(jobs.client.probe().allowed_hosts)) # csub probe: the policy's patterns + self.probe = probe or (lambda: list(jobs.client.probe().allowed_hosts)) # ponytail: in memory, so a worker restart re-sends each PR once (the workflow dedupes by pr_state) self._sent: dict[int, tuple] = {} # poll: PR number -> the (head, state, merge, needs_human) last signalled def all(self) -> list: """The main queue's activities; await_job runs on the -await queue.""" - return [self.admit, self.limits, self.poll, self.set_stage, self.comment, self.prepare_match, self.prepare_job, self.probe_hosts, - self.submit, self.read_match, self.read_smoke, self.read_verify, self.publish_gallery, + return [self.admit, self.limits, self.poll, self.set_stage, self.comment, self.prepare_match, self.prepare_job, + self.probe_hosts, self.submit, self.read_match, self.read_smoke, self.read_verify, self.publish_gallery, self.publish_crops, self.refuse_label, self.open_snippet_issue, self.pr_state, self.set_crop_status, self.tree_sha, self.adopt_pr_binding] @@ -142,14 +179,15 @@ def all(self) -> list: def _clean(self, text: str) -> str: return neutralize(text, self.gh_data.tokens()) - async def _bot_comment(self, gh, n: int, marker: str) -> dict | None: + async def _bot_comment(self, gh: GitHub, number: int, marker: str) -> dict | None: """The bot's own comment carrying `marker` (an outsider's copy must not count).""" - return next((c for c in await gh.issue_comments(n) - if (c.get("user") or {}).get("login") == self.bot and marker in (c.get("body") or "")), None) + return next((comment for comment in await gh.issue_comments(number) + if (comment.get("user") or {}).get("login") == self.bot and marker in (comment.get("body") or "")), + None) - async def _comment_once(self, gh, n: int, body: str, marker: str) -> str | None: - """The comment's URL (the earlier one, on a retry).""" - return (await self._bot_comment(gh, n, marker) or await gh.comment(n, body)).get("html_url") + async def _comment_once(self, gh: GitHub, number: int, body: str, marker: str) -> str | None: + """Post the comment unless one with this marker exists. Returns the comment's URL (the earlier one, on a retry).""" + return (await self._bot_comment(gh, number, marker) or await gh.comment(number, body)).get("html_url") def _round_dir(self, round_id: str) -> Path: """state_dir/rounds/: the round's record, downloads entry, pins and bindings (never mounted).""" @@ -160,39 +198,42 @@ def _round_dir(self, round_id: str) -> Path: def _pins(self, round_id: str) -> Pins: return Pins(**json.loads((self._round_dir(round_id) / "pins.json").read_text())) - def _round_ws(self, issue: int, round_id: str) -> Path: - """work_root/issue-/, the parent of the round's workspaces; a test-pr round's is + def _round_workspace(self, issue: int, round_id: str) -> Path: + """work_root/issue-/, the parent of the round's job workspaces; a test-pr round's is work_root/test-pr//.""" - rd = self._round_dir(round_id) - if m := TEST_PR_RE.fullmatch(round_id): - return self.cfg.paths.work_root / "test-pr" / m.group(1) / self._pins(round_id).snippets_commit - return self.cfg.paths.work_root / f"issue-{int(issue)}" / rd.name + round_dir = self._round_dir(round_id) + if match := TEST_PR_RE.fullmatch(round_id): + return self.cfg.paths.work_root / "test-pr" / match.group(1) / self._pins(round_id).snippets_commit + return self.cfg.paths.work_root / f"issue-{int(issue)}" / round_dir.name - def _ws(self, issue: int, round_id: str, kind: str, attempt: int) -> Path: + def _make_workspace(self, issue: int, round_id: str, kind: str, attempt: int) -> Path: + """Create (if needed) and return the workspace for one job: /-.""" if not (KIND_RE.fullmatch(kind) and type(attempt) is int and attempt >= 1): raise ValueError(f"bad job kind/attempt {kind!r}/{attempt!r}") - ws = self._round_ws(issue, round_id) / f"{kind}-{attempt}" - ws.mkdir(parents=True, exist_ok=True) - return ws + workspace = self._round_workspace(issue, round_id) / f"{kind}-{attempt}" + workspace.mkdir(parents=True, exist_ok=True) + return workspace - def _job_ws(self, issue: int, round_id: str, ws: str) -> Path: - """A workspace of this issue's round, as prepare_* made it.""" - p = Path(ws) - if p.parent != self._round_ws(issue, round_id) or p.is_symlink(): - raise ValueError(f"{ws} is not a workspace of issue {issue} round {round_id}") - return p + def _job_workspace(self, issue: int, round_id: str, workspace: str) -> Path: + """Check that a workspace path (an activity argument) really is one of this issue's round.""" + path = Path(workspace) + if path.parent != self._round_workspace(issue, round_id) or path.is_symlink(): + raise ValueError(f"{workspace} is not a workspace of issue {issue} round {round_id}") + return path @staticmethod - def _crops(ws: Path) -> Path: - d = ws / "crops" # written by the crop job: it must not point outside its workspace - if d.is_symlink() or not d.is_dir(): - raise ValueError(f"{d} is not a directory") - return d + def _crops_dir(workspace: Path) -> Path: + """The crops/ directory a crop job wrote, which must be a real directory inside its workspace.""" + crops_dir = workspace / "crops" + if crops_dir.is_symlink() or not crops_dir.is_dir(): + raise ValueError(f"{crops_dir} is not a directory") + return crops_dir def _record(self, round_id: str) -> dict: return yaml.safe_load((self._round_dir(round_id) / "record.yaml").read_text()) or {} def _dataset_dir(self, round_id: str, pins: Pins) -> Path: + """Where the dataset's crops are published under storage_root.""" return self.cfg.paths.storage_root / catalog.dataset_folder(self._record(round_id), pins.record) async def _snapshot(self, sha: str) -> Path: @@ -203,122 +244,125 @@ async def _snapshot(self, sha: str) -> Path: @activity.defn(name="admit") @mia_permanent_errors async def admit(self, issue: int, event_id: str) -> Pins: - """Pin the round (§8.1): record id from the issue's first-line marker, record_commit from the downloads - state, snippets main, the harness checkout. Written once to state_dir/rounds//; a retry returns - the same pins.""" + """Pin the round: the record id from the issue's first-line marker, record_commit from the downloads + state, snippets main, and the harness checkout. Written once to state_dir/rounds//; a retry + returns the same pins.""" if not re.fullmatch(r"\d+", str(event_id)): raise ValueError(f"bad event id {event_id!r}") - rd = self._round_dir(f"i{int(issue)}-e{event_id}") - if (rd / "pins.json").is_file(): - return Pins(**json.loads((rd / "pins.json").read_text())) - m = MARKER_RE.match((await self.gh_data.get_issue(issue)).get("body") or "") - if not (m and ID_RE.fullmatch(m.group(1))): + round_dir = self._round_dir(f"i{int(issue)}-e{event_id}") + if (round_dir / "pins.json").is_file(): + return Pins(**json.loads((round_dir / "pins.json").read_text())) + match = MARKER_RE.match((await self.gh_data.get_issue(issue)).get("body") or "") + if not (match and ID_RE.fullmatch(match.group(1))): raise ValueError(f"issue #{issue} has no valid record marker on its first line") - rid = m.group(1) - entry = (await catalog.downloaded(self.gh_catalog)).get(rid) + record_id = match.group(1) + entry = (await catalog.downloaded(self.gh_catalog)).get(record_id) if not (entry and SHA_RE.fullmatch(str(entry.get("record_commit")))): - raise ValueError(f"{rid}: not in the downloads state, or no record_commit") - path = (await catalog.record_paths(self.gh_catalog)).get(rid) + raise ValueError(f"{record_id}: not in the downloads state, or no record_commit") + path = (await catalog.record_paths(self.gh_catalog)).get(record_id) if path is None: - raise ValueError(f"{rid}: no record on main") - rec = await catalog.record(self.gh_catalog, path, entry["record_commit"]) - pins = Pins(rid, entry["record_commit"], await self.gh_snippets.get_ref_sha("main"), + raise ValueError(f"{record_id}: no record on main") + record = await catalog.record(self.gh_catalog, path, entry["record_commit"]) + pins = Pins(record_id, entry["record_commit"], await self.gh_snippets.get_ref_sha("main"), *await asyncio.to_thread(harness_pin, self.cfg)) - _write(rd / "record.yaml", yaml.safe_dump(rec, sort_keys=False)) - _write(rd / "downloads.json", json.dumps(entry, indent=1)) - _write(rd / "pins.json", json.dumps(dataclasses.asdict(pins))) # last: marks the round admitted + write_atomic(round_dir / "record.yaml", yaml.safe_dump(record, sort_keys=False)) + write_atomic(round_dir / "downloads.json", json.dumps(entry, indent=1)) + write_atomic(round_dir / "pins.json", json.dumps(dataclasses.asdict(pins))) # last: marks the round admitted return pins @activity.defn(name="limits") async def limits(self) -> RoundLimits: - c = self.cfg - return RoundLimits(c.crops.test_per_round, c.crops.per_round, c.limits.job_walltime_min, - c.limits.max_pr_revisions) + """The config values the workflow needs but cannot read itself.""" + cfg = self.cfg + return RoundLimits(cfg.crops.test_per_round, cfg.crops.per_round, cfg.limits.job_walltime_min, + cfg.limits.max_pr_revisions) @activity.defn(name="set_stage") @mia_permanent_errors async def set_stage(self, issue: int, stage: str, state: DatasetState | None = None, detail: str = "", extra: str = "") -> None: - """Swap the `data:` label and, within a round, post the stage's comment once (§4): the stage line, - `detail` (neutralized), the caller's `extra` (crops table, totals, PR link) and links to the folders to - look at. Comments are never edited: each stage and result is a new one.""" + """Swap the `data:` label and, within a round, post the stage's comment once. + + The comment has the stage line, `detail` (neutralized), the caller's `extra` (crops table, totals, PR + link) and links to the folders to look at. Comments are never edited: each stage and result is a new one.""" if stage not in STAGES: raise ValueError(f"unknown stage {stage!r}") gh, want = self.gh_data, self.cfg.label(stage) - current = {lb["name"] for lb in (await gh.get_issue(issue)).get("labels", [])} - for s in STAGES: - if (name := self.cfg.label(s)) in current and name != want: + current = {label["name"] for label in (await gh.get_issue(issue)).get("labels", [])} + for other in STAGES: + if (name := self.cfg.label(other)) in current and name != want: await gh.remove_label(issue, name) if want not in current: await gh.add_labels(issue, [want]) if not (state and state.round_id): return - rid, paths = state.round_id, self.cfg.paths - links = {"Workspace": paths.link(self._round_ws(issue, rid))} + round_id, paths = state.round_id, self.cfg.paths + links = {"Workspace": paths.link(self._round_workspace(issue, round_id))} if stage == "done": - pins = self._pins(rid) - links["Crops"] = paths.link(self._dataset_dir(rid, pins)) + pins = self._pins(round_id) + links["Crops"] = paths.link(self._dataset_dir(round_id, pins)) links["Gallery"] = (f"https://github.com/{gh.repo}/tree/{self.cfg.gallery.branch}/galleries/" - f"{pins.record}/{rid}") - m = self.cfg.marker(f"stage {rid} {stage}") + f"{pins.record}/{round_id}") + marker = self.cfg.marker(f"stage {round_id} {stage}") parts = [render_stage(stage, self._clean(detail)), self._clean(extra) if extra else "", render_links(links)] - await self._comment_once(gh, issue, "\n".join(x for x in parts if x) + f"\n{m}", m) + await self._comment_once(gh, issue, "\n".join(part for part in parts if part) + f"\n{marker}", marker) @activity.defn(name="comment") @mia_permanent_errors async def comment(self, issue: int, body: str, marker: str, repo: str = "data", binding_path: str | None = None, binding_hash: str | None = None) -> str | None: - """Once per marker, on a data issue or (repo="snippets") a snippets issue or PR, with the binding appended - when one is given. Returns its URL.""" - m = self.cfg.marker(marker) + """Post once per marker, on a data issue or (repo="snippets") a snippets issue or PR, with the binding + appended when one is given. Returns the comment's URL.""" + full_marker = self.cfg.marker(marker) body = self._clean(body) + (f"\n\n{render_binding(binding_path, binding_hash)}" if binding_path else "") return await self._comment_once(self.gh_snippets if repo == "snippets" else self.gh_data, issue, - f"{body}\n\n{m}", m) + f"{body}\n\n{full_marker}", full_marker) @activity.defn(name="refuse_label") @mia_permanent_errors async def refuse_label(self, issue: int, label: str, reason: str, event_id: str) -> None: """Remove a human label we won't act on and say why, once per event (busy `go`, stray `approve`, ...).""" - if label not in {self.cfg.label(k) for k in HUMAN_LABELS} or not re.fullmatch(r"\d+", str(event_id)): + if label not in {self.cfg.label(kind) for kind in HUMAN_LABELS} or not re.fullmatch(r"\d+", str(event_id)): raise ValueError(f"refuse_label: bad label/event {label!r}/{event_id!r}") await self.gh_data.remove_label(issue, label) - m = self.cfg.marker(f"refused {label} {event_id}") - await self._comment_once(self.gh_data, issue, f"Ignoring `{label}`: {self._clean(reason)}\n\n{m}", m) + marker = self.cfg.marker(f"refused {label} {event_id}") + await self._comment_once(self.gh_data, issue, f"Ignoring `{label}`: {self._clean(reason)}\n\n{marker}", marker) # --- jobs -------------------------------------------------------------------------------- @activity.defn(name="prepare_match") @mia_permanent_errors async def prepare_match(self, issue: int, round_id: str, pins: Pins, attempt: int = 1) -> str: - """The match workspace (§7.2): prompt.md, record.yaml, downloads.json, the downloader's conversion.json / - manifest.json, catalog/.yaml (visible snippets only) and SPEC.md. Returns its path.""" - ws, rd = self._ws(issue, round_id, "match", attempt), self._round_dir(round_id) - snap = await self._snapshot(pins.snippets_commit) - shutil.rmtree(ws / "catalog", ignore_errors=True) - (ws / "catalog").mkdir() - for name, text in _catalog(snap).items(): - (ws / "catalog" / f"{name}.yaml").write_text(text) - shutil.copyfile(rd / "record.yaml", ws / "record.yaml") - shutil.copyfile(rd / "downloads.json", ws / "downloads.json") - dl = json.loads((rd / "downloads.json").read_text()).get("dataset_dir") - for f in ("conversion.json", "manifest.json"): - if isinstance(dl, str) and (t := read_untrusted(Path(dl), f, 16 << 20)) is not None: - (ws / f).write_text(t) - (ws / "SPEC.md").write_text(read_untrusted(snap, "SPEC.md", 1 << 20) or "") - (ws / "prompt.md").write_text(match_prompt(pins)) - return str(ws) + """Make the match agent's workspace: prompt.md, record.yaml, downloads.json, the downloader's + conversion.json / manifest.json, catalog/.yaml (visible snippets only) and SPEC.md. Returns its path.""" + workspace, round_dir = self._make_workspace(issue, round_id, "match", attempt), self._round_dir(round_id) + snapshot_dir = await self._snapshot(pins.snippets_commit) + shutil.rmtree(workspace / "catalog", ignore_errors=True) + (workspace / "catalog").mkdir() + for name, text in visible_snippets(snapshot_dir).items(): + (workspace / "catalog" / f"{name}.yaml").write_text(text) + shutil.copyfile(round_dir / "record.yaml", workspace / "record.yaml") + shutil.copyfile(round_dir / "downloads.json", workspace / "downloads.json") + dataset_dir = json.loads((round_dir / "downloads.json").read_text()).get("dataset_dir") + for name in ("conversion.json", "manifest.json"): + if isinstance(dataset_dir, str) and (text := read_untrusted(Path(dataset_dir), name, 16 << 20)) is not None: + (workspace / name).write_text(text) + (workspace / "SPEC.md").write_text(read_untrusted(snapshot_dir, "SPEC.md", 1 << 20) or "") + (workspace / "prompt.md").write_text(match_prompt(pins)) + return str(workspace) @activity.defn(name="prepare_job") @mia_permanent_errors async def prepare_job(self, issue: int, round_id: str, kind: str, attempt: int, binding: str | None) -> str: - """A smoke/crop/verify workspace with record.yaml and (from state_dir) binding.yaml. Returns its path.""" - ws, rd = self._ws(issue, round_id, kind, attempt), self._round_dir(round_id) - shutil.copyfile(rd / "record.yaml", ws / "record.yaml") + """Make a smoke/crop/verify workspace with record.yaml and (copied from state_dir) binding.yaml. + Returns its path.""" + workspace, round_dir = self._make_workspace(issue, round_id, kind, attempt), self._round_dir(round_id) + shutil.copyfile(round_dir / "record.yaml", workspace / "record.yaml") if binding: - if not Path(binding).resolve().is_relative_to(rd.resolve()): + if not Path(binding).resolve().is_relative_to(round_dir.resolve()): raise ValueError(f"binding {binding} is not one of round {round_id}'s") - shutil.copyfile(binding, ws / "binding.yaml") - return str(ws) + shutil.copyfile(binding, workspace / "binding.yaml") + return str(workspace) @activity.defn(name="probe_hosts") @mia_permanent_errors @@ -326,228 +370,256 @@ async def probe_hosts(self) -> list[str]: """`csub probe`: the policy's allowed_hosts patterns.""" return list(await asyncio.to_thread(self.probe)) - def _spec(self, kind: str, a: JobArgs, snap: Path | None): - ws, pins, cfg = Path(a.ws), a.pins, self.cfg + def _spec(self, kind: str, args: JobArgs, snapshot_dir: Path | None) -> JobSpec: + """Build the JobSpec for one job kind from its arguments.""" + workspace, pins, cfg = Path(args.workspace), args.pins, self.cfg if kind == "match": - return match_job(cfg, ws, pins, "record.yaml") - existing = self._dataset_dir(a.round_id, pins) / "manifest.json" if a.existing else None + return match_job(cfg, workspace, pins, "record.yaml") + existing = self._dataset_dir(args.round_id, pins) / "manifest.json" if args.existing else None existing = existing if existing and existing.is_file() else None if kind == "smoke": - return smoke_job(cfg, ws, pins, "binding.yaml", snap, "record.yaml") + return smoke_job(cfg, workspace, pins, "binding.yaml", snapshot_dir, "record.yaml") if kind == "crop": - return crop_job(cfg, ws, pins, "binding.yaml", snap, "record.yaml", a.n, a.seed, a.round_id, a.batch, - existing, a.trusted) + return crop_job(cfg, workspace, pins, "binding.yaml", snapshot_dir, "record.yaml", args.n_crops, args.seed, + args.round_id, args.batch, existing, args.trusted) if kind == "verify": - if Path(a.crop_ws).parent != ws.parent: - raise ValueError(f"crop workspace {a.crop_ws} is not of this round") - return verify_job(cfg, ws, pins, self._crops(Path(a.crop_ws)), "binding.yaml", existing) + if Path(args.crop_workspace).parent != workspace.parent: + raise ValueError(f"crop workspace {args.crop_workspace} is not of this round") + return verify_job(cfg, workspace, pins, self._crops_dir(Path(args.crop_workspace)), "binding.yaml", existing) raise ValueError(f"unknown job kind {kind!r}") @activity.defn(name="submit") @mia_permanent_errors - async def submit(self, key: str, kind: str, a: JobArgs) -> str: - """Build the job (the builders refuse a moved harness pin), refuse hosts a fresh `csub probe` doesn't allow - (§9), and submit at most once per key. Returns the job id.""" + async def submit(self, key: str, kind: str, args: JobArgs) -> str: + """Build the job (the builders refuse a moved harness pin), refuse hosts a fresh `csub probe` doesn't + allow, and submit at most once per key. Returns the job id.""" from csub import CsubError # pixi-only dependency, imported where used (as autodev does) patterns = await asyncio.to_thread(self.probe) - snap = await self._snapshot(a.sha or a.pins.snippets_commit) if kind in ("smoke", "crop") else None + snapshot_dir = await self._snapshot(args.sha or args.pins.snippets_commit) if kind in ("smoke", "crop") else None - def build(): - spec = self._spec(kind, a, snap) + def build() -> JobSpec: + spec = self._spec(kind, args, snapshot_dir) check_hosts_allowed(spec, patterns) return spec try: return await self.jobs.submit(key, build) - except CsubError as e: - if e.code in PERMANENT_CSUB: - raise ApplicationError(str(e), non_retryable=True) from e + except CsubError as error: + if error.code in PERMANENT_CSUB: + raise ApplicationError(str(error), non_retryable=True) from error raise @activity.defn(name="await_job") - async def await_job(self, job_id: str, ws: str) -> JobDone: - o = await self.jobs.wait(job_id, ws, activity.heartbeat) - return JobDone(o.job_id, o.state, o.exit_code) + async def await_job(self, job_id: str, workspace: str) -> JobDone: + outcome = await self.jobs.wait(job_id, workspace, activity.heartbeat) + return JobDone(outcome.job_id, outcome.state, outcome.exit_code) # --- job results ------------------------------------------------------------------------- @activity.defn(name="read_match") @mia_permanent_errors - async def read_match(self, ws: str) -> MatchResult: - """outcome.json, and for matched/ineligible the checked binding.yaml, copied (with the round's - snippets_commit) to state_dir/rounds//binding-.yaml. Anything we can't trust is `blocked` - with the reason.""" - p = Path(ws) - if not (m := re.fullmatch(r"match-(\d+)", p.name)): - raise ValueError(f"{ws} is not a match workspace") - rd, pins = self._round_dir(p.parent.name), self._pins(p.parent.name) - o = _json(ws, "outcome.json") - if not isinstance(o, dict): + async def read_match(self, workspace: str) -> MatchResult: + """Read the match agent's outcome.json and, for matched/ineligible, check its binding.yaml. + + A checked binding is stored (with the round's snippets_commit) as + state_dir/rounds//binding-.yaml. Anything we can't trust is `blocked` with the reason.""" + path = Path(workspace) + if not (match := re.fullmatch(r"match-(\d+)", path.name)): + raise ValueError(f"{workspace} is not a match workspace") + round_id = path.parent.name + round_dir, pins = self._round_dir(round_id), self._pins(round_id) + outcome = _job_json(workspace, "outcome.json") + if not isinstance(outcome, dict): return MatchResult("blocked", "outcome.json is missing, unreadable or not a JSON object") - kind = o.get("outcome") + kind = outcome.get("outcome") if kind == "blocked": - return MatchResult("blocked", _text(o.get("reason")) or "the match agent reported blocked, with no reason") + return MatchResult("blocked", _text(outcome.get("reason")) or "the match agent reported blocked, with no reason") if kind == "needs_snippet": - draft = _small({k: o.get(k) for k in ("draft_binding", "what_fails", "tried")}, 32768) + draft = _small_dict({key: outcome.get(key) for key in ("draft_binding", "what_fails", "tried")}, 32768) return MatchResult("needs_snippet", draft=draft or {"what_fails": "(the agent's draft was too large)"}) if kind not in ("matched", "ineligible"): return MatchResult("blocked", f"unknown match outcome {_text(kind, 100)!r}") - b, errs = await self._check(read_untrusted(p, "binding.yaml", 1 << 20), p.parent.name, pins, - pins.snippets_commit) - if errs: - return MatchResult("blocked", _text("binding check failed: " + "; ".join(errs))) - path, h = self._store(rd / f"binding-{m.group(1)}.yaml", {**b, "snippets_commit": pins.snippets_commit}) - return MatchResult(kind, binding_path=path, binding_hash=h, - shapes=_small(o.get("shapes")) if kind == "ineligible" else None, - reasons=_texts(b.get("reasons"))) - - async def _check(self, text: str | None, round_id: str, pins: Pins, sha: str) -> tuple[dict | None, list[str]]: - """An agent-written binding, parsed and checked against the round's record and the snippets at `sha`.""" + binding, errors = await self._check_binding(read_untrusted(path, "binding.yaml", 1 << 20), round_id, pins, + pins.snippets_commit) + if errors: + return MatchResult("blocked", _text("binding check failed: " + "; ".join(errors))) + binding_path, digest = self._store_binding(round_dir / f"binding-{match.group(1)}.yaml", + {**binding, "snippets_commit": pins.snippets_commit}) + return MatchResult(kind, binding_path=binding_path, binding_hash=digest, + shapes=_small_dict(outcome.get("shapes")) if kind == "ineligible" else None, + reasons=_texts(binding.get("reasons"))) + + async def _check_binding(self, text: str | None, round_id: str, pins: Pins, + sha: str) -> tuple[dict | None, list[str]]: + """Parse an agent-written binding and check it against the round's record and the snippets at `sha`. + Returns (binding, errors); the binding is None when there are errors.""" if text is None: return None, ["the binding is missing or unreadable"] try: - b = yaml.safe_load(text) - errs = check_binding(b, self._record(round_id), set(_catalog(await self._snapshot(sha))), pins=pins) - if not errs: - hosts([u for a in b["raw"] + (b.get("labels") or []) for u in a["urls"]], self.cfg) - except (yaml.YAMLError, ValueError, RecursionError) as e: # a malformed URL raises in urlsplit - errs = [f"{type(e).__name__}: {e}"] - return b if not errs else None, errs + binding = yaml.safe_load(text) + errors = check_binding(binding, self._record(round_id), set(visible_snippets(await self._snapshot(sha))), + pins=pins) + if not errors: # hosts() raises if a URL would grant a GitHub host + hosts([url for array in binding["raw"] + (binding.get("labels") or []) for url in array["urls"]], self.cfg) + except (yaml.YAMLError, ValueError, RecursionError) as error: # a malformed URL raises in urlsplit + errors = [f"{type(error).__name__}: {error}"] + return binding if not errors else None, errors @staticmethod - def _store(path: Path, b: dict) -> tuple[str, str]: - _write(path, data := yaml.safe_dump(b, sort_keys=False).encode()) + def _store_binding(path: Path, binding: dict) -> tuple[str, str]: + """Write the binding as YAML. Returns (path, sha256 of the written bytes).""" + write_atomic(path, data := yaml.safe_dump(binding, sort_keys=False).encode()) return str(path), hashlib.sha256(data).hexdigest() @activity.defn(name="read_smoke") @mia_permanent_errors - async def read_smoke(self, ws: str) -> SmokeResult: - o = _json(ws, "smoke.json") - if not isinstance(o, dict): + async def read_smoke(self, workspace: str) -> SmokeResult: + """Read the smoke job's smoke.json into a SmokeResult, tolerating any shape.""" + report = _job_json(workspace, "smoke.json") + if not isinstance(report, dict): return SmokeResult(False, errors=["smoke.json is missing, unreadable or not a JSON object"]) - b = o.get("bytes_fetched") - return SmokeResult(o.get("ok") is True, o.get("eligible") is True, _small(o.get("shapes")), _texts(o.get("errors")), - _texts(o.get("warnings")), b if type(b) is int and b >= 0 else 0, - _text(o.get("host_not_allowed"), 300) or None) + fetched = report.get("bytes_fetched") + return SmokeResult(report.get("ok") is True, report.get("eligible") is True, _small_dict(report.get("shapes")), + _texts(report.get("errors")), _texts(report.get("warnings")), + fetched if type(fetched) is int and fetched >= 0 else 0, + _text(report.get("host_not_allowed"), 300) or None) @activity.defn(name="read_verify") @mia_permanent_errors - async def read_verify(self, ws: str, crop_ws: str | None = None) -> VerifyResult: - """verify.json: only crops with ok, a valid id and sha256s for crop.json and raw.png count. Top-level errors - or n == 0 mean nothing was verified. With `crop_ws`, n_effective is the crop report's lowered n.""" - o = _json(ws, "verify.json") - if not isinstance(o, dict): + async def read_verify(self, workspace: str, crop_workspace: str | None = None) -> VerifyResult: + """Read verify.json: only crops with ok, a valid id and sha256s for crop.json and raw.png count. + + Top-level errors or n == 0 mean nothing was verified. With `crop_workspace`, n_effective and + bytes_fetched come from the crop job's report.json.""" + report = _job_json(workspace, "verify.json") + if not isinstance(report, dict): return VerifyResult(0, 0, ["verify.json is missing, unreadable or not a JSON object"], []) - errors, n = _texts(o.get("errors")), o.get("n") if type(o.get("n")) is int and o["n"] >= 0 else 0 - ok, failed, seen = [], [], set() - for c in (o.get("crops") if isinstance(o.get("crops"), list) else [])[:1000]: - if not isinstance(c, dict): + errors = _texts(report.get("errors")) + n_crops = report.get("n") if type(report.get("n")) is int and report["n"] >= 0 else 0 + verified, failed, seen = [], [], set() + for crop in (report.get("crops") if isinstance(report.get("crops"), list) else [])[:1000]: + if not isinstance(crop, dict): continue - cid, files = c.get("crop_id"), c.get("files") - if (c.get("ok") is True and isinstance(cid, str) and CROP_ID_RE.fullmatch(cid) and cid not in seen + crop_id, files = crop.get("crop_id"), crop.get("files") + if (crop.get("ok") is True and isinstance(crop_id, str) and CROP_ID_RE.fullmatch(crop_id) and crop_id not in seen and isinstance(files, dict) and len(files) <= 64 and {"crop.json", "raw.png"} <= files.keys() - and all(isinstance(k, str) and isinstance(v, str) and HEX64.fullmatch(v) for k, v in files.items())): - ok.append({"crop_id": cid, "files": files}) - seen.add(cid) + and all(isinstance(rel, str) and isinstance(digest, str) and HEX64.fullmatch(digest) + for rel, digest in files.items())): + verified.append({"crop_id": crop_id, "files": files}) + seen.add(crop_id) else: - failed.append(f"{_text(cid, 100)}: {'; '.join(_texts(c.get('errors'), 5)) or 'not verified'}") - if not n and not errors: + failed.append(f"{_text(crop_id, 100)}: {'; '.join(_texts(crop.get('errors'), 5)) or 'not verified'}") + if not n_crops and not errors: errors = ["verify checked no crops"] if errors: - ok = [] - rep = _json(crop_ws, "crops/report.json") if crop_ws else None - rep = rep if isinstance(rep, dict) else {} - num = lambda k: rep[k] if type(rep.get(k)) is int and rep[k] >= 0 else None # noqa: E731 - return VerifyResult(len(ok), n, (errors + failed)[:20], ok, num("n"), num("bytes_fetched") or 0) + verified = [] + crop_report = _job_json(crop_workspace, "crops/report.json") if crop_workspace else None + crop_report = crop_report if isinstance(crop_report, dict) else {} + + def count(key: str) -> int | None: + value = crop_report.get(key) + return value if type(value) is int and value >= 0 else None + + return VerifyResult(len(verified), n_crops, (errors + failed)[:20], verified, count("n"), + count("bytes_fetched") or 0) # --- publication ------------------------------------------------------------------------- @activity.defn(name="publish_gallery") @mia_permanent_errors - async def publish_gallery(self, issue: int, round_id: str, kind: str, crop_ws: str, verify_ws: str, + async def publish_gallery(self, issue: int, round_id: str, kind: str, crop_workspace: str, verify_workspace: str, verified: list[dict]) -> GalleryResult: - """raw.png + overlay.png of the verified crops, with the binding and verify report, under - galleries/// (kind = the tested SHA, or production) on the galleries branch (§7.4); a - test-pr round's under galleries/_test-pr///.""" + """Publish raw.png + overlay.png of the verified crops, with the binding and verify report, under + galleries/// (kind = the tested SHA, or production) on the galleries branch; a + test-pr round's go under galleries/_test-pr///. Returns the URLs and a crops table.""" if not (kind == "production" or SHA_RE.fullmatch(kind)): raise ValueError(f"bad gallery kind {kind!r}") - cw, vw = self._job_ws(issue, round_id, crop_ws), self._job_ws(issue, round_id, verify_ws) - rec, m = self._pins(round_id).record, TEST_PR_RE.fullmatch(round_id) - prefix = f"galleries/_test-pr/{rec}/{m.group(1)}/{kind}" if m else f"galleries/{rec}/{round_id}/{kind}" - files = await asyncio.to_thread(gallery_files, cw, vw, verified) + crop_dir = self._job_workspace(issue, round_id, crop_workspace) + verify_dir = self._job_workspace(issue, round_id, verify_workspace) + record_id, test_pr = self._pins(round_id).record, TEST_PR_RE.fullmatch(round_id) + prefix = (f"galleries/_test-pr/{record_id}/{test_pr.group(1)}/{kind}" if test_pr + else f"galleries/{record_id}/{round_id}/{kind}") + files = await asyncio.to_thread(gallery_files, crop_dir, verify_dir, verified) urls = await publish_gallery(self.gh_data, self.cfg.gallery.branch, prefix, files) rows = [] - for v in verified: - cid = v["crop_id"] - raw = read_untrusted(cw, f"crops/{cid}/crop.json", 1 << 20, binary=True) - if raw is None or hashlib.sha256(raw).hexdigest() != v["files"]["crop.json"]: - raise ValueError(f"{cid}/crop.json is missing or differs from the verified checksum") - cj = d if isinstance(d := _loads(raw), dict) else {} - rows.append({"crop_id": cid, "plane": cj.get("plane") if isinstance(cj.get("plane"), dict) else {}, - "box": cj.get("box") if isinstance(cj.get("box"), dict) else {}, - "raw_url": urls[f"{cid}/raw.png"], "overlay_url": urls[f"{cid}/overlay.png"]}) + for crop in verified: + crop_id = crop["crop_id"] + raw = read_untrusted(crop_dir, f"crops/{crop_id}/crop.json", 1 << 20, binary=True) + if raw is None or hashlib.sha256(raw).hexdigest() != crop["files"]["crop.json"]: + raise ValueError(f"{crop_id}/crop.json is missing or differs from the verified checksum") + crop_json = parsed if isinstance(parsed := _loads(raw), dict) else {} + rows.append({"crop_id": crop_id, + "plane": crop_json.get("plane") if isinstance(crop_json.get("plane"), dict) else {}, + "box": crop_json.get("box") if isinstance(crop_json.get("box"), dict) else {}, + "raw_url": urls[f"{crop_id}/raw.png"], "overlay_url": urls[f"{crop_id}/overlay.png"]}) return GalleryResult(prefix, urls, render_crops_table(rows)) @activity.defn(name="publish_crops") @mia_permanent_errors - async def publish_crops(self, issue: int, round_id: str, crop_ws: str, verified: list[dict], sha: str) -> dict: - """Publish the verified crops into the dataset folder (§7.6), marking crops of snippets withdrawn in + async def publish_crops(self, issue: int, round_id: str, crop_workspace: str, verified: list[dict], sha: str) -> dict: + """Publish the verified crops into the dataset folder, marking crops of snippets withdrawn in mia-snippets at `sha`. Returns {"published", "already", "total"}.""" - crops = self._crops(self._job_ws(issue, round_id, crop_ws)) - withdrawn = _withdrawn(await self._snapshot(sha)) - t = activity.info().start_to_close_timeout if activity.in_activity() else None - return await asyncio.to_thread(pub.publish, self._dataset_dir(round_id, self._pins(round_id)), crops, verified, - withdrawn, tmp_max_age_s=2 * (t.total_seconds() if t else 3600)) + crops_dir = self._crops_dir(self._job_workspace(issue, round_id, crop_workspace)) + withdrawn = withdrawn_snippets(await self._snapshot(sha)) + timeout = activity.info().start_to_close_timeout if activity.in_activity() else None + return await asyncio.to_thread(crop_publish.publish, self._dataset_dir(round_id, self._pins(round_id)), crops_dir, + verified, withdrawn, + tmp_max_age_s=2 * (timeout.total_seconds() if timeout else 3600)) # --- snippet development ----------------------------------------------------------------- @activity.defn(name="open_snippet_issue") @mia_permanent_errors async def open_snippet_issue(self, issue: int, round_id: str, draft: dict) -> int: - """The mia-snippets issue for a new snippet (§7.3), found again by its first-line marker on a retry.""" - gh, g, pins = self.gh_snippets, self.cfg.github, self._pins(round_id) - m = self.cfg.marker(f"issue={int(issue)} round={round_id}") + """Open the mia-snippets issue asking autodev for a new snippet, or find it again by its first-line + marker on a retry. Returns the issue number.""" + gh, github_cfg, pins = self.gh_snippets, self.cfg.github, self._pins(round_id) + marker = self.cfg.marker(f"issue={int(issue)} round={round_id}") # ponytail: lists every issue; one call per round. Filter by author here: an outsider can copy the marker. - for i in await gh.list_issues(state="all"): - if (i.get("user") or {}).get("login") == self.bot and (i.get("body") or "").startswith(m): - return i["number"] - rec = self._record(round_id) - urls = record_urls(rec) - fmt = _text(((rec.get("data") or {}).get("formats") or ["unknown"])[0], 40) + for existing in await gh.list_issues(state="all"): + if (existing.get("user") or {}).get("login") == self.bot and (existing.get("body") or "").startswith(marker): + return existing["number"] + record = self._record(round_id) + urls = record_urls(record) + data_format = _text(((record.get("data") or {}).get("formats") or ["unknown"])[0], 40) # ponytail: container guessed from the record's URLs; the agent's what_fails is the authority - container = ("zip" if any("::" in u or u.split("?")[0].endswith(".zip") for u in urls) - else "s3" if any(u.startswith("s3://") for u in urls) else "http") - excerpt = yaml.safe_dump({k: rec.get(k) for k in ("title", "imaging", "data", "technical")}, sort_keys=False) - d = draft or {} + container = ("zip" if any("::" in url or url.split("?")[0].endswith(".zip") for url in urls) + else "s3" if any(url.startswith("s3://") for url in urls) else "http") + excerpt = yaml.safe_dump({key: record.get(key) for key in ("title", "imaging", "data", "technical")}, + sort_keys=False) + draft = draft or {} body = "\n\n".join([ - m, f"New snippet needed for dataset `{pins.record}` ([{g.data_repo}#{issue}]" - f"(https://github.com/{g.data_repo}/issues/{issue})). Read " - f"[SPEC.md](https://github.com/{g.snippets_repo}/blob/main/SPEC.md) first, and add the binding you " - f"test as `examples/{pins.record}.yaml`.", + marker, f"New snippet needed for dataset `{pins.record}` ([{github_cfg.data_repo}#{issue}]" + f"(https://github.com/{github_cfg.data_repo}/issues/{issue})). Read " + f"[SPEC.md](https://github.com/{github_cfg.snippets_repo}/blob/main/SPEC.md) first, and add the " + f"binding you test as `examples/{pins.record}.yaml`.", f"**Record** (at `{pins.record_commit}`):", _fenced(self._clean(excerpt[:8000]), "yaml"), - "**What fails** (the match agent's report):", _fenced(self._clean(_text(d.get("what_fails"), 4000))), - "**Tried:**", _fenced(self._clean(json.dumps(d.get("tried"), indent=1)[:8000]), "json"), - "**Draft binding:**", _fenced(self._clean(yaml.safe_dump(d.get("draft_binding"), sort_keys=False)[:16000]), "yaml")]) - title = f"New snippet for {self._clean(fmt)}/{container} ({g.data_repo.split('/')[-1]}#{issue})" + "**What fails** (the match agent's report):", _fenced(self._clean(_text(draft.get("what_fails"), 4000))), + "**Tried:**", _fenced(self._clean(json.dumps(draft.get("tried"), indent=1)[:8000]), "json"), + "**Draft binding:**", + _fenced(self._clean(yaml.safe_dump(draft.get("draft_binding"), sort_keys=False)[:16000]), "yaml")]) + title = (f"New snippet for {self._clean(data_format)}/{container} " + f"({github_cfg.data_repo.split('/')[-1]}#{issue})") return (await gh.create_issue(title, body, ["snippets:go"]))["number"] @activity.defn(name="pr_state") @mia_permanent_errors async def pr_state(self, pr: int) -> PrUpdate: - """The PR's current head, state and merge SHA, read fresh (§8.3), and whether its issue needs a human.""" - p = await self.gh_snippets.get_pr(pr) - m = re.fullmatch(r"snippets/issue-(\d+)", p["head"]["ref"]) - if not m or ((p["head"].get("repo") or {}).get("full_name")) != self.gh_snippets.repo: + """The PR's current head, state and merge SHA, read fresh, and whether its issue needs a human.""" + pr_data = await self.gh_snippets.get_pr(pr) + match = re.fullmatch(r"snippets/issue-(\d+)", pr_data["head"]["ref"]) + if not match or ((pr_data["head"].get("repo") or {}).get("full_name")) != self.gh_snippets.repo: raise ValueError(f"PR #{pr} is not on a snippets/issue- branch of {self.gh_snippets.repo}") - k, merged = int(m.group(1)), bool(p.get("merged")) - labels = {lb["name"] for lb in (await self.gh_snippets.get_issue(k)).get("labels", [])} - return PrUpdate(pr, p["head"]["sha"], "merged" if merged else p["state"], - p.get("merge_commit_sha") if merged else None, k, "snippets:needs-human" in labels, - p.get("html_url")) + snippet_issue, merged = int(match.group(1)), bool(pr_data.get("merged")) + labels = {label["name"] for label in (await self.gh_snippets.get_issue(snippet_issue)).get("labels", [])} + return PrUpdate(pr, pr_data["head"]["sha"], "merged" if merged else pr_data["state"], + pr_data.get("merge_commit_sha") if merged else None, snippet_issue, + "snippets:needs-human" in labels, pr_data.get("html_url")) @activity.defn(name="set_crop_status") @mia_permanent_errors async def set_crop_status(self, sha: str, state: str, url: str | None, description: str = "") -> None: + """Post the `mia-data/crops` commit status on a snippets PR head.""" if not SHA_RE.fullmatch(sha) or state not in CROP_STATUS: raise ValueError(f"bad status {sha!r}/{state!r}") await self.gh_snippets.set_status(sha, "mia-data/crops", state, description or CROP_STATUS[state], url) @@ -555,7 +627,7 @@ async def set_crop_status(self, sha: str, state: str, url: str | None, descripti @activity.defn(name="tree_sha") @mia_permanent_errors async def tree_sha(self, sha: str, name: str) -> str | None: - """Tree SHA of snippets/ at `sha` (§7.3 step 5); None if absent.""" + """Tree SHA of snippets/ at `sha` (tells whether a merge changed the snippet); None if absent.""" if not SHA_RE.fullmatch(sha) or not NAME_RE.fullmatch(name): raise ValueError(f"bad tree lookup {sha!r}/{name!r}") return await self.gh_snippets.dir_sha(f"snippets/{name}", sha) @@ -563,99 +635,109 @@ async def tree_sha(self, sha: str, name: str) -> str | None: @activity.defn(name="adopt_pr_binding") @mia_permanent_errors async def adopt_pr_binding(self, round_id: str, pr: int, sha: str) -> dict: - """The PR's example binding, `snippets//examples/.yaml` at `sha` (a head or the merge commit), - where is the one snippet directory the PR adds. Checked like the match agent's (§7.4: this record, - its hosts, the schema) and using , then stored with `snippets_commit: ` as - state_dir/rounds//binding-pr-.yaml. Returns {"path", "hash", "name", "errors": []}, or - {"errors"}: a bad example fails that head's test.""" + """Take the PR's example binding, `snippets//examples/.yaml` at `sha` (a head or the merge + commit), where is the one snippet directory the PR adds. + + It is checked like the match agent's binding (this record, its hosts, the schema) and must use , + then stored with `snippets_commit: ` as state_dir/rounds//binding-pr-.yaml. Returns + {"path", "hash", "name", "errors": []}, or {"errors": [...]}: a bad example fails that head's test.""" if not SHA_RE.fullmatch(sha): raise ValueError(f"bad sha {sha!r}") - gs, rd, pins = self.gh_snippets, self._round_dir(round_id), self._pins(round_id) - names = sorted({f.split("/")[1] for f in await gs.pr_files(pr) if f.startswith("snippets/") and "/" in f[9:]}) + gh, round_dir, pins = self.gh_snippets, self._round_dir(round_id), self._pins(round_id) + names = sorted({file.split("/")[1] for file in await gh.pr_files(pr) + if file.startswith("snippets/") and "/" in file[9:]}) if len(names) != 1 or not NAME_RE.fullmatch(name := names[0]): return {"errors": [_text(f"the PR must add exactly one snippets// directory, not {names}", 500)]} - if not await gs.dir_sha(f"snippets/{name}", sha): + if not await gh.dir_sha(f"snippets/{name}", sha): return {"errors": [f"snippets/{name} is not in {sha}"]} rel = f"snippets/{name}/examples/{pins.record}.yaml" - raw = await gs.get_file(rel, sha) + raw = await gh.get_file(rel, sha) text = raw.decode(errors="replace") if raw is not None and len(raw) <= 1 << 20 else None - b, errs = await self._check(text, round_id, pins, sha) - if b and name not in {a["snippet"] for a in b["raw"] + (b.get("labels") or [])}: - errs = [f"{rel} does not use snippet {name!r}"] - if errs: - return {"errors": [f"{rel}: {x}" for x in _texts(errs)]} - path, h = self._store(rd / f"binding-pr-{sha[:7]}.yaml", {**b, "snippets_commit": sha}) - return {"path": path, "hash": h, "name": name, "errors": []} + binding, errors = await self._check_binding(text, round_id, pins, sha) + if binding and name not in {array["snippet"] for array in binding["raw"] + (binding.get("labels") or [])}: + errors = [f"{rel} does not use snippet {name!r}"] + if errors: + return {"errors": [f"{rel}: {error}" for error in _texts(errors)]} + binding_path, digest = self._store_binding(round_dir / f"binding-pr-{sha[:7]}.yaml", + {**binding, "snippets_commit": sha}) + return {"path": binding_path, "hash": digest, "name": name, "errors": []} # --- the poller -------------------------------------------------------------------------- @activity.defn(name="poll") @mia_permanent_errors async def poll(self) -> list[str]: - """MiaDataPoll (§8): deliver `data:go`/`data:approve` as signals, wake dataset workflows when their - snippet PR changes, and pass `mia-data/crops` on PRs that add no snippet (§5.5). One bad issue or PR never - skips the rest; the next poll retries it.""" + """One poll: deliver `data:go`/`data:approve` labels as signals, wake dataset workflows whose snippet + PR changed, and pass the `mia-data/crops` status on PRs that add no snippet. + + One bad issue or PR never skips the rest; the next poll retries it. Returns a log of what was done.""" from temporalio.service import RPCError, RPCStatusCode - cfg, gd, gs, did = self.cfg, self.gh_data, self.gh_snippets, [] + cfg, gh_data, gh_snippets, done = self.cfg, self.gh_data, self.gh_snippets, [] for kind in HUMAN_LABELS: label = cfg.label(kind) - for i in await gd.list_issues(labels=label, state="open"): - n = i["number"] + for issue in await gh_data.list_issues(labels=label, state="open"): + number = issue["number"] try: - actor, eid, at = await gd.label_event(n, label) - if not actor or await gd.permission(actor) not in WRITE_PERMISSIONS: - await gd.remove_label(n, label) - m = cfg.marker(f"ignored {label} {eid}") - await self._comment_once(gd, n, f"Ignoring `{label}`: it was added by `{actor or 'unknown'}`, " - f"who does not have write access to this repository.\n\n{m}", m) - elif not (eid and at): - log.warning("poll: #%s %s: label event has no id or time", n, label) - elif kind == "go" and "licence-excluded" in {lb["name"] for lb in i.get("labels", [])}: - await self.refuse_label(n, label, "the dataset's licence is not in `crops.licences` " - "(`licence-excluded`)", eid) + actor, event_id, labeled_at = await gh_data.label_event(number, label) + if not actor or await gh_data.permission(actor) not in WRITE_PERMISSIONS: + await gh_data.remove_label(number, label) + marker = cfg.marker(f"ignored {label} {event_id}") + await self._comment_once(gh_data, number, + f"Ignoring `{label}`: it was added by `{actor or 'unknown'}`, who does " + f"not have write access to this repository.\n\n{marker}", marker) + elif not (event_id and labeled_at): + log.warning("poll: #%s %s: label event has no id or time", number, label) + elif kind == "go" and "licence-excluded" in {lb["name"] for lb in issue.get("labels", [])}: + await self.refuse_label(number, label, "the dataset's licence is not in `crops.licences` " + "(`licence-excluded`)", event_id) else: + # start_signal: starts the workflow if needed and delivers the label as a signal either way await self.client.start_workflow( - "MiaDataDataset", DatasetState(n), id=cfg.workflow_id(n), task_queue=cfg.task_queue, - start_signal=kind, start_signal_args=[LabelEvent(kind, eid, actor, at)]) - await gd.remove_label(n, label) # only once the signal is accepted (§8.3) - did.append(f"{kind} #{n}") + "MiaDataDataset", DatasetState(number), id=cfg.workflow_id(number), task_queue=cfg.task_queue, + start_signal=kind, start_signal_args=[LabelEvent(kind, event_id, actor, labeled_at)]) + await gh_data.remove_label(number, label) # only once the signal is accepted + done.append(f"{kind} #{number}") except Exception: - log.exception("poll: issue #%s %s failed", n, label) + log.exception("poll: issue #%s %s failed", number, label) recent = datetime.now(timezone.utc) - timedelta(days=1) # ponytail: the first page (50 most recently updated PRs); paginate if more than 50 are active - for p in await gs.list_prs(state="all"): + for pr_data in await gh_snippets.list_prs(state="all"): try: - head = p["head"] - if p["state"] == "open" and not any(s.get("context") == "mia-data/crops" - for s in await gs.statuses(head["sha"])): - files = await gs.pr_files(p["number"]) - if len(files) < 100 and not any(f.startswith("snippets/") for f in files): # 100: a full page - await gs.set_status(head["sha"], "mia-data/crops", "success", "no snippet in this PR", None) - did.append(f"status PR #{p['number']}") - m = re.fullmatch(r"snippets/issue-(\d+)", head["ref"]) - if (not m or (head.get("repo") or {}).get("full_name") != gs.repo - or (p["state"] != "open" and datetime.fromisoformat(p["updated_at"]) < recent)): + head = pr_data["head"] + if pr_data["state"] == "open" and not any(status.get("context") == "mia-data/crops" + for status in await gh_snippets.statuses(head["sha"])): + files = await gh_snippets.pr_files(pr_data["number"]) + if len(files) < 100 and not any(file.startswith("snippets/") for file in files): # 100: a full page + await gh_snippets.set_status(head["sha"], "mia-data/crops", "success", "no snippet in this PR", + None) + done.append(f"status PR #{pr_data['number']}") + match = re.fullmatch(r"snippets/issue-(\d+)", head["ref"]) + if (not match or (head.get("repo") or {}).get("full_name") != gh_snippets.repo + or (pr_data["state"] != "open" and datetime.fromisoformat(pr_data["updated_at"]) < recent)): continue - k = await gs.get_issue(int(m.group(1))) - mk = SNIPPET_ISSUE_RE.match(k.get("body") or "") - if not mk or (k.get("user") or {}).get("login") != self.bot: # an outsider can copy the marker + snippet_issue = await gh_snippets.get_issue(int(match.group(1))) + issue_marker = SNIPPET_ISSUE_RE.match(snippet_issue.get("body") or "") + if not issue_marker or (snippet_issue.get("user") or {}).get("login") != self.bot: # an outsider can copy the marker continue - merged = bool(p.get("merged_at")) - u = PrUpdate(p["number"], head["sha"], "merged" if merged else p["state"], - p.get("merge_commit_sha") if merged else None, k["number"], - "snippets:needs-human" in {lb["name"] for lb in k.get("labels", [])}, p.get("html_url")) - if self._sent.get(u.pr) == (key := (u.head_sha, u.state, u.merge_sha, u.needs_human)): + merged = bool(pr_data.get("merged_at")) + update = PrUpdate(pr_data["number"], head["sha"], "merged" if merged else pr_data["state"], + pr_data.get("merge_commit_sha") if merged else None, snippet_issue["number"], + "snippets:needs-human" in {lb["name"] for lb in snippet_issue.get("labels", [])}, + pr_data.get("html_url")) + fingerprint = (update.head_sha, update.state, update.merge_sha, update.needs_human) + if self._sent.get(update.pr) == fingerprint: continue # signalled already: a wake-up per change, not per poll try: - await self.client.get_workflow_handle(cfg.workflow_id(int(mk.group(1)))).signal("pr_update", u) - self._sent[u.pr] = key - did.append(f"pr_update PR #{p['number']}") - except RPCError as e: - if e.status != RPCStatusCode.NOT_FOUND: # no dataset workflow (any more): nothing to wake + await self.client.get_workflow_handle(cfg.workflow_id(int(issue_marker.group(1)))).signal( + "pr_update", update) + self._sent[update.pr] = fingerprint + done.append(f"pr_update PR #{pr_data['number']}") + except RPCError as error: + if error.status != RPCStatusCode.NOT_FOUND: # no dataset workflow (any more): nothing to wake raise except Exception: - log.exception("poll: PR #%s failed", p.get("number")) - if did: - log.info("poll: %s", ", ".join(did)) - return did + log.exception("poll: PR #%s failed", pr_data.get("number")) + if done: + log.info("poll: %s", ", ".join(done)) + return done diff --git a/mia_data/binding.py b/mia_data/binding.py index 3b31ea5..49fad9a 100644 --- a/mia_data/binding.py +++ b/mia_data/binding.py @@ -1,13 +1,23 @@ -"""Binding checks and job hosts (spec §5.4, §9). Worker side: plain dicts, never the harness.""" +"""Checking agent-written bindings, and deciding which hosts a job may reach. + +A binding is the YAML the match agent writes: which snippet loads which URL of the dataset. This module +checks it on the worker side as plain dicts, without the harness, and derives the job's network allowlist +from the URLs it names. +""" from __future__ import annotations import fnmatch import re +from typing import TYPE_CHECKING from urllib.parse import urlsplit from autodev.config import is_github_host +if TYPE_CHECKING: + from .config import Config + from .models import Pins + LABEL_KEY_RE = re.compile(r"[a-z_]+-[a-z0-9_]+-(instance|semantic|binary|points)") @@ -25,56 +35,65 @@ def _host(url: str) -> str: def record_urls(record: dict) -> list[str]: - data, tech = record.get("data") or {}, record.get("technical") or {} - d = data.get("download_url") or [] - return [*([d] if isinstance(d, str) else d), *((tech.get("sample") or {}).get("urls") or []), - *(a["path_pattern"] for a in tech.get("arrays") or [] if a.get("path_pattern"))] + """Every URL a catalog record names: download URLs, sample URLs and array path patterns.""" + data, technical = record.get("data") or {}, record.get("technical") or {} + download = data.get("download_url") or [] + return [*([download] if isinstance(download, str) else download), + *((technical.get("sample") or {}).get("urls") or []), + *(array["path_pattern"] for array in technical.get("arrays") or [] if array.get("path_pattern"))] + +def check_binding(binding: dict, record: dict, catalog_names: set[str], *, pins: Pins) -> list[str]: + """Errors in an agent-written binding (empty = ok). Rewrites s3:// URLs to https in place. -def check_binding(b: dict, record: dict, catalog_names: set[str], *, pins) -> list[str]: - """Errors in an agent-written binding (empty = ok). Rewrites s3:// URLs to https in place. `pins` (a - models.Pins) names the record: the record's own `id` field is not trusted (see seed).""" - if not isinstance(b, dict): + The binding must name the pinned record and commit, use only snippets from the catalog, and only URLs + on hosts the record itself names. `pins` is the trusted source of the record id: the record's own `id` + field is not trusted (see seed).""" + if not isinstance(binding, dict): return ["binding is not a mapping"] - errs = [f"{k} {b.get(k)!r} is not {want!r}" for k, want in (("record", pins.record), ("record_commit", pins.record_commit)) - if b.get(k) != want] - raw, labels = b.get("raw"), b.get("labels") or [] + errors = [f"{key} {binding.get(key)!r} is not {want!r}" + for key, want in (("record", pins.record), ("record_commit", pins.record_commit)) + if binding.get(key) != want] + raw, labels = binding.get("raw"), binding.get("labels") or [] if not (isinstance(raw, list) and raw and isinstance(labels, list)): - return errs + ["raw must be a non-empty list and labels a list"] - allowed = {h for u in record_urls(record) if (h := _host(u))} - for where, arrs in (("raw", raw), ("labels", labels)): - for i, a in enumerate(arrs): - p = f"{where}[{i}]" - if not (isinstance(a, dict) and isinstance(a.get("urls"), list) and a["urls"]): - errs.append(f"{p}: needs a non-empty urls list") + return errors + ["raw must be a non-empty list and labels a list"] + allowed_hosts = {host for url in record_urls(record) if (host := _host(url))} + for section, arrays in (("raw", raw), ("labels", labels)): + for index, array in enumerate(arrays): + location = f"{section}[{index}]" + if not (isinstance(array, dict) and isinstance(array.get("urls"), list) and array["urls"]): + errors.append(f"{location}: needs a non-empty urls list") continue - if not (isinstance(a.get("snippet"), str) and a["snippet"] in catalog_names): - errs.append(f"{p}: unknown snippet {a.get('snippet')!r}") - for j, u in enumerate(a["urls"]): - if not (isinstance(u, str) and (u := https(u)).startswith("https://")): - errs.append(f"{p}: {u!r} is not an https URL") + if not (isinstance(array.get("snippet"), str) and array["snippet"] in catalog_names): + errors.append(f"{location}: unknown snippet {array.get('snippet')!r}") + for url_index, url in enumerate(array["urls"]): + if not (isinstance(url, str) and (url := https(url)).startswith("https://")): + errors.append(f"{location}: {url!r} is not an https URL") continue - a["urls"][j] = u - if (h := _host(u)) not in allowed: - errs.append(f"{p}: host {h!r} is not one of the record's hosts {sorted(allowed)}") - if where == "labels": - if not (isinstance(a.get("label_key"), str) and LABEL_KEY_RE.fullmatch(a["label_key"])): - errs.append(f"{p}: bad label_key {a.get('label_key')!r}") - if not (type(a.get("pairs_with")) is int and 0 <= a["pairs_with"] < len(raw)): - errs.append(f"{p}: pairs_with {a.get('pairs_with')!r} is not an index into raw") - return errs - - -def hosts(urls: list[str], cfg) -> list[str]: + array["urls"][url_index] = url + if (host := _host(url)) not in allowed_hosts: + errors.append(f"{location}: host {host!r} is not one of the record's hosts {sorted(allowed_hosts)}") + if section == "labels": + if not (isinstance(array.get("label_key"), str) and LABEL_KEY_RE.fullmatch(array["label_key"])): + errors.append(f"{location}: bad label_key {array.get('label_key')!r}") + if not (type(array.get("pairs_with")) is int and 0 <= array["pairs_with"] < len(raw)): + errors.append(f"{location}: pairs_with {array.get('pairs_with')!r} is not an index into raw") + return errors + + +def hosts(urls: list[str], cfg: Config) -> list[str]: """Exact hosts a job may reach: the URLs' hosts, their configured redirect targets, and the package hosts.""" - hs = {h for u in urls if (h := _host(u))} - redirects = {k.lower(): v for k, v in cfg.network.redirect_hosts.items()} - out = sorted(hs | {r.lower() for h in hs for r in redirects.get(h, [])} | {p.lower() for p in cfg.network.package_hosts}) - if bad := [h for h in out if is_github_host(h)]: + url_hosts = {host for url in urls if (host := _host(url))} + redirects = {host.lower(): targets for host, targets in cfg.network.redirect_hosts.items()} + allowed = sorted(url_hosts + | {target.lower() for host in url_hosts for target in redirects.get(host, [])} + | {host.lower() for host in cfg.network.package_hosts}) + if bad := [host for host in allowed if is_github_host(host)]: raise ValueError(f"GitHub hosts {bad} must never be granted to a job") - return out + return allowed def missing_hosts(hosts: list[str], patterns: list[str]) -> list[str]: """Hosts no `csub probe` allowed_hosts pattern matches (csub/broker/resolve.py: check_hosts).""" - return [h for h in hosts if not any(h == p or fnmatch.fnmatchcase(h, p) for p in patterns)] + return [host for host in hosts if not any(host == pattern or fnmatch.fnmatchcase(host, pattern) + for pattern in patterns)] diff --git a/mia_data/catalog.py b/mia_data/catalog.py index aa711c8..06e3fe7 100644 --- a/mia_data/catalog.py +++ b/mia_data/catalog.py @@ -1,41 +1,56 @@ -"""Read mia-agentic-search: the downloaded-state files (branch `state`) and dataset records (branch `main`).""" +"""Reading the catalog repository (mia-agentic-search). + +Its `main` branch holds one YAML record per dataset under datasets/; its `state` branch holds the +downloaded-state files that say which datasets are on disk, where, and at which record commit. The +dataset folder naming at the bottom mirrors the catalog's own layout tool, so crops land next to the data. +""" from __future__ import annotations import json import re +from typing import TYPE_CHECKING import yaml -FILES = {"state/downloads.json": "tensorswitch", "state/downloads-native.json": "native"} +if TYPE_CHECKING: + from autodev.github import GitHub + +FILES = {"state/downloads.json": "tensorswitch", "state/downloads-native.json": "native"} # path -> downloader name + +async def downloaded(gh_catalog: GitHub) -> dict[str, dict]: + """record id -> its downloads entry, merging both downloaders' state files. -async def downloaded(gh_catalog) -> dict[str, dict]: - """Union of both `downloaded` maps. Newest `at` wins; `downloaders` lists every downloader that has the id.""" - out: dict[str, dict] = {} - for path, name in FILES.items(): + When both have an id, the entry with the newest `at` wins; `downloaders` lists every downloader that has it.""" + merged: dict[str, dict] = {} + for path, downloader in FILES.items(): raw = await gh_catalog.get_file(path, "state") - for id_, e in (json.loads(raw)["downloaded"] if raw else {}).items(): - prev = out.get(id_) - names = [*(prev["downloaders"] if prev else []), name] - out[id_] = {**(e if not prev or e.get("at", "") > prev.get("at", "") else prev), "downloaders": names} - return out + for record_id, entry in (json.loads(raw)["downloaded"] if raw else {}).items(): + previous = merged.get(record_id) + downloaders = [*(previous["downloaders"] if previous else []), downloader] + newest = entry if not previous or entry.get("at", "") > previous.get("at", "") else previous + merged[record_id] = {**newest, "downloaders": downloaders} + return merged -async def record_paths(gh_catalog) -> dict[str, str]: - tree = (await gh_catalog.request("GET", f"/repos/{gh_catalog.repo}/git/trees/main", params={"recursive": 1})).json() - return {p.rsplit("/", 1)[-1][:-5]: p for t in tree["tree"] if t["type"] == "blob" - and (p := t["path"]).startswith("datasets/") and p.endswith(".yaml")} +async def record_paths(gh_catalog: GitHub) -> dict[str, str]: + """record id -> path of its YAML record on main.""" + tree = (await gh_catalog.request("GET", f"/repos/{gh_catalog.repo}/git/trees/main", + params={"recursive": 1})).json() + return {path.rsplit("/", 1)[-1][:-5]: path for node in tree["tree"] if node["type"] == "blob" + and (path := node["path"]).startswith("datasets/") and path.endswith(".yaml")} -async def record(gh_catalog, path: str, ref: str) -> dict: +async def record(gh_catalog: GitHub, path: str, ref: str) -> dict: + """The parsed YAML record at `path` as of `ref`.""" raw = await gh_catalog.get_file(path, ref) if raw is None: raise FileNotFoundError(f"{gh_catalog.repo}:{path}@{ref}") return yaml.safe_load(raw) -# §6.3 Placement: folder names as mia-agentic-search's tools/miao_layout.py@c25d216 builds them (FAMILIES from +# Folder names as mia-agentic-search's tools/miao_layout.py@c25d216 builds them (FAMILIES from # tools/readiness.py@8e9d14f). ponytail: copied tables; re-sync by hand when upstream adds an organism or modality. FAMILIES = { "em": {"FIB-SEM", "SBF-SEM", "ssTEM", "ssSEM", "TEM", "cryo-EM", "cryo-ET"}, @@ -59,18 +74,24 @@ def _slug(text: str) -> str: return re.sub(r"[^a-z0-9]+", "-", text.lower()).strip("-") -def dataset_folder(rec: dict, record_id: str) -> str: - """`{modality family}-{organism}-{name}`; ValueError where miao_layout would stop (family or organism unclear). - `record_id` is the trusted id (the record's own `id` field is not, see seed).""" - im = rec.get("imaging") or {} - mods, orgs = im.get("modality") or [], im.get("organism") or [] - fams = [{f for m in ms for f, members in FAMILIES.items() if m in members} for ms in (mods[:1], mods)] - fam = next((next(iter(f)) for f in fams if len(f) == 1), None) - if fam is None or len(orgs) != 1 or not _slug(str(orgs[0])): - raise ValueError(f"{record_id}: no dataset folder: modality {mods} / organism {orgs} unclear (one of each needed)") +def dataset_folder(record: dict, record_id: str) -> str: + """The dataset's folder name under storage_root: `{modality family}-{organism}-{name}`. + + Raises ValueError where miao_layout would stop (family or organism unclear). `record_id` is the + trusted id (the record's own `id` field is not, see seed).""" + imaging = record.get("imaging") or {} + modalities, organisms = imaging.get("modality") or [], imaging.get("organism") or [] + # The family of the first modality if it is unambiguous, else the family all modalities share. + family_sets = [{family for modality in subset for family, members in FAMILIES.items() if modality in members} + for subset in (modalities[:1], modalities)] + family = next((next(iter(candidates)) for candidates in family_sets if len(candidates) == 1), None) + if family is None or len(organisms) != 1 or not _slug(str(organisms[0])): + raise ValueError(f"{record_id}: no dataset folder: modality {modalities} / organism {organisms} unclear " + "(one of each needed)") words = record_id.split("-") if len(words) > 2 and re.fullmatch(r"\d+", words[1]): # accession prefix: zenodo-7142003-... words = words[2:] - modality = {_slug(m).replace("-", "") for fam_ in FAMILIES.values() for m in fam_} | {"em", "lm", "sem", "tem", "cryoet"} - name = _slug("-".join(w for w in words if w not in modality) or record_id) - return f"{fam}-{ORGANISM_SHORT.get(orgs[0]) or _slug(str(orgs[0]))}-{name}" + modality_words = ({_slug(modality).replace("-", "") for members in FAMILIES.values() for modality in members} + | {"em", "lm", "sem", "tem", "cryoet"}) + name = _slug("-".join(word for word in words if word not in modality_words) or record_id) + return f"{family}-{ORGANISM_SHORT.get(organisms[0]) or _slug(str(organisms[0]))}-{name}" diff --git a/mia_data/cli.py b/mia_data/cli.py index 3e1e397..54d900b 100644 --- a/mia_data/cli.py +++ b/mia_data/cli.py @@ -1,4 +1,7 @@ -"""mia-data command line (mirrors autodev/cli.py).""" +"""The `mia-data` command line: health checks, seeding issues, labels, the worker, the poll schedule, a +synchronous crop test for snippet PRs, storage maintenance, and deployment management (shared with autodev +through autodev.ops). Heavy imports happen inside the commands that need them. +""" from __future__ import annotations @@ -15,6 +18,7 @@ from datetime import timedelta from fnmatch import fnmatchcase from pathlib import Path +from typing import TYPE_CHECKING, Any, Awaitable, Callable import yaml @@ -24,9 +28,13 @@ from .config import EXTRA_LABELS, HUMAN_LABELS, PROJECT_DIR, STAGES, load_config +if TYPE_CHECKING: + from .activities import Activities + from .config import Config + log = logging.getLogger("mia_data") -# P3 (plan): the catalog's data hosts the broker policy must allow +# the catalog's data hosts the broker policy must allow DATA_HOSTS = ["zenodo.org", "ftp.ebi.ac.uk", "www.ebi.ac.uk", "data.broadinstitute.org", "files.cryoetdataportal.cziscience.com", "huggingface.co", "*.huggingface.co", "ndownloader.figshare.com", "figshare.com", "allencell.s3.amazonaws.com", "s3.amazonaws.com"] @@ -34,17 +42,18 @@ def _lsf_queues() -> set[str] | None: """The cluster's LSF queue names, or None where bqueues is not installed (then the check is skipped).""" - if not (bq := shutil.which("bqueues")): + if not (bqueues := shutil.which("bqueues")): return None - out = subprocess.run([bq, "-w"], capture_output=True, text=True, timeout=60).stdout.splitlines() - return {l.split()[0] for l in out[1:] if l.strip()} + out = subprocess.run([bqueues, "-w"], capture_output=True, text=True, timeout=60).stdout.splitlines() + return {line.split()[0] for line in out[1:] if line.strip()} -def _gh(cfg, repo: str, tokens=None) -> GitHub: +def _gh(cfg: Config, repo: str, tokens: Callable[[], str | None] | None = None) -> GitHub: return GitHub(repo, tokens or token_fn(cfg), api_url=cfg.github.api_url) -async def cmd_check(cfg, args) -> int: +async def cmd_check(cfg: Config, args: argparse.Namespace) -> int: + """Try every external dependency and print one ok/BAD line per check.""" from autodev.jobs import csub_client from .binding import missing_hosts @@ -52,71 +61,75 @@ async def cmd_check(cfg, args) -> int: rows: list[tuple[str, bool, str]] = [] - async def row(name, fn): + async def row(name: str, check: Callable[[], Awaitable[Any]]) -> None: try: - rows.append((name, True, str(await fn()))) - except Exception as e: # report every check, then fail - rows.append((name, False, f"{type(e).__name__}: {e}")) + rows.append((name, True, str(await check()))) + except Exception as error: # report every check, then fail + rows.append((name, False, f"{type(error).__name__}: {error}")) - def repo(r): - async def probe(): - gh = _gh(cfg, r) + def repo_check(repo: str) -> Callable[[], Awaitable[str]]: + async def check(): + gh = _gh(cfg, repo) try: - d = (await gh.request("GET", f"/repos/{r}")).json() + info = (await gh.request("GET", f"/repos/{repo}")).json() finally: await gh.aclose() - return f"{d['full_name']} as {await bot_login(cfg)}" - return probe + return f"{info['full_name']} as {await bot_login(cfg)}" + return check async def temporal(): from temporalio.api.workflowservice.v1 import DescribeNamespaceRequest - c = await _client(cfg) - await c.workflow_service.describe_namespace(DescribeNamespaceRequest(namespace=cfg.temporal.namespace)) + client = await _client(cfg) + await client.workflow_service.describe_namespace(DescribeNamespaceRequest(namespace=cfg.temporal.namespace)) return f"{cfg.temporal.address} ns={cfg.temporal.namespace}" async def csub(): - p = await asyncio.to_thread(csub_client(cfg.deployment).probe) - n = cfg.network - want = [*n.package_hosts, *n.redirect_hosts, *(x for v in n.redirect_hosts.values() for x in v), *DATA_HOSTS] - bad = [f"host not allowed: {h}" for h in missing_hosts(want, list(p.allowed_hosts))] - if "claude" not in p.agents: + """Everything in the broker policy that would stop mia-data's jobs: hosts, the agent, env vars, mounts, queues.""" + probe = await asyncio.to_thread(csub_client(cfg.deployment).probe) + network = cfg.network + want = [*network.package_hosts, *network.redirect_hosts, + *(target for targets in network.redirect_hosts.values() for target in targets), *DATA_HOSTS] + bad = [f"host not allowed: {host}" for host in missing_hosts(want, list(probe.allowed_hosts))] + if "claude" not in probe.agents: bad.append('agent "claude" not permitted (broker.toml: add [agents] claude = ["api.anthropic.com", ' '"claude.ai", "platform.claude.com"])') - if miss := [e for e in HARNESS_ENV if not any(fnmatchcase(e, pat) for pat in p.env_allow)]: - bad.append(f"env not allowed: {', '.join(miss)} (broker.toml [broker]: add \"MIA_HARNESS_*\" to env_allow)") + if missing := [var for var in HARNESS_ENV if not any(fnmatchcase(var, pattern) for pattern in probe.env_allow)]: + bad.append(f"env not allowed: {', '.join(missing)} (broker.toml [broker]: add \"MIA_HARNESS_*\" to env_allow)") mounted = [cfg.harness.dir, cfg.paths.work_root, cfg.paths.storage_root] # every job mounts under these - if miss := [str(d) for d in mounted if not any(d.is_relative_to(r) for r in p.allowed_roots)]: - bad.append(f"outside allowed_roots: {', '.join(miss)} (broker.toml [broker]: add " - f"{', '.join(json.dumps(d) for d in miss)} to allowed_roots)") + if missing := [str(directory) for directory in mounted + if not any(directory.is_relative_to(root) for root in probe.allowed_roots)]: + bad.append(f"outside allowed_roots: {', '.join(missing)} (broker.toml [broker]: add " + f"{', '.join(json.dumps(directory) for directory in missing)} to allowed_roots)") home = Path.home().resolve() - if inside := [str(d) for d in mounted if d.is_relative_to(home) or home.is_relative_to(d)]: + if inside := [str(directory) for directory in mounted + if directory.is_relative_to(home) or home.is_relative_to(directory)]: bad.append(f"under or containing the home directory {home}, which csub never mounts: {', '.join(inside)} " "(move it out of $HOME)") # a policy queue table named after a queue LSF does not have fails every submit with "No such queue" - if (have := await asyncio.to_thread(_lsf_queues)) is not None: - if miss := [n for n, q in p.queues.items() if not q.get("gpu") and n not in have]: - bad.append(f"not an LSF queue: {', '.join(miss)} (broker.toml: name the CPU [queues.] tables and " + if (lsf_queues := await asyncio.to_thread(_lsf_queues)) is not None: + if missing := [name for name, queue in probe.queues.items() if not queue.get("gpu") and name not in lsf_queues]: + bad.append(f"not an LSF queue: {', '.join(missing)} (broker.toml: name the CPU [queues.] tables and " f"[limits] cpu_short_queue/cpu_default_queue after real queues from `bqueues`, e.g. short, local)") if bad: raise RuntimeError("; ".join(bad)) - return f"broker user {p.user}, {len(want)} hosts ok" + return f"broker user {probe.user}, {len(want)} hosts ok" async def dirs(): - for d in (cfg.paths.state_dir, cfg.paths.work_root, cfg.paths.storage_root): - t = d / ".write-test" - t.touch() - t.unlink() + for directory in (cfg.paths.state_dir, cfg.paths.work_root, cfg.paths.storage_root): + probe_file = directory / ".write-test" + probe_file.touch() + probe_file.unlink() return f"{cfg.paths.state_dir}, {cfg.paths.work_root}, {cfg.paths.storage_root}" async def pin(): - c, digest = harness_pin(cfg) - if not (b := harness_bin(cfg)).is_file() or not os.access(b, os.X_OK): - raise RuntimeError(f"{b} is missing or not executable: run `cd harness && pixi install` in harness.dir") - return f"{c[:12]} env {digest[:12]}" + commit, digest = harness_pin(cfg) + if not (binary := harness_bin(cfg)).is_file() or not os.access(binary, os.X_OK): + raise RuntimeError(f"{binary} is missing or not executable: run `cd harness && pixi install` in harness.dir") + return f"{commit[:12]} env {digest[:12]}" for kind in ("data", "snippets", "catalog"): - await row(f"github {kind}", repo(getattr(cfg.github, f"{kind}_repo"))) + await row(f"github {kind}", repo_check(getattr(cfg.github, f"{kind}_repo"))) await row("temporal", temporal) await row("csub broker", csub) await row("directories", dirs) @@ -126,54 +139,60 @@ async def pin(): return 0 if all(ok for _, ok, _ in rows) else 1 -async def cmd_crops_by_snippet(cfg, args) -> int: - for m in sorted(cfg.paths.storage_root.glob("*/manifest.json")): - for c in json.loads(m.read_text())["crops"]: - if any(s.get("name") == args.name for s in c.get("snippets") or []): - print(m.parent.name, c["crop_id"], str(c.get("withdrawn", False)).lower()) +async def cmd_crops_by_snippet(cfg: Config, args: argparse.Namespace) -> int: + """List every published crop made with the named snippet, from the dataset manifests.""" + for manifest in sorted(cfg.paths.storage_root.glob("*/manifest.json")): + for crop in json.loads(manifest.read_text())["crops"]: + if any(snippet.get("name") == args.name for snippet in crop.get("snippets") or []): + print(manifest.parent.name, crop["crop_id"], str(crop.get("withdrawn", False)).lower()) return 0 -async def cmd_reindex(cfg, args) -> int: - from .activities import _withdrawn +async def cmd_reindex(cfg: Config, args: argparse.Namespace) -> int: + """Rebuild manifest.json for one dataset or all of them, using the harness checkout's WITHDRAWN.yaml.""" + from .activities import withdrawn_snippets from .publish import reindex - withdrawn = _withdrawn(cfg.harness.dir) + withdrawn = withdrawn_snippets(cfg.harness.dir) root = cfg.paths.storage_root - for d in [root / args.dataset] if args.dataset else sorted(p for p in root.iterdir() if p.is_dir()): - print(f"{d.name}: {reindex(d, withdrawn)} crop(s)") + datasets = [root / args.dataset] if args.dataset else sorted(path for path in root.iterdir() if path.is_dir()) + for dataset_dir in datasets: + print(f"{dataset_dir.name}: {reindex(dataset_dir, withdrawn)} crop(s)") return 0 -async def cmd_seed(cfg, args) -> int: +async def cmd_seed(cfg: Config, args: argparse.Namespace) -> int: from .seed import seed gh_data, gh_catalog = _gh(cfg, cfg.github.data_repo), _gh(cfg, cfg.github.catalog_repo) try: - made = await seed(cfg, gh_data, gh_catalog, args.dry_run) + created = await seed(cfg, gh_data, gh_catalog, args.dry_run) finally: await gh_data.aclose() await gh_catalog.aclose() - print(f"{'would create' if args.dry_run else 'created'} {len(made)} issue(s)") + print(f"{'would create' if args.dry_run else 'created'} {len(created)} issue(s)") return 0 -async def cmd_init_labels(cfg, args) -> int: +async def cmd_init_labels(cfg: Config, args: argparse.Namespace) -> int: gh = _gh(cfg, cfg.github.data_repo) - labels = {**{cfg.label(k): v for k, v in {**STAGES, **HUMAN_LABELS}.items()}, **EXTRA_LABELS} + labels = {**{cfg.label(kind): spec for kind, spec in {**STAGES, **HUMAN_LABELS}.items()}, **EXTRA_LABELS} try: - for name, (color, desc) in labels.items(): - if await gh.ensure_label(name, color, desc): + for name, (color, description) in labels.items(): + if await gh.ensure_label(name, color, description): print(f"created {name}") finally: await gh.aclose() return 0 -async def run_test_pr(acts, pr: int, binding_file: Path, record_id: str) -> int: - """§12.4: §7.4 on PR 's head in the snippets repo, synchronously, with `mia-data/crops` on that SHA.""" +async def run_test_pr(activities: Activities, pr: int, binding_file: Path, record_id: str) -> int: + """Run the test crops on a snippets PR's head with a binding from disk, synchronously, and post the + `mia-data/crops` status and a result comment on the PR. Returns 0 if the crops passed. + + Used by `mia-data test-pr` for snippets developed by hand, outside a dataset round.""" from . import catalog - from .activities import _catalog, _write + from .activities import visible_snippets, write_atomic from .binding import check_binding, https from .crop_test import judge, run_crop_test from .jobspec import SHA_RE, harness_pin @@ -182,77 +201,91 @@ async def run_test_pr(acts, pr: int, binding_file: Path, record_id: str) -> int: if not ID_RE.fullmatch(record_id): raise ValueError(f"bad record id {record_id!r}") - cfg, gs, gc = acts.cfg, acts.gh_snippets, acts.gh_catalog - head = (await gs.get_pr(pr))["head"]["sha"] - rd = acts._round_dir(rid := f"i0-e{int(pr)}-{head[:7]}") - b = yaml.safe_load(binding_file.read_text()) - for a in [x for k in ("raw", "labels") if isinstance(b, dict) and isinstance(b.get(k), list) for x in b[k]]: - if isinstance(a, dict) and isinstance(a.get("urls"), list): # check_binding's s3:// -> https rewrite - a["urls"] = [https(u) if isinstance(u, str) else u for u in a["urls"]] - if isinstance(b, dict): - b["snippets_commit"] = head # the head under test, whatever the file says - text = yaml.safe_dump(b, sort_keys=False) - if (rd / "binding.yaml").is_file() and (rd / "binding.yaml").read_text() != text: # its jobs would be reused - print(f"{head} was already tested with another binding ({rd}); push a new head to test this one") + cfg, gh_snippets, gh_catalog = activities.cfg, activities.gh_snippets, activities.gh_catalog + head = (await gh_snippets.get_pr(pr))["head"]["sha"] + round_id = f"i0-e{int(pr)}-{head[:7]}" + round_dir = activities._round_dir(round_id) + binding = yaml.safe_load(binding_file.read_text()) + arrays = [array for key in ("raw", "labels") if isinstance(binding, dict) and isinstance(binding.get(key), list) + for array in binding[key]] + for array in arrays: + if isinstance(array, dict) and isinstance(array.get("urls"), list): # check_binding's s3:// -> https rewrite + array["urls"] = [https(url) if isinstance(url, str) else url for url in array["urls"]] + if isinstance(binding, dict): + binding["snippets_commit"] = head # the head under test, whatever the file says + text = yaml.safe_dump(binding, sort_keys=False) + if (round_dir / "binding.yaml").is_file() and (round_dir / "binding.yaml").read_text() != text: # its jobs would be reused + print(f"{head} was already tested with another binding ({round_dir}); push a new head to test this one") return 1 # the head's status stays as it is - await acts.set_crop_status(head, "pending", None, "crop test running") + await activities.set_crop_status(head, "pending", None, "crop test running") try: - rc, note = ((await catalog.downloaded(gc)).get(record_id) or {}).get("record_commit"), "" - if not SHA_RE.fullmatch(str(rc)): - rc = await gc.get_ref_sha("main") - note = f" `{record_id}` is not in the downloads state: its record was read at the catalog's main, `{rc}`." - if (path := (await catalog.record_paths(gc)).get(record_id)) is None: + record_commit = ((await catalog.downloaded(gh_catalog)).get(record_id) or {}).get("record_commit") + note = "" + if not SHA_RE.fullmatch(str(record_commit)): + record_commit = await gh_catalog.get_ref_sha("main") + note = (f" `{record_id}` is not in the downloads state: its record was read at the catalog's main, " + f"`{record_commit}`.") + if (path := (await catalog.record_paths(gh_catalog)).get(record_id)) is None: raise ValueError(f"{record_id}: no record on the catalog's main") - rec = await catalog.record(gc, path, rc) - pins = Pins(record_id, rc, head, *harness_pin(cfg)) - if errs := check_binding(b, rec, set(_catalog(await acts._snapshot(head))), pins=pins): - print("binding check failed:", *errs, sep="\n ") - await acts.set_crop_status(head, "failure", None, "binding check failed") + record = await catalog.record(gh_catalog, path, record_commit) + pins = Pins(record_id, record_commit, head, *harness_pin(cfg)) + if errors := check_binding(binding, record, set(visible_snippets(await activities._snapshot(head))), pins=pins): + print("binding check failed:", *errors, sep="\n ") + await activities.set_crop_status(head, "failure", None, "binding check failed") return 1 - _write(rd / "record.yaml", yaml.safe_dump(rec, sort_keys=False)) - _write(rd / "binding.yaml", text) - _write(rd / "pins.json", json.dumps(dataclasses.asdict(pins))) - v, g, jobs = await run_crop_test(acts, issue=0, round_id=rid, pins=pins, sha=head, trusted=False, - binding_path=str(rd / "binding.yaml"), seed=f"test-pr-{pr}-{head}") - passed, bad = judge(v, jobs) - summary = f"{v.ok_count} of {v.n} crops verified" + write_atomic(round_dir / "record.yaml", yaml.safe_dump(record, sort_keys=False)) + write_atomic(round_dir / "binding.yaml", text) + write_atomic(round_dir / "pins.json", json.dumps(dataclasses.asdict(pins))) + verify, gallery, jobs = await run_crop_test(activities, issue=0, round_id=round_id, pins=pins, sha=head, + trusted=False, binding_path=str(round_dir / "binding.yaml"), + seed=f"test-pr-{pr}-{head}") + passed, failed_jobs = judge(verify, jobs) + summary = f"{verify.ok_count} of {verify.n} crops verified" body = "\n".join([f"Crop test of `{head}` with `{record_id}`: **{'passed' if passed else 'failed'}**, {summary}; " - f"the crop job fetched {v.bytes_fetched} bytes.{note}", *(f"- {x}" for x in bad + v.errors), - "", g.table if g else ""]) - m = cfg.marker(f"test-pr {head}") - body = f"{acts._clean(body)}\n\n{m}" - if c := await acts._bot_comment(gs, pr, m): # a rerun: the comment shows the latest result - if c.get("body") != body: - await gs.edit_comment(c["id"], body) + f"the crop job fetched {verify.bytes_fetched} bytes.{note}", + *(f"- {line}" for line in failed_jobs + verify.errors), "", gallery.table if gallery else ""]) + marker = cfg.marker(f"test-pr {head}") + body = f"{activities._clean(body)}\n\n{marker}" + if posted := await activities._bot_comment(gh_snippets, pr, marker): # a rerun: the comment shows the latest result + if posted.get("body") != body: + await gh_snippets.edit_comment(posted["id"], body) else: - c = await gs.comment(pr, body) - url = c.get("html_url") - await acts.set_crop_status(head, "success" if passed else "failure", url, summary) - except Exception as e: # never leave the required status pending - await acts.set_crop_status(head, "error", None, acts._clean(f"{type(e).__name__}: {e}")) + posted = await gh_snippets.comment(pr, body) + url = posted.get("html_url") + await activities.set_crop_status(head, "success" if passed else "failure", url, summary) + except Exception as error: # never leave the required status pending + await activities.set_crop_status(head, "error", None, activities._clean(f"{type(error).__name__}: {error}")) raise print(f"PR #{pr} at {head}: {'passed' if passed else 'failed'}, {summary}: {url}") - if (now := (await gs.get_pr(pr))["head"]["sha"]) != head: - print(f"the PR head moved to {now} during the test; the status is on {head} only") + if (current := (await gh_snippets.get_pr(pr))["head"]["sha"]) != head: + print(f"the PR head moved to {current} during the test; the status is on {head} only") return 0 if passed else 1 -async def cmd_test_pr(cfg, args) -> int: +def _github_clients(cfg: Config) -> list[GitHub]: + """Clients for the data, snippets and catalog repos, in the order Activities takes them.""" + tokens, github_cfg = token_fn(cfg), cfg.github + return [_gh(cfg, repo, tokens) for repo in (github_cfg.data_repo, github_cfg.snippets_repo, github_cfg.catalog_repo)] + + +async def cmd_test_pr(cfg: Config, args: argparse.Namespace) -> int: from autodev.jobs import CsubJobs from .activities import Activities - tokens, g = token_fn(cfg), cfg.github - ghs = [_gh(cfg, r, tokens) for r in (g.data_repo, g.snippets_repo, g.catalog_repo)] + clients = _github_clients(cfg) try: - acts = Activities(cfg, *ghs, CsubJobs(cfg.deployment, cfg.paths.state_dir / "jobs"), await bot_login(cfg)) - return await run_test_pr(acts, args.pr, Path(args.binding), args.record) + activities = Activities(cfg, *clients, CsubJobs(cfg.deployment, cfg.paths.state_dir / "jobs"), + await bot_login(cfg)) + return await run_test_pr(activities, args.pr, Path(args.binding), args.record) finally: - for gh in ghs: + for gh in clients: await gh.aclose() -async def cmd_worker(cfg, args) -> int: +async def cmd_worker(cfg: Config, args: argparse.Namespace) -> int: + """Run the Temporal worker until SIGTERM: one worker for workflows and short activities, one for the + long-running job waits.""" from temporalio.worker import Worker from autodev.jobs import CsubJobs @@ -261,29 +294,30 @@ async def cmd_worker(cfg, args) -> int: from .workflows import MiaDataDataset, MiaDataPoll client = await _client(cfg) - tokens, g = token_fn(cfg), cfg.github - ghs = [_gh(cfg, r, tokens) for r in (g.data_repo, g.snippets_repo, g.catalog_repo)] + clients = _github_clients(cfg) bot = await bot_login(cfg) - acts = Activities(cfg, *ghs, CsubJobs(cfg.deployment, cfg.paths.state_dir / "jobs"), bot, client) - main = Worker(client, task_queue=cfg.task_queue, workflows=[MiaDataDataset, MiaDataPoll], activities=acts.all(), - max_concurrent_activities=8) - waits = Worker(client, task_queue=cfg.await_queue, activities=[acts.await_job], max_concurrent_activities=20) + activities = Activities(cfg, *clients, CsubJobs(cfg.deployment, cfg.paths.state_dir / "jobs"), bot, client) + main_worker = Worker(client, task_queue=cfg.task_queue, workflows=[MiaDataDataset, MiaDataPoll], + activities=activities.all(), max_concurrent_activities=8) + await_worker = Worker(client, task_queue=cfg.await_queue, activities=[activities.await_job], + max_concurrent_activities=20) task = asyncio.current_task() asyncio.get_running_loop().add_signal_handler(signal.SIGTERM, task.cancel) log.info("worker up as %s: %s ns=%s queues %s, %s", bot, cfg.temporal.address, cfg.temporal.namespace, cfg.task_queue, cfg.await_queue) try: - async with main, waits: + async with main_worker, await_worker: await asyncio.Event().wait() except asyncio.CancelledError: log.info("worker shutting down") finally: - for gh in ghs: + for gh in clients: await gh.aclose() return 0 -async def cmd_schedule(cfg, args) -> int: +async def cmd_schedule(cfg: Config, args: argparse.Namespace) -> int: + """Create or update the Temporal schedule that runs MiaDataPoll every poll interval.""" from temporalio.client import (Schedule, ScheduleActionStartWorkflow, ScheduleAlreadyRunningError, ScheduleIntervalSpec, ScheduleOverlapPolicy, SchedulePolicy, ScheduleSpec, ScheduleUpdate) @@ -291,33 +325,33 @@ async def cmd_schedule(cfg, args) -> int: from .workflows import MiaDataPoll client = await _client(cfg) - sid = f"{cfg.deployment}-poll" - sched = Schedule( + schedule_id = f"{cfg.deployment}-poll" + schedule = Schedule( action=ScheduleActionStartWorkflow(MiaDataPoll.run, id=f"{cfg.deployment}-poll-run", task_queue=cfg.task_queue), spec=ScheduleSpec(intervals=[ScheduleIntervalSpec(every=timedelta(minutes=cfg.temporal.poll_minutes))]), policy=SchedulePolicy(overlap=ScheduleOverlapPolicy.SKIP)) try: - await client.create_schedule(sid, sched) - print(f"created schedule {sid}") + await client.create_schedule(schedule_id, schedule) + print(f"created schedule {schedule_id}") except ScheduleAlreadyRunningError: - await client.get_schedule_handle(sid).update(lambda _: ScheduleUpdate(schedule=sched)) - print(f"updated schedule {sid}") + await client.get_schedule_handle(schedule_id).update(lambda _: ScheduleUpdate(schedule=schedule)) + print(f"updated schedule {schedule_id}") return 0 -def main(argv=None) -> int: - p = argparse.ArgumentParser(prog="mia-data", description=__doc__) - p.add_argument("--config", help="path to mia_data.yaml (default: $MIA_DATA_CONFIG or ./mia_data.yaml)") - sub = p.add_subparsers(dest="cmd", required=True) +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(prog="mia-data", description=__doc__) + parser.add_argument("--config", help="path to mia_data.yaml (default: $MIA_DATA_CONFIG or ./mia_data.yaml)") + sub = parser.add_subparsers(dest="cmd", required=True) sub.add_parser("check", help="verify GitHub, Temporal, csub policy, directories and the harness pin") sub.add_parser("crops-by-snippet", help="list crops made with a snippet").add_argument("name") sub.add_parser("reindex", help="rebuild manifest.json with current WITHDRAWN marks").add_argument("dataset", nargs="?") sub.add_parser("seed", help="create one issue per downloaded dataset").add_argument("--dry-run", action="store_true") sub.add_parser("init-labels", help="create the stage, human and extra labels on the data repo") - t = sub.add_parser("test-pr", help="run the test crops on a snippets PR head and post mia-data/crops") - t.add_argument("pr", type=int) - t.add_argument("--binding", required=True, help="binding YAML to test (e.g. the PR's examples/.yaml)") - t.add_argument("--record", required=True, help="catalog record id") + test_pr = sub.add_parser("test-pr", help="run the test crops on a snippets PR head and post mia-data/crops") + test_pr.add_argument("pr", type=int) + test_pr.add_argument("--binding", required=True, help="binding YAML to test (e.g. the PR's examples/.yaml)") + test_pr.add_argument("--record", required=True, help="catalog record id") sub.add_parser("worker", help="run the Temporal workers (main and await queues)") sub.add_parser("schedule", help="manage the poll Schedule").add_argument("action", choices=["apply"]) sub.add_parser("up", help="start this deployment (tmux: broker, worker; Temporal must already run)") @@ -325,19 +359,20 @@ def main(argv=None) -> int: sub.add_parser("restart", help="down, then up") sub.add_parser("ps", help="show whether this deployment is running") sub.add_parser("attach", help="attach to the deployment's tmux session") - s = sub.add_parser("logs", help="tail a log (default: worker)") - s.add_argument("name", nargs="?", default="worker", choices=["worker", "broker"]) - s.add_argument("-n", "--lines", type=int, default=50) - s.add_argument("-f", "--follow", action="store_true", help="keep following the log") - args = p.parse_args(argv) + logs = sub.add_parser("logs", help="tail a log (default: worker)") + logs.add_argument("name", nargs="?", default="worker", choices=["worker", "broker"]) + logs.add_argument("-n", "--lines", type=int, default=50) + logs.add_argument("-f", "--follow", action="store_true", help="keep following the log") + args = parser.parse_args(argv) cfg = load_config(args.config) if args.cmd in OPS: args.config_path = Path(args.config or os.environ.get("MIA_DATA_CONFIG") or PROJECT_DIR / "mia_data.yaml").resolve() args.up_script, args.config_env = PROJECT_DIR / "scripts" / "mia-data-up.sh", "MIA_DATA_CONFIG" return OPS[args.cmd](cfg, args) _setup_logging(cfg, args.cmd if args.cmd == "worker" else None) - cmd = {"check": cmd_check, "crops-by-snippet": cmd_crops_by_snippet, "reindex": cmd_reindex, "seed": cmd_seed, "init-labels": cmd_init_labels, "worker": cmd_worker, "test-pr": cmd_test_pr, "schedule": cmd_schedule} - return asyncio.run(cmd[args.cmd](cfg, args)) + commands = {"check": cmd_check, "crops-by-snippet": cmd_crops_by_snippet, "reindex": cmd_reindex, "seed": cmd_seed, + "init-labels": cmd_init_labels, "worker": cmd_worker, "test-pr": cmd_test_pr, "schedule": cmd_schedule} + return asyncio.run(commands[args.cmd](cfg, args)) if __name__ == "__main__": diff --git a/mia_data/config.py b/mia_data/config.py index 9ab86fd..a4fe0c6 100644 --- a/mia_data/config.py +++ b/mia_data/config.py @@ -1,4 +1,10 @@ -"""Config: mia_data.yaml + environment overrides. Secrets come only from env / ./.env and key files.""" +"""mia-data configuration: mia_data.yaml, with a few environment-variable overrides. + +mia-data works with three GitHub repositories: the data repo (one issue per dataset, where humans label +and the galleries branch lives), the snippets repo (mia-snippets, the loader code) and the catalog repo +(mia-agentic-search, the dataset records and download state). Secrets come only from the environment, +./.env and key files, never from the YAML. +""" from __future__ import annotations @@ -66,19 +72,20 @@ class PathsCfg(BaseModel): @model_validator(mode="before") @classmethod - def _flatten(cls, d): - if isinstance(d, dict): - d = dict(d) - for k in ("work_root", "storage_root"): - if isinstance(v := d.get(k), dict): - d[k], d[f"{k}_url"] = v.get("path"), v.get("url") - return d - - def link(self, p: Path) -> str | None: - """The browse URL of `p`, from the url of the root it is under; None when that root has none.""" + def _flatten(cls, data): + """Accept `{path, url}` for a root and split it into the `` and `_url` fields.""" + if isinstance(data, dict): + data = dict(data) + for key in ("work_root", "storage_root"): + if isinstance(value := data.get(key), dict): + data[key], data[f"{key}_url"] = value.get("path"), value.get("url") + return data + + def link(self, path: Path) -> str | None: + """The browse URL of `path`, from the url of the root it is under; None when that root has none.""" for root, url in ((self.work_root, self.work_root_url), (self.storage_root, self.storage_root_url)): - if url and p.is_relative_to(root): - rel = p.relative_to(root) + if url and path.is_relative_to(root): + rel = path.relative_to(root) return url.rstrip("/") + ("" if rel == Path() else "/" + quote(rel.as_posix())) return None @@ -105,15 +112,16 @@ class LimitsCfg(BaseModel): class HarnessCfg(BaseModel): - dir: Path + dir: Path # a checkout of the harness repo with its pixi environment installed class NetworkCfg(BaseModel): package_hosts: list[str] = ["pypi.org", "files.pythonhosted.org", "conda.anaconda.org", "repo.prefix.dev"] - redirect_hosts: dict[str, list[str]] = {} # exact hosts, no patterns + redirect_hosts: dict[str, list[str]] = {} # data host -> hosts it redirects to; exact names, no patterns class Config(BaseModel): + """The whole mia_data.yaml, validated, plus the derived names (labels, markers, queues) built from it.""" deployment: str = "mia-data" github: GitHubCfg temporal: TemporalCfg = TemporalCfg(namespace="mia-data") @@ -127,38 +135,40 @@ class Config(BaseModel): @model_validator(mode="after") def _finish(self) -> "Config": + """Cross-field checks and path normalization that pydantic's per-field validation cannot do.""" if not re.fullmatch(r"[a-z][a-z0-9-]{1,30}", self.deployment): raise ValueError(f"deployment {self.deployment!r}: use 2-31 chars of a-z, 0-9, '-'") - g = self.github - if g.auth == "app" and not (g.app_id and g.installation_id and g.private_key_path): + github = self.github + if github.auth == "app" and not (github.app_id and github.installation_id and github.private_key_path): raise ValueError("github.auth=app needs github.app_id, installation_id and private_key_path") for attr in ("private_key_path", "token_file"): - if getattr(g, attr): - setattr(g, attr, getattr(g, attr).expanduser()) - p = self.paths - for o in (p, self.harness): - for k, v in vars(o).items(): - if isinstance(v, Path): - setattr(o, k, v.expanduser().resolve()) - for other in (p.work_root, p.storage_root): - if p.state_dir == other or p.state_dir in other.parents or other in p.state_dir.parents: - raise ValueError(f"paths.state_dir {p.state_dir} overlaps {other}") - for k, v in vars(self.limits).items(): - if not (math.isfinite(v) and v > 0): - raise ValueError(f"limits.{k} must be finite and positive") + if getattr(github, attr): + setattr(github, attr, getattr(github, attr).expanduser()) + paths = self.paths + for section in (paths, self.harness): + for name, value in vars(section).items(): + if isinstance(value, Path): + setattr(section, name, value.expanduser().resolve()) + for other in (paths.work_root, paths.storage_root): # state_dir holds secrets and is never mounted + if paths.state_dir == other or paths.state_dir in other.parents or other in paths.state_dir.parents: + raise ValueError(f"paths.state_dir {paths.state_dir} overlaps {other}") + for name, value in vars(self.limits).items(): + if not (math.isfinite(value) and value > 0): + raise ValueError(f"limits.{name} must be finite and positive") if self.limits.max_stage_attempts != 1: raise ValueError("limits.max_stage_attempts: only 1 is supported (a failed job ends the round)") - for k in ("allow_hosts", "extra_mounts", "api_key_env"): # autodev's; mia-data's jobs never read them - if getattr(self.agent, k): - raise ValueError(f"agent.{k} is not used by mia-data (job hosts come from the binding and network.*, " + for name in ("allow_hosts", "extra_mounts", "api_key_env"): # autodev's; mia-data's jobs never read them + if getattr(self.agent, name): + raise ValueError(f"agent.{name} is not used by mia-data (job hosts come from the binding and network.*, " "mounts from the job builders); remove it") if not self.crops.licences: raise ValueError("crops.licences must not be empty") - n = self.network - for h in n.package_hosts + list(n.redirect_hosts) + [x for v in n.redirect_hosts.values() for x in v]: - if is_github_host(h): - raise ValueError(f"network: {h!r} is a GitHub host") - if bad := [x for v in n.redirect_hosts.values() for x in v if "*" in x]: + network = self.network + redirect_targets = [target for targets in network.redirect_hosts.values() for target in targets] + for host in network.package_hosts + list(network.redirect_hosts) + redirect_targets: + if is_github_host(host): + raise ValueError(f"network: {host!r} is a GitHub host") + if bad := [target for target in redirect_targets if "*" in target]: raise ValueError(f"network.redirect_hosts: {bad} must be exact hosts, no '*'") return self @@ -168,6 +178,7 @@ def task_queue(self) -> str: @property def await_queue(self) -> str: + """The task queue for the long-running job waits, kept apart from the short activities.""" return self.deployment + "-await" def label(self, stage: str) -> str: @@ -177,25 +188,26 @@ def workflow_id(self, issue: int) -> str: return f"mia-data/{self.github.data_repo}/issue-{issue}" def marker(self, text: str) -> str: + """The HTML comment hidden in our own GitHub comments so we can find them again.""" return f"" def load_config(path: str | os.PathLike | None = None) -> Config: - """Load config; exits with a readable message if it is missing or invalid.""" + """Load and validate the config file, exiting with a readable message if it is missing or invalid.""" load_dotenv() path = Path(path or os.environ.get("MIA_DATA_CONFIG") or PROJECT_DIR / "mia_data.yaml") if not path.is_file(): sys.exit(f"mia-data: config not found at {path} (copy mia_data.yaml.example, or set $MIA_DATA_CONFIG)") data = yaml.safe_load(path.read_text()) or {} - for env, keys in _ENV_OVERRIDES.items(): - if os.environ.get(env): - d = data - for k in keys[:-1]: - if d.get(k) is None: - d[k] = {} - d = d[k] - d[keys[-1]] = os.environ[env] + for env_var, key_path in _ENV_OVERRIDES.items(): + if os.environ.get(env_var): + node = data + for key in key_path[:-1]: + if node.get(key) is None: # a bare `temporal:` in YAML parses as None + node[key] = {} + node = node[key] + node[key_path[-1]] = os.environ[env_var] try: return Config(**data) - except ValidationError as e: - sys.exit(f"mia-data: invalid config {path}:\n{e}") + except ValidationError as err: + sys.exit(f"mia-data: invalid config {path}:\n{err}") diff --git a/mia_data/crop_test.py b/mia_data/crop_test.py index cf8ae05..fbd5f7c 100644 --- a/mia_data/crop_test.py +++ b/mia_data/crop_test.py @@ -1,40 +1,55 @@ -"""§7.4 test crops: `judge` (pure, shared with MiaDataDataset) and `run_crop_test`, the activity methods in -MiaDataDataset._crop's order without Temporal (used by `test-pr`). The orchestration is twice because the workflow -can't await activity methods directly and `test-pr` has no worker; what a result means lives once, in `judge`.""" +"""Judging a test-crop run, and running one outside Temporal. + +`judge` is the single definition of "the test crops passed", shared by the workflow and the `test-pr` +command. `run_crop_test` calls the activity methods in the same order as MiaDataDataset._crop but +synchronously, for `test-pr`, which has no worker. The orchestration exists twice because the workflow +cannot await activity methods directly; what a result means lives once, in `judge`. +""" from __future__ import annotations +from typing import TYPE_CHECKING, Any + from .models import GalleryResult, JobArgs, JobDone, Pins, VerifyResult, job_key +if TYPE_CHECKING: + from .activities import Activities + + +def job_succeeded(job: JobDone) -> bool: + return job.state == "DONE" and job.exit_code in (0, None) -def _ok(d: JobDone) -> bool: - return d.state == "DONE" and d.exit_code in (0, None) +def describe_failure(kind: str, job: JobDone) -> str: + return f"{kind} job {job.job_id} ended {job.state} (exit {job.exit_code})" -def _failed(kind: str, d: JobDone) -> str: - return f"{kind} job {d.job_id} ended {d.state} (exit {d.exit_code})" +def judge(verify: VerifyResult, jobs: list[JobDone]) -> tuple[bool, list[str]]: + """(passed, descriptions of failed jobs): passed when both jobs succeeded and every test crop verified.""" + failed = [describe_failure(kind, job) for kind, job in zip(("crop", "verify"), jobs) if not job_succeeded(job)] + return not failed and verify.ok_count > 0 and not verify.errors, failed -def judge(v: VerifyResult, jobs: list[JobDone]) -> tuple[bool, list[str]]: - """(passed, failed jobs): both jobs succeeded and every test crop verified.""" - bad = [_failed(k, j) for k, j in zip(("crop", "verify"), jobs) if not _ok(j)] - return not bad and v.ok_count > 0 and not v.errors, bad +async def run_crop_test(activities: Activities, *, issue: int, round_id: str, pins: Pins, sha: str, binding_path: str, + seed: str, trusted: bool) -> tuple[VerifyResult, GalleryResult | None, list[JobDone]]: + """crop (`crops.test_per_round`, batch 0) → verify (+ preview) → read_verify → publish_gallery, in the + round's crop- / verify- workspaces, waiting for each job in turn. -async def run_crop_test(acts, *, issue: int, round_id: str, pins: Pins, sha: str, binding_path: str, seed: str, - trusted: bool) -> tuple[VerifyResult, GalleryResult | None, list[JobDone]]: - """crop (`crops.test_per_round`, batch 0) → verify (+ preview) → read_verify → publish_gallery of the verified - crops, in workspaces crop- / verify- of the round, waiting synchronously. The jobs are returned, so - the caller judges them; an uncertain crop job skips verify.""" - async def job(kind: str, **kw) -> tuple[str, JobDone]: - ws = await acts.prepare_job(issue, round_id, f"{kind}-{sha}", 1, binding_path) - jid = await acts.submit(job_key(issue, round_id, f"{kind}-{sha}", 1), kind, JobArgs(ws, round_id, pins, **kw)) - o = await acts.jobs.wait(jid, ws, lambda _: None) - return ws, JobDone(o.job_id, o.state, o.exit_code) + Returns (verify result, gallery or None, the jobs) so the caller can `judge` them; an uncertain crop job + skips verify.""" + async def run_job(kind: str, **job_args: Any) -> tuple[str, JobDone]: + workspace = await activities.prepare_job(issue, round_id, f"{kind}-{sha}", 1, binding_path) + job_id = await activities.submit(job_key(issue, round_id, f"{kind}-{sha}", 1), kind, + JobArgs(workspace, round_id, pins, **job_args)) + outcome = await activities.jobs.wait(job_id, workspace, lambda _: None) + return workspace, JobDone(outcome.job_id, outcome.state, outcome.exit_code) - cw, c = await job("crop", sha=sha, n=acts.cfg.crops.test_per_round, seed=seed, trusted=trusted) - if c.state not in ("DONE", "EXIT"): - return VerifyResult(0, 0, [f"crop job {c.job_id} ended {c.state}"], []), None, [c] - vw, d = await job("verify", crop_ws=cw) - v = await acts.read_verify(vw, cw) - return v, await acts.publish_gallery(issue, round_id, sha, cw, vw, v.crops) if v.crops else None, [c, d] + crop_workspace, crop_job = await run_job("crop", sha=sha, n_crops=activities.cfg.crops.test_per_round, seed=seed, + trusted=trusted) + if crop_job.state not in ("DONE", "EXIT"): + return VerifyResult(0, 0, [f"crop job {crop_job.job_id} ended {crop_job.state}"], []), None, [crop_job] + verify_workspace, verify_job = await run_job("verify", crop_workspace=crop_workspace) + verify = await activities.read_verify(verify_workspace, crop_workspace) + gallery = (await activities.publish_gallery(issue, round_id, sha, crop_workspace, verify_workspace, verify.crops) + if verify.crops else None) + return verify, gallery, [crop_job, verify_job] diff --git a/mia_data/gallery.py b/mia_data/gallery.py index 17ac7f9..c39f44d 100644 --- a/mia_data/gallery.py +++ b/mia_data/gallery.py @@ -1,39 +1,47 @@ -"""Crops on the `galleries` branch of the data repo, embedded in comments (spec §7.4, §7.5).""" +"""Publishing crop thumbnails to the `galleries` branch of the data repo, so issue comments can show them.""" from __future__ import annotations import hashlib from pathlib import Path +from typing import TYPE_CHECKING from autodev.jobs import read_untrusted from .publish import MAX_FILE, check_crop_id +if TYPE_CHECKING: + from autodev.github import GitHub -def _read(ws: Path, rel: str) -> bytes: - if (data := read_untrusted(ws, rel, MAX_FILE, binary=True)) is None: - raise ValueError(f"{rel} is missing or unreadable in {ws}") + +def _read(workspace: Path, rel: str) -> bytes: + if (data := read_untrusted(workspace, rel, MAX_FILE, binary=True)) is None: + raise ValueError(f"{rel} is missing or unreadable in {workspace}") return data -def gallery_files(crop_ws: Path, verify_ws: Path, verified: list[dict]) -> dict[str, bytes]: - """/raw.png (crop job's crops/) and /overlay.png (verify job's preview/) for verified crops - only, plus the verify job's binding.yaml and verify.json. Job output: read bounded, no symlinks.""" +def gallery_files(crop_workspace: Path, verify_workspace: Path, verified: list[dict]) -> dict[str, bytes]: + """The files to publish, as relative path -> bytes: each verified crop's raw.png (from the crop job) and + overlay.png (from the verify job's preview), plus the verify job's binding.yaml and verify.json. + + `verified` is read_verify's crop list; a raw.png is published only if it still matches the verified checksum.""" out = {} - for v in verified: - cid = check_crop_id(v["crop_id"]) - raw = _read(crop_ws, f"crops/{cid}/raw.png") - if hashlib.sha256(raw).hexdigest() != v["files"]["raw.png"]: # publish only what verify checked - raise ValueError(f"{cid}/raw.png differs from the verified checksum") - out[f"{cid}/raw.png"], out[f"{cid}/overlay.png"] = raw, _read(verify_ws, f"preview/{cid}/overlay.png") - out["binding.yaml"], out["verify.json"] = _read(verify_ws, "binding.yaml"), _read(verify_ws, "verify.json") + for crop in verified: + crop_id = check_crop_id(crop["crop_id"]) + raw = _read(crop_workspace, f"crops/{crop_id}/raw.png") + if hashlib.sha256(raw).hexdigest() != crop["files"]["raw.png"]: + raise ValueError(f"{crop_id}/raw.png differs from the verified checksum") + out[f"{crop_id}/raw.png"] = raw + out[f"{crop_id}/overlay.png"] = _read(verify_workspace, f"preview/{crop_id}/overlay.png") + out["binding.yaml"], out["verify.json"] = _read(verify_workspace, "binding.yaml"), _read(verify_workspace, "verify.json") return out -async def publish_gallery(gh_data, branch: str, prefix: str, files: dict[str, bytes]) -> dict[str, str]: - """ensure_branch once; upsert_file for each prefix/ (existing files are skipped); returns - rel -> https://github.com//blob///?raw=true.""" - # ponytail: PNGs on a branch are fine for a demo; move to real hosting (§15) before it reaches a few hundred MB +async def publish_gallery(gh_data: GitHub, branch: str, prefix: str, files: dict[str, bytes]) -> dict[str, str]: + """Commit `files` under `prefix` on the gallery branch (files already there are left alone). + + Returns rel -> https://github.com//blob///?raw=true, the URLs markdown can embed.""" + # ponytail: PNGs on a branch are fine for a demo; move to real hosting before it reaches a few hundred MB await gh_data.ensure_branch(branch) for rel, data in files.items(): await gh_data.upsert_file(f"{prefix}/{rel}", data, branch, f"mia-data: {prefix}") diff --git a/mia_data/jobspec.py b/mia_data/jobspec.py index 84e7a2f..7d8fc4d 100644 --- a/mia_data/jobspec.py +++ b/mia_data/jobspec.py @@ -1,10 +1,12 @@ -"""Job specs (match, smoke, crop, verify), snippet snapshots at a SHA, and the harness pin (spec §6, §7.2, §8.1, §9). +"""Building the csub job specs for the four job kinds (match, smoke, crop, verify), plus the two things +every job needs: a read-only snapshot of mia-snippets at a commit, and the harness pin. -The record and bindings live under `paths.state_dir`, which is never mounted into a job: the caller copies them -into the job's workspace first, and the builders take their names relative to it (`record.yaml`, `binding.yaml`). -The snapshot, the harness checkout, the dataset cache and any extra input are mounted read-only; only the workspace -(and the dataset cache, for jobs that run reviewed code) is writable. Every builder refuses (PinMoved) when the -harness checkout is no longer the round's admitted pin, and gives the job the admitted pin values.""" +The record and bindings live under `paths.state_dir`, which is never mounted into a job: the caller copies +them into the job's workspace first, and the builders take their names relative to it (`record.yaml`, +`binding.yaml`). The snapshot, the harness checkout, the dataset cache and any extra input are mounted +read-only; only the workspace (and the dataset cache, for jobs that run reviewed code) is writable. Every +builder refuses (PinMoved) when the harness checkout is no longer the round's admitted pin. +""" from __future__ import annotations @@ -18,6 +20,7 @@ from importlib.resources import files from pathlib import Path from string import Template +from typing import TYPE_CHECKING, Any, Callable import yaml @@ -27,11 +30,15 @@ from .binding import hosts, missing_hosts, record_urls from .seed import ID_RE +if TYPE_CHECKING: + from .config import Config + from .models import Pins + SHA_RE = re.compile(r"[0-9a-f]{40}") MATCH_TOOLS = "Bash(harness smoke:*),Read,Write,Glob,Grep" HARNESS_ENV = ("MIA_HARNESS_JOB_BYTES", "MIA_HARNESS_COMMIT", "MIA_HARNESS_ENV_DIGEST", "MIA_HARNESS_ALLOWED_HOSTS", "MIA_HARNESS_CACHE_DIR", "MIA_HARNESS_CACHE_WRITABLE") # every job's env; csub's env_allow must cover it -q = shlex.quote +quote = shlex.quote class PinMoved(RuntimeError): @@ -39,7 +46,7 @@ class PinMoved(RuntimeError): class HostsNotAllowed(RuntimeError): - """A job needs hosts the csub policy doesn't allow. Not retryable: the fix is a broker.toml edit (§9).""" + """A job needs hosts the csub policy doesn't allow. Not retryable: the fix is a broker.toml edit.""" def __init__(self, missing: list[str]): self.missing = missing @@ -47,16 +54,16 @@ def __init__(self, missing: list[str]): "(add them to broker.toml, and to network.redirect_hosts if they are redirect targets)") -def _snap_dir(cfg, sha: str) -> Path: +def _snapshot_dir(cfg: Config, sha: str) -> Path: return cfg.paths.work_root / "_snapshots" / sha -def snapshot(cfg, sha: str, tokens=lambda: None) -> Path: +def snapshot(cfg: Config, sha: str, tokens: Callable[[], str | None] = lambda: None) -> Path: """`snippets/`, `WITHDRAWN.yaml` and `SPEC.md` of mia-snippets at `sha`, extracted once and read-only, at `work_root/_snapshots/`. The bare mirror in `state_dir` is fetched only when it lacks `sha`.""" if not SHA_RE.fullmatch(sha): raise ValueError(f"not a full commit SHA: {sha!r}") - dest = _snap_dir(cfg, sha) + dest = _snapshot_dir(cfg, sha) if dest.exists(): return dest dest.parent.mkdir(parents=True, exist_ok=True) @@ -64,41 +71,43 @@ def snapshot(cfg, sha: str, tokens=lambda: None) -> Path: fcntl.flock(lock, fcntl.LOCK_EX) if dest.exists(): return dest - g = cfg.github - m = Mirror(cfg.paths.state_dir / "mirror" / "mia-snippets.git", - g.snippets_git_url or f"https://github.com/{g.snippets_repo}.git", tokens) - if not (m.path / "HEAD").exists() or subprocess.run( - ["git", "-C", str(m.path), "cat-file", "-e", f"{sha}^{{commit}}"], capture_output=True).returncode: - m.sync() + github = cfg.github + mirror = Mirror(cfg.paths.state_dir / "mirror" / "mia-snippets.git", + github.snippets_git_url or f"https://github.com/{github.snippets_repo}.git", tokens) + if not (mirror.path / "HEAD").exists() or subprocess.run( + ["git", "-C", str(mirror.path), "cat-file", "-e", f"{sha}^{{commit}}"], capture_output=True).returncode: + mirror.sync() tmp = dest.with_name(f".{sha}.tmp") if tmp.exists(): # left by a crash; read-only, like a finished one subprocess.run(["chmod", "-R", "u+w", str(tmp)], check=True) shutil.rmtree(tmp) tmp.mkdir() # no LFS smudge (or any global/system filter) from a PR head's .gitattributes - r = subprocess.run(["git", "-c", "core.hooksPath=/dev/null", "-c", "filter.lfs.smudge=", - "-c", "filter.lfs.process=", "-c", "filter.lfs.required=false", "-C", str(m.path), - "archive", sha, "snippets", "WITHDRAWN.yaml", "SPEC.md"], capture_output=True, - env={**os.environ, "GIT_CONFIG_GLOBAL": "/dev/null", "GIT_CONFIG_NOSYSTEM": "1"}) - if r.returncode: - raise GitError(f"git archive {sha}: {r.stderr.decode(errors='replace').strip()[-2000:]}") - subprocess.run(["tar", "-x", "-C", str(tmp)], input=r.stdout, check=True) + archive = subprocess.run(["git", "-c", "core.hooksPath=/dev/null", "-c", "filter.lfs.smudge=", + "-c", "filter.lfs.process=", "-c", "filter.lfs.required=false", "-C", str(mirror.path), + "archive", sha, "snippets", "WITHDRAWN.yaml", "SPEC.md"], capture_output=True, + env={**os.environ, "GIT_CONFIG_GLOBAL": "/dev/null", "GIT_CONFIG_NOSYSTEM": "1"}) + if archive.returncode: + raise GitError(f"git archive {sha}: {archive.stderr.decode(errors='replace').strip()[-2000:]}") + subprocess.run(["tar", "-x", "-C", str(tmp)], input=archive.stdout, check=True) subprocess.run(["chmod", "-R", "a-w", str(tmp)], check=True) os.rename(tmp, dest) return dest -def harness_pin(cfg) -> tuple[str, str]: +def harness_pin(cfg: Config) -> tuple[str, str]: """(commit, env digest) of the `harness.dir` checkout: HEAD and the sha256 of harness/pixi.lock.""" - h = cfg.harness.dir - return git("rev-parse", "HEAD", cwd=h), hashlib.sha256((h / "harness" / "pixi.lock").read_bytes()).hexdigest() + harness_dir = cfg.harness.dir + return (git("rev-parse", "HEAD", cwd=harness_dir), + hashlib.sha256((harness_dir / "harness" / "pixi.lock").read_bytes()).hexdigest()) -def assert_pin(cfg, pins) -> None: - now = harness_pin(cfg) - if now != (pins.harness_commit, pins.env_digest): +def assert_pin(cfg: Config, pins: Pins) -> None: + """Raise PinMoved unless the harness checkout is still exactly what the round was admitted with.""" + current = harness_pin(cfg) + if current != (pins.harness_commit, pins.env_digest): raise PinMoved(f"harness checkout {cfg.harness.dir} moved since admission: commit {pins.harness_commit[:12]} " - f"-> {now[0][:12]}, pixi.lock {pins.env_digest[:12]} -> {now[1][:12]}") + f"-> {current[0][:12]}, pixi.lock {pins.env_digest[:12]} -> {current[1][:12]}") def check_hosts_allowed(spec: JobSpec, patterns: list[str]) -> None: @@ -107,91 +116,108 @@ def check_hosts_allowed(spec: JobSpec, patterns: list[str]) -> None: raise HostsNotAllowed(missing) -def match_prompt(pins) -> str: +def match_prompt(pins: Pins) -> str: + """The match agent's prompt (prompts/match.md) with the round's pins filled in.""" return Template(files("mia_data").joinpath("prompts/match.md").read_text()).substitute( record=pins.record, record_commit=pins.record_commit, snippets_commit=pins.snippets_commit) -def harness_bin(cfg) -> Path: +def harness_bin(cfg: Config) -> Path: """The harness environment's entry point. Run directly: `pixi run` wants the install lock on the prefix, and `harness.dir` is mounted read-only.""" return cfg.harness.dir / "harness" / ".pixi" / "envs" / "default" / "bin" / "harness" -def _harness(cfg) -> str: - return q(str(harness_bin(cfg))) +def _harness(cfg: Config) -> str: + return quote(str(harness_bin(cfg))) -def _binding(ws, rel: str) -> tuple[str, list[str]]: +def _binding(workspace: Path, rel: str) -> tuple[str, list[str]]: """(record id, URLs) of a binding the worker has already checked (check_binding).""" - b = yaml.safe_load(Path(ws, rel).read_text()) - return b.get("record"), [u for a in (b.get("raw") or []) + (b.get("labels") or []) for u in a.get("urls") or []] + binding = yaml.safe_load(Path(workspace, rel).read_text()) + return binding.get("record"), [url for array in (binding.get("raw") or []) + (binding.get("labels") or []) + for url in array.get("urls") or []] -def _parent(f): +def _parent(file: Path | None) -> Path | None: """The directory to mount for an input file (csub mounts directories only).""" - return Path(f).parent if f else None + return Path(file).parent if file else None + +def _spec(cfg: Config, kind: str, workspace: Path, pins: Pins, body: str, allow_hosts: list[str], + ro_inputs: list[Path | None], record: str | None = None, writable: bool = False, **kwargs: Any) -> JobSpec: + """The JobSpec shared by every kind: harness env vars, mounts, and the wrapper that records the exit code. -def _spec(cfg, kind: str, ws, pins, body: str, allow: list[str], ro: list, record: str | None = None, - writable: bool = False, **kw) -> JobSpec: + `record` enables the per-dataset download cache (read-write when `writable`); `ro_inputs` are extra + read-only mounts (None entries are skipped).""" assert_pin(cfg, pins) # the admitted harness, or no job env = {"MIA_HARNESS_JOB_BYTES": str(int(cfg.limits.job_bytes)), "MIA_HARNESS_COMMIT": pins.harness_commit, - "MIA_HARNESS_ENV_DIGEST": pins.env_digest, "MIA_HARNESS_ALLOWED_HOSTS": ",".join(allow), + "MIA_HARNESS_ENV_DIGEST": pins.env_digest, "MIA_HARNESS_ALLOWED_HOSTS": ",".join(allow_hosts), "MIA_HARNESS_CACHE_DIR": "", "MIA_HARNESS_CACHE_WRITABLE": "0"} - ro, rw = [str(cfg.harness.dir), *(str(p) for p in ro if p)], [] - if record is not None: # the dataset cache, §6.6 + ro_mounts = [str(cfg.harness.dir), *(str(path) for path in ro_inputs if path)] + rw_mounts: list[str] = [] + if record is not None: # the dataset cache if not (isinstance(record, str) and ID_RE.fullmatch(record)): raise ValueError(f"bad record id {record!r}") cache = cfg.paths.work_root / "_cache" / record cache.mkdir(parents=True, exist_ok=True) # a read-only mount must exist - (rw if writable else ro).append(str(cache)) + (rw_mounts if writable else ro_mounts).append(str(cache)) env.update(MIA_HARNESS_CACHE_DIR=str(cache), MIA_HARNESS_CACHE_WRITABLE="1" if writable else "0") - path = f'export PATH={":".join(q(p) for p in cfg.agent.path_prepend)}:"$PATH"\n' if cfg.agent.path_prepend else "" + path = (f'export PATH={":".join(quote(entry) for entry in cfg.agent.path_prepend)}:"$PATH"\n' + if cfg.agent.path_prepend else "") # ponytail: plain jobs take JobSpec's 1 cpu / 4000 MB; add limits.job_cpus / job_mem_mb if crops need more. - return JobSpec(name=f"{cfg.deployment}-{kind}", command=f'set -u\n{path}{body}\nrc=$?\necho "$rc" > exit_code\nexit "$rc"\n', - workspace=str(ws), walltime_min=cfg.limits.job_walltime_min, allow_hosts=allow, ro_mounts=ro, - rw_mounts=rw, env=env, **kw) + return JobSpec(name=f"{cfg.deployment}-{kind}", + command=f'set -u\n{path}{body}\nrc=$?\necho "$rc" > exit_code\nexit "$rc"\n', + workspace=str(workspace), walltime_min=cfg.limits.job_walltime_min, allow_hosts=allow_hosts, + ro_mounts=ro_mounts, rw_mounts=rw_mounts, env=env, **kwargs) -def match_job(cfg, ws, pins, record_path: str = "record.yaml") -> JobSpec: - """The match agent (§7.2). The caller has written `prompt.md` (match_prompt), `record.yaml`, `downloads.json`, - any `conversion.json` / `manifest.json`, `catalog/` and `SPEC.md` into `ws`, and made the snapshot of +def match_job(cfg: Config, workspace: Path, pins: Pins, record_path: str = "record.yaml") -> JobSpec: + """The match agent: Claude Code with the harness's `smoke` command as its only tool besides reading. + + The caller has written `prompt.md` (match_prompt), `record.yaml`, `downloads.json`, any `conversion.json` + / `manifest.json`, `catalog/` and `SPEC.md` into the workspace, and made the snapshot of `pins.snippets_commit`. Anthropic hosts come from the csub policy's [agents] table, never from here.""" - rec = yaml.safe_load(Path(ws, record_path).read_text()) or {} - snap = _snap_dir(cfg, pins.snippets_commit) - body = (f"ln -sfn {q(str(snap))} snapshot && mkdir -p bin && \\\n" - f"printf '#!/bin/sh\\nexec %s \"$@\"\\n' {q(_harness(cfg))} > bin/harness && chmod +x bin/harness && \\\n" - f'PATH="$PWD/bin:$PATH" claude -p "$(cat prompt.md)" --model {q(cfg.agent.model)} ' + record = yaml.safe_load(Path(workspace, record_path).read_text()) or {} + snapshot_dir = _snapshot_dir(cfg, pins.snippets_commit) + body = (f"ln -sfn {quote(str(snapshot_dir))} snapshot && mkdir -p bin && \\\n" + f"printf '#!/bin/sh\\nexec %s \"$@\"\\n' {quote(_harness(cfg))} > bin/harness && chmod +x bin/harness && \\\n" + f'PATH="$PWD/bin:$PATH" claude -p "$(cat prompt.md)" --model {quote(cfg.agent.model)} ' f"--max-budget-usd {cfg.limits.agent_max_budget_usd} --output-format json \\\n" f' --allowedTools "{MATCH_TOOLS}" > claude.json 2> claude.err') - return _spec(cfg, "match", ws, pins, body, hosts(record_urls(rec), cfg), [snap], + return _spec(cfg, "match", workspace, pins, body, hosts(record_urls(record), cfg), [snapshot_dir], record=pins.record, agent="claude", cpus=cfg.agent.cpus, mem_mb=cfg.agent.mem_mb) -def smoke_job(cfg, ws, pins, binding_path: str, snapshot, record_path: str) -> JobSpec: - """The worker's own smoke of a binding, so the agent's claim is checked (§7.2). Reviewed code: cache rw.""" - record, urls = _binding(ws, binding_path) - body = f"{_harness(cfg)} smoke {q(binding_path)} --snippets {q(str(snapshot))} --record {q(record_path)} --out smoke.json" - return _spec(cfg, "smoke", ws, pins, body, hosts(urls, cfg), [snapshot], record=record, writable=True) +def smoke_job(cfg: Config, workspace: Path, pins: Pins, binding_path: str, snapshot: Path, + record_path: str) -> JobSpec: + """The worker's own smoke of a binding, so the agent's claim is checked. Reviewed code: cache read-write.""" + record, urls = _binding(workspace, binding_path) + body = (f"{_harness(cfg)} smoke {quote(binding_path)} --snippets {quote(str(snapshot))} " + f"--record {quote(record_path)} --out smoke.json") + return _spec(cfg, "smoke", workspace, pins, body, hosts(urls, cfg), [snapshot], record=record, writable=True) + +def crop_job(cfg: Config, workspace: Path, pins: Pins, binding_path: str, snapshot: Path, record_path: str, + n_crops: int, seed: str, round_id: str, batch: int, existing: Path | None, trusted: bool) -> JobSpec: + """`harness crop` into `/crops`. -def crop_job(cfg, ws, pins, binding_path: str, snapshot, record_path: str, n: int, seed: str, round_id: str, batch: int, - existing, trusted: bool) -> JobSpec: - """`harness crop` into `ws/crops` (§7.4, §7.5). `trusted` (a matched or merged SHA, or production) mounts the - dataset cache read-write; a PR head gets it read-only and keeps a private cache (§6.6).""" - record, urls = _binding(ws, binding_path) - body = (f"{_harness(cfg)} crop {q(binding_path)} --snippets {q(str(snapshot))} --record {q(record_path)} " - f"--n {int(n)} --seed {q(seed)} --round {q(round_id)} --batch {int(batch)} --out crops" - + (f" --existing {q(str(existing))}" if existing else "")) - return _spec(cfg, "crop", ws, pins, body, hosts(urls, cfg), [snapshot, _parent(existing)], record=record, + `trusted` (a matched or merged SHA, or production) mounts the dataset cache read-write; a PR head gets it + read-only and keeps a private cache. `existing` is a published manifest.json to avoid duplicating.""" + record, urls = _binding(workspace, binding_path) + body = (f"{_harness(cfg)} crop {quote(binding_path)} --snippets {quote(str(snapshot))} --record {quote(record_path)} " + f"--n {int(n_crops)} --seed {quote(seed)} --round {quote(round_id)} --batch {int(batch)} --out crops" + + (f" --existing {quote(str(existing))}" if existing else "")) + return _spec(cfg, "crop", workspace, pins, body, hosts(urls, cfg), [snapshot, _parent(existing)], record=record, writable=trusted) -def verify_job(cfg, ws, pins, crops_dir, binding_path: str, existing) -> JobSpec: - """`harness verify` then `harness preview` on a crop job's output, mounted read-only (§6.4, §6.7). Loads no - snippet code: no network, no dataset cache.""" - h, d = _harness(cfg), q(str(crops_dir)) - body = (f"{h} verify {d} --binding {q(binding_path)}" + (f" --existing {q(str(existing))}" if existing else "") - + f" --out verify.json; v=$?\n{h} preview {d} --out preview && (exit $v)") # overlays even if a crop fails - return _spec(cfg, "verify", ws, pins, body, [], [crops_dir, _parent(existing)]) +def verify_job(cfg: Config, workspace: Path, pins: Pins, crops_dir: Path, binding_path: str, + existing: Path | None) -> JobSpec: + """`harness verify` then `harness preview` on a crop job's output, mounted read-only. Loads no snippet + code: no network, no dataset cache.""" + harness, crops = _harness(cfg), quote(str(crops_dir)) + body = (f"{harness} verify {crops} --binding {quote(binding_path)}" + + (f" --existing {quote(str(existing))}" if existing else "") + + f" --out verify.json; v=$?\n{harness} preview {crops} --out preview && (exit $v)") # overlays even if a crop fails + return _spec(cfg, "verify", workspace, pins, body, [], [crops_dir, _parent(existing)]) diff --git a/mia_data/models.py b/mia_data/models.py index b5d9f3c..06b584d 100644 --- a/mia_data/models.py +++ b/mia_data/models.py @@ -1,4 +1,8 @@ -"""Plain dataclasses, JSON-serialisable for Temporal.""" +"""Dataclasses passed between the mia-data workflow and its activities. + +Temporal serializes them as JSON, so they hold only strings, numbers, bools, lists and dicts. Job +reports are reduced to these small, typed results before they enter workflow history. +""" from __future__ import annotations @@ -7,6 +11,7 @@ @dataclass class LabelEvent: + """A human adding `data:go` or `data:approve` to a dataset issue, delivered to the workflow as a signal.""" kind: str # go | approve event_id: str actor: str @@ -15,6 +20,7 @@ class LabelEvent: @dataclass class PrUpdate: + """The current state of a snippet PR in mia-snippets, as the poller or the pr_state activity reads it.""" pr: int head_sha: str state: str @@ -26,6 +32,9 @@ class PrUpdate: @dataclass class Pins: + """What a round is pinned to at admission: the record, the snippets commit and the harness version. + + A round never moves off its pins, so every job in it sees the same inputs.""" record: str record_commit: str snippets_commit: str @@ -42,6 +51,7 @@ class JobDone: @dataclass class Test: + """The test crops run against one snippets commit: whether it passed and where its gallery is.""" __test__ = False # not a pytest class sha: str test_id: str @@ -53,6 +63,9 @@ class Test: @dataclass class DatasetState: + """The durable state of one dataset issue's workflow, carried across continue_as_new. + + Most fields describe the current round and are reset when a new `go` starts one.""" issue: int schema: int = 1 handled: list[str] = field(default_factory=list) # go + approve event ids, newest last; keep <= 50 @@ -67,7 +80,7 @@ class DatasetState: pr: int | None = None tests: list[Test] = field(default_factory=list) attempts: dict[str, int] = field(default_factory=dict) # stage -> attempts used - pending: list[PrUpdate] = field(default_factory=list) + pending: list[PrUpdate] = field(default_factory=list) # PR updates not yet acted on def job_key(issue: int, round_id: str, kind: str, attempt: int) -> str: @@ -77,20 +90,22 @@ def job_key(issue: int, round_id: str, kind: str, attempt: int) -> str: @dataclass class JobArgs: - ws: str # from prepare_match / prepare_job + """Arguments of the `submit` activity. Which fields matter depends on the job kind.""" + workspace: str # from prepare_match / prepare_job round_id: str pins: Pins sha: str | None = None # smoke/crop: mia-snippets commit to load; default pins.snippets_commit - n: int = 0 # crop + n_crops: int = 0 # crop: how many crops to make seed: str = "" batch: int = 0 trusted: bool = False # crop: reviewed code, dataset cache read-write existing: bool = False # crop/verify: --existing /manifest.json, when there is one - crop_ws: str | None = None # verify: the crop job's workspace + crop_workspace: str | None = None # verify: the crop job's workspace @dataclass class MatchResult: + """What the match agent concluded about the dataset, after the worker checked its binding.""" outcome: str # matched | needs_snippet | ineligible | blocked (blocked = needs-human, with the reason) reason: str = "" binding_path: str | None = None # matched/ineligible: the checked binding, under state_dir @@ -102,6 +117,7 @@ class MatchResult: @dataclass class SmokeResult: + """The worker's own quick check of a binding: can the data be opened, and is it big enough to crop?""" ok: bool eligible: bool = False shapes: dict | None = None @@ -113,8 +129,9 @@ class SmokeResult: @dataclass class VerifyResult: + """The verify job's report, reduced to the crops that passed.""" ok_count: int - n: int + n: int # how many crops verify was asked to check (named `n` on the wire: it is in recorded histories) errors: list[str] crops: list[dict] # verified crops only: [{"crop_id", "files": {rel: sha256}}] n_effective: int | None = None # the crop job's n, lowered by capacity (its report.json); None if unknown @@ -134,4 +151,4 @@ class RoundLimits: class GalleryResult: prefix: str urls: dict[str, str] # rel -> ?raw=true URL - table: str # render_crops_table + table: str # the markdown table from render_crops_table diff --git a/mia_data/publish.py b/mia_data/publish.py index 62d0435..8114b96 100644 --- a/mia_data/publish.py +++ b/mia_data/publish.py @@ -1,4 +1,9 @@ -"""Idempotent publication of verified crops into the dataset folder (spec §6.3 Placement, §7.6, §5.6).""" +"""Copying verified crops into the dataset folder under storage_root, and keeping its manifest.json current. + +Publication is idempotent and safe against a second attempt running at the same time: each crop is staged +in a private temp directory, re-hashed, and renamed into place; a crop that already exists with the same +hashes counts as published, one with different hashes is a Conflict for a human. +""" from __future__ import annotations @@ -11,6 +16,7 @@ import time import uuid from pathlib import Path, PurePosixPath +from typing import Any from autodev.jobs import read_untrusted @@ -22,84 +28,93 @@ class Conflict(RuntimeError): """A crop exists with different checksums (or its source changed since verify): needs a human.""" -def check_crop_id(cid) -> str: - if not (isinstance(cid, str) and CROP_ID_RE.fullmatch(cid)): - raise ValueError(f"bad crop_id {cid!r}") - return cid +def check_crop_id(crop_id: Any) -> str: + if not (isinstance(crop_id, str) and CROP_ID_RE.fullmatch(crop_id)): + raise ValueError(f"bad crop_id {crop_id!r}") + return crop_id -def _hashes(d: Path) -> dict[str, str]: - return {p.relative_to(d).as_posix(): hashlib.sha256(p.read_bytes()).hexdigest() for p in d.rglob("*") if p.is_file()} +def _hashes(directory: Path) -> dict[str, str]: + """relative path -> sha256 of every file under `directory`.""" + return {path.relative_to(directory).as_posix(): hashlib.sha256(path.read_bytes()).hexdigest() + for path in directory.rglob("*") if path.is_file()} -def _same(dest: Path, files: dict, cid: str) -> None: +def _assert_same(dest: Path, files: dict, crop_id: str) -> None: if _hashes(dest) != files: - raise Conflict(f"{cid} is already published with different checksums") - - -def publish(dataset_dir: Path, src_dir: Path, verified: list[dict], withdrawn: dict[str, str], *, tmp_max_age_s: float) -> dict: - """verified = [{"crop_id", "files": {rel: sha256}}]. Copies only the verified files of each crop into a - per-attempt `.tmp--`, re-hashes, and renames it into place; an existing crop with the same - hashes counts as already published. Then rebuilds manifest.json. Returns {"published", "already", "total"}.""" - for v in verified: - check_crop_id(v["crop_id"]) - if bad := [r for r in v["files"] if not r or PurePosixPath(r).is_absolute() or ".." in PurePosixPath(r).parts]: - raise ValueError(f"{v['crop_id']}: bad file paths {bad}") - if not {"crop.json", "raw.png"} <= set(v["files"]): - raise ValueError(f"{v['crop_id']}: crop.json and raw.png must be verified") + raise Conflict(f"{crop_id} is already published with different checksums") + + +def publish(dataset_dir: Path, src_dir: Path, verified: list[dict], withdrawn: dict[str, str], *, + tmp_max_age_s: float) -> dict: + """Copy the verified files of each crop from `src_dir` (a crop job's crops/) into `dataset_dir`. + + `verified` is [{"crop_id", "files": {rel: sha256}}] from read_verify; only those files are copied, and + they must still hash the same. Temp directories older than `tmp_max_age_s` are left by dead attempts and + are cleaned up. Returns {"published": [ids], "already": [ids], "total": count in the dataset}.""" + for crop in verified: + check_crop_id(crop["crop_id"]) + if bad := [rel for rel in crop["files"] + if not rel or PurePosixPath(rel).is_absolute() or ".." in PurePosixPath(rel).parts]: + raise ValueError(f"{crop['crop_id']}: bad file paths {bad}") + if not {"crop.json", "raw.png"} <= set(crop["files"]): + raise ValueError(f"{crop['crop_id']}: crop.json and raw.png must be verified") dataset_dir.mkdir(parents=True, exist_ok=True) now = time.time() - for t in dataset_dir.glob(".tmp-*"): # younger ones may belong to a live attempt + for stale in dataset_dir.glob(".tmp-*"): # younger ones may belong to a live attempt try: - if now - t.lstat().st_mtime > tmp_max_age_s: - shutil.rmtree(t) if t.is_dir() else t.unlink() + if now - stale.lstat().st_mtime > tmp_max_age_s: + shutil.rmtree(stale) if stale.is_dir() else stale.unlink() except FileNotFoundError: # another attempt got there first pass published, already = [], [] try: - for v in verified: - cid, files, dest = v["crop_id"], v["files"], dataset_dir / v["crop_id"] + for crop in verified: + crop_id, files, dest = crop["crop_id"], crop["files"], dataset_dir / crop["crop_id"] if dest.exists(): - _same(dest, files, cid) - already.append(cid) + _assert_same(dest, files, crop_id) + already.append(crop_id) continue - tmp = dataset_dir / f".tmp-{cid}-{uuid.uuid4().hex}" + tmp = dataset_dir / f".tmp-{crop_id}-{uuid.uuid4().hex}" for rel in files: - data = read_untrusted(src_dir, f"{cid}/{rel}", MAX_FILE, binary=True) # job output: no symlinks + data = read_untrusted(src_dir, f"{crop_id}/{rel}", MAX_FILE, binary=True) # job output: no symlinks if data is None: shutil.rmtree(tmp, ignore_errors=True) - raise Conflict(f"{cid}/{rel} is missing or unreadable in the crop job's output") + raise Conflict(f"{crop_id}/{rel} is missing or unreadable in the crop job's output") (tmp / rel).parent.mkdir(parents=True, exist_ok=True) (tmp / rel).write_bytes(data) if _hashes(tmp) != files: shutil.rmtree(tmp) - raise Conflict(f"{cid}: source files differ from the verified checksums") + raise Conflict(f"{crop_id}: source files differ from the verified checksums") try: os.rename(tmp, dest) - published.append(cid) - except OSError as e: # another attempt renamed first - if e.errno not in (errno.EEXIST, errno.ENOTEMPTY): + published.append(crop_id) + except OSError as error: # another attempt renamed first + if error.errno not in (errno.EEXIST, errno.ENOTEMPTY): raise shutil.rmtree(tmp) - _same(dest, files, cid) - already.append(cid) + _assert_same(dest, files, crop_id) + already.append(crop_id) finally: # a Conflict leaves the manifest matching what is published total = reindex(dataset_dir, withdrawn) return {"published": published, "already": already, "total": total} def reindex(dataset_dir: Path, withdrawn: dict[str, str]) -> int: - """Rebuild manifest.json from */crop.json (dot-dirs ignored), marking crops of withdrawn snippets. - crop.json["snippets"] is [{"name", "commit"}]. Each entry carries "files" (rel -> sha256), which harness verify - --existing reads for its duplicate-checksum check. Returns the crop count.""" + """Rebuild manifest.json from every /crop.json (dot-dirs ignored). Returns the crop count. + + `withdrawn` maps snippet name -> reason; crops made with a withdrawn snippet are marked, not removed. + Each manifest entry carries "files" (rel -> sha256), which `harness verify --existing` reads for its + duplicate-checksum check.""" crops = [] - for d in sorted(p for p in dataset_dir.iterdir() if p.is_dir() and not p.name.startswith(".")): - if not (d / "crop.json").is_file(): + for crop_dir in sorted(path for path in dataset_dir.iterdir() if path.is_dir() and not path.name.startswith(".")): + if not (crop_dir / "crop.json").is_file(): continue - cj = json.loads((d / "crop.json").read_text()) - hit = next((s["name"] for s in cj.get("snippets") or [] if s.get("name") in withdrawn), None) - crops.append({**cj, "crop_id": d.name, "files": _hashes(d), "withdrawn": hit is not None, - "withdrawn_reason": withdrawn.get(hit)}) + crop_json = json.loads((crop_dir / "crop.json").read_text()) + withdrawn_snippet = next((snippet["name"] for snippet in crop_json.get("snippets") or [] + if snippet.get("name") in withdrawn), None) + crops.append({**crop_json, "crop_id": crop_dir.name, "files": _hashes(crop_dir), + "withdrawn": withdrawn_snippet is not None, "withdrawn_reason": withdrawn.get(withdrawn_snippet)}) tmp = dataset_dir / f".tmp-manifest-{uuid.uuid4().hex}.json" # per writer: a zombie attempt can't interleave tmp.write_text(json.dumps({"crops": crops, "count": len(crops)}, indent=1)) os.replace(tmp, dataset_dir / "manifest.json") diff --git a/mia_data/seed.py b/mia_data/seed.py index 6c4c428..74afb8a 100644 --- a/mia_data/seed.py +++ b/mia_data/seed.py @@ -1,12 +1,23 @@ -"""`mia-data seed`: one data-repo issue per downloaded dataset (spec §7.1).""" +"""`mia-data seed`: create one issue in the data repo for every dataset the catalog has downloaded. + +The issue body starts with a hidden marker naming the record id, which is how every later step finds +the record for an issue, and it carries labels for the licence, modality and size so humans can pick +datasets to `go`. +""" from __future__ import annotations import logging import re +from typing import TYPE_CHECKING from . import catalog -from .status import _c +from .status import safe_text + +if TYPE_CHECKING: + from autodev.github import GitHub + + from .config import Config log = logging.getLogger(__name__) MARKER = "" @@ -15,62 +26,83 @@ ID_RE = re.compile(r"[a-z0-9][a-z0-9._-]{0,120}") -def render_issue(rec: dict, entry: dict, licences: list[str], repo: str = "", path: str = "", id_: str | None = None) -> tuple[str, str, list[str]]: - id_, img, spdx = id_ or rec["id"], rec.get("imaging") or {}, (rec.get("license") or {}).get("spdx") - arrays = (rec.get("technical") or {}).get("arrays") or [] - family = (img.get("modality") or ["unknown"])[0] +def render_issue(record: dict, entry: dict, licences: list[str], repo: str = "", path: str = "", + record_id: str | None = None) -> tuple[str, str, list[str]]: + """Build (title, body, labels) for a dataset issue. + + `record` is the catalog's YAML record, `entry` its downloads-state entry (dataset_dir, record_commit, + bytes_on_disk, downloaders), `repo`/`path` locate the record on GitHub for links.""" + record_id = record_id or record["id"] + imaging, spdx = record.get("imaging") or {}, (record.get("license") or {}).get("spdx") + arrays = (record.get("technical") or {}).get("arrays") or [] + family = (imaging.get("modality") or ["unknown"])[0] labels = ["data:new" if spdx in licences else "data:ineligible", f"modality:{family}"] if spdx not in licences: labels.append("licence-excluded") - if any(a["role"] == "label" for a in arrays): + if any(array["role"] == "label" for array in arrays): labels.append("has-labels") - def yx(a): - s, ax = a["shape"], a.get("axes") or "" - return (s[ax.index("y")], s[ax.index("x")]) if {"y", "x"} <= set(ax) and len(ax) == len(s) else tuple(s[-2:]) - raws = [a for a in arrays if a["role"] == "raw" and a.get("shape")] - if not any(len(a["shape"]) >= 2 and min(yx(a)) >= 256 for a in raws): + def yx_size(array: dict) -> tuple: + """The (y, x) extent of an array, from its axes when known, else its last two dimensions.""" + shape, axes = array["shape"], array.get("axes") or "" + if {"y", "x"} <= set(axes) and len(axes) == len(shape): + return shape[axes.index("y")], shape[axes.index("x")] + return tuple(shape[-2:]) + + raws = [array for array in arrays if array["role"] == "raw" and array.get("shape")] + if not any(len(array["shape"]) >= 2 and min(yx_size(array)) >= 256 for array in raws): labels.append("maybe-small") - base = f"https://github.com/{repo}/blob" - vox = img.get("voxel_size_nm") or {} - data = rec.get("data") or {} + base_url = f"https://github.com/{repo}/blob" + voxel = imaging.get("voxel_size_nm") or {} + data = record.get("data") or {} urls = data.get("download_url") or [] - lines = [MARKER.format(id_), "", - f"- Record: [main]({base}/main/{path}), [at `{entry['record_commit'][:12]}`]({base}/{entry['record_commit']}/{path})", - f"- Licence: {_c(spdx)}", f"- Modality: {_c(', '.join(img.get('modality') or []))}", - f"- Organism: {_c(', '.join(img.get('organism') or []))}", - f"- Voxel size (nm): {_c(f"x={vox.get('x')} y={vox.get('y')} z={vox.get('z')}")}", + lines = [MARKER.format(record_id), "", + f"- Record: [main]({base_url}/main/{path}), " + f"[at `{entry['record_commit'][:12]}`]({base_url}/{entry['record_commit']}/{path})", + f"- Licence: {safe_text(spdx)}", f"- Modality: {safe_text(', '.join(imaging.get('modality') or []))}", + f"- Organism: {safe_text(', '.join(imaging.get('organism') or []))}", + f"- Voxel size (nm): {safe_text(f"x={voxel.get('x')} y={voxel.get('y')} z={voxel.get('z')}")}", f"- Data size: {entry.get('bytes_on_disk')} bytes", - f"- Source: {_c(', '.join([urls] if isinstance(urls, str) else urls))}", - f"- Downloaded by: {', '.join(entry['downloaders'])}", f"- Dataset dir: `{_c(entry['dataset_dir'])}`", "", + f"- Source: {safe_text(', '.join([urls] if isinstance(urls, str) else urls))}", + f"- Downloaded by: {', '.join(entry['downloaders'])}", + f"- Dataset dir: `{safe_text(entry['dataset_dir'])}`", "", "| role | axes | shape | dtype |", "|---|---|---|---|", - *(f"| {_c(a['role'])} | {_c(a.get('axes'))} | {_c(a.get('shape'))} | {_c(a.get('dtype'))} |" for a in arrays)] + *(f"| {safe_text(array['role'])} | {safe_text(array.get('axes'))} | {safe_text(array.get('shape'))} | " + f"{safe_text(array.get('dtype'))} |" for array in arrays)] if spdx not in licences: - lines += ["", f"**Ineligible:** licence `{_c(spdx)}` is not in `crops.licences` ({', '.join(licences)}); `go` is refused."] - return f"{id_}: {_c(rec.get('short_name') or rec.get('title'))}", "\n".join(lines) + "\n", labels + lines += ["", f"**Ineligible:** licence `{safe_text(spdx)}` is not in `crops.licences` ({', '.join(licences)}); " + "`go` is refused."] + title = f"{record_id}: {safe_text(record.get('short_name') or record.get('title'))}" + return title, "\n".join(lines) + "\n", labels + +async def seed(cfg: Config, gh_data: GitHub, gh_catalog: GitHub, dry_run: bool = False) -> list[str]: + """Create an issue in the data repo for each downloaded dataset that has none yet. -async def seed(cfg, gh_data, gh_catalog, dry_run: bool = False) -> list[str]: - """Create the missing issues; returns the record ids created (or that would be, with dry_run).""" - have = {m.group(1) for i in await gh_data.list_issues(state="all") if (m := MARKER_RE.match(i.get("body") or ""))} + `gh_data` is the GitHub client for the data repo (where the issues go) and `gh_catalog` the one for the + catalog repo (where the records and download state are read). Returns the record ids created (or that + would be, with `dry_run`).""" + existing = {match.group(1) for issue in await gh_data.list_issues(state="all") + if (match := MARKER_RE.match(issue.get("body") or ""))} paths = await catalog.record_paths(gh_catalog) - made = [] - for id_, entry in sorted((await catalog.downloaded(gh_catalog)).items()): - if id_ in have: + created = [] + for record_id, entry in sorted((await catalog.downloaded(gh_catalog)).items()): + if record_id in existing: continue - if not ID_RE.fullmatch(id_): - log.warning("seed: bad record id %r; skipping", id_) + if not ID_RE.fullmatch(record_id): + log.warning("seed: bad record id %r; skipping", record_id) continue - if id_ not in paths: - log.warning("seed: %s is downloaded but not on main; skipping", id_) + if record_id not in paths: + log.warning("seed: %s is downloaded but not on main; skipping", record_id) continue - rec = await catalog.record(gh_catalog, paths[id_], "main") - if rec.get("id") not in (None, id_): - log.warning("seed: %s has id %r inside; skipping", id_, rec["id"]) + record = await catalog.record(gh_catalog, paths[record_id], "main") + if record.get("id") not in (None, record_id): # the file name is the trusted id; the field must agree + log.warning("seed: %s has id %r inside; skipping", record_id, record["id"]) continue - title, body, labels = render_issue(rec, entry, cfg.crops.licences, gh_catalog.repo, paths[id_], id_) + title, body, labels = render_issue(record, entry, cfg.crops.licences, gh_catalog.repo, paths[record_id], + record_id) print(f"{'would create' if dry_run else 'creating'} {title} {labels}", flush=True) if not dry_run: await gh_data.create_issue(title, body, labels) - made.append(id_) - return made + created.append(record_id) + return created diff --git a/mia_data/status.py b/mia_data/status.py index 5d7e2f6..68b89f5 100644 --- a/mia_data/status.py +++ b/mia_data/status.py @@ -1,45 +1,54 @@ -"""Issue comments: one new comment per stage and result, never edited (spec §4, §7.4, §7.5).""" +"""Rendering the pieces of mia-data's issue comments. + +Every stage change and every result is a new comment, never an edit, so the issue reads as a log. All +text that came from a record or a job goes through `safe_text` or `neutralize` first. +""" from __future__ import annotations import re from pathlib import Path +from typing import Any from urllib.parse import quote from autodev.activities import neutralize -def _c(x) -> str: - """Record or agent text is data: no forged markers, no table/line breaks.""" - return " ".join(str(x).split()).replace("", - "created_at": "2026-10-10T01:30:00Z"}, - {"id": 10, "user": {"login": "alice"}, "body": "/revise also docs", "created_at": "2026-10-10T02:00:00Z"}])) + respx.get(f"{API}{R}/issues/5/comments").mock( + return_value=httpx.Response( + 200, + json=[ + { + "id": 9, + "user": {"login": "bot"}, + "body": "round 2\n", + "created_at": "2026-10-10T01:30:00Z", + }, + { + "id": 10, + "user": {"login": "alice"}, + "body": "/revise also docs", + "created_at": "2026-10-10T02:00:00Z", + }, + ], + ) + ) for who in ("alice", "bot"): respx.get(f"{API}{R}/collaborators/{who}/permission").mock( - return_value=httpx.Response(200, json={"permission": "write"})) + return_value=httpx.Response(200, json={"permission": "write"}) + ) respx.get(f"{API}{R}/commits/{head}/check-runs").mock(return_value=httpx.Response(200, json={"check_runs": []})) respx.get(f"{API}{R}/commits/{head}/status").mock(return_value=httpx.Response(200, json={"statuses": []})) snap = await acts(tmp_path, gh).observe_pr(ObserveInput(5)) @@ -174,18 +233,32 @@ async def test_observe_skips_handled_feedback(tmp_path, gh): @pytest.mark.parametrize("approve_at,want", [("2026-10-10T01:00:00Z", False), ("2026-10-10T03:00:00Z", True)]) async def test_observe_counts_approve_comments_only_after_latest_round(tmp_path, gh, approve_at, want): head = "abc1234def" - respx.get(f"{API}{R}/pulls/5").mock(return_value=httpx.Response(200, json={ - "state": "open", "merged": False, "head": {"sha": head}})) + respx.get(f"{API}{R}/pulls/5").mock( + return_value=httpx.Response(200, json={"state": "open", "merged": False, "head": {"sha": head}}) + ) respx.get(f"{API}{R}/pulls/5/reviews").mock(return_value=httpx.Response(200, json=[])) respx.get(f"{API}{R}/pulls/5/comments").mock(return_value=httpx.Response(200, json=[])) - respx.get(f"{API}{R}/issues/5/comments").mock(return_value=httpx.Response(200, json=sorted([ - {"id": 9, "user": {"login": "bot"}, "body": f"round 2\n", - "created_at": "2026-10-10T02:00:00Z"}, - {"id": 10, "user": {"login": "alice"}, "body": "/approve abc1234", "created_at": approve_at}], - key=lambda c: c["created_at"]))) + respx.get(f"{API}{R}/issues/5/comments").mock( + return_value=httpx.Response( + 200, + json=sorted( + [ + { + "id": 9, + "user": {"login": "bot"}, + "body": f"round 2\n", + "created_at": "2026-10-10T02:00:00Z", + }, + {"id": 10, "user": {"login": "alice"}, "body": "/approve abc1234", "created_at": approve_at}, + ], + key=lambda c: c["created_at"], + ), + ) + ) for who in ("alice", "bot"): respx.get(f"{API}{R}/collaborators/{who}/permission").mock( - return_value=httpx.Response(200, json={"permission": "write"})) + return_value=httpx.Response(200, json={"permission": "write"}) + ) respx.get(f"{API}{R}/commits/{head}/check-runs").mock(return_value=httpx.Response(200, json={"check_runs": []})) respx.get(f"{API}{R}/commits/{head}/status").mock(return_value=httpx.Response(200, json={"statuses": []})) assert (await acts(tmp_path, gh).observe_pr(ObserveInput(5))).approved is want @@ -194,8 +267,9 @@ async def test_observe_counts_approve_comments_only_after_latest_round(tmp_path, async def _prepared(tmp_path, gh, issue=7): origin = make_origin(tmp_path) a = acts(tmp_path, gh, origin=origin) - respx.get(f"{API}{R}/issues/{issue}").mock(return_value=httpx.Response(200, json={ - "number": issue, "title": "T", "body": "b", "labels": []})) + respx.get(f"{API}{R}/issues/{issue}").mock( + return_value=httpx.Response(200, json={"number": issue, "title": "T", "body": "b", "labels": []}) + ) respx.get(f"{API}{R}/issues/{issue}/comments").mock(return_value=httpx.Response(200, json=[])) ws = await a.prepare_round(RoundInput(issue, K, 1, None, None)) return a, ws, Path(ws.dir) @@ -245,8 +319,16 @@ async def test_failure_log_cannot_forge_markers(tmp_path, gh): assert "" in body -SECRETS = ["sk-ant-oat01-Ab_c-D", "ghp_" + "a1" * 18, "gho_X1", "ghs_y2", "ghu_z3", "ghr_w4", "github_pat_11AB_cd9", - "eyJhbGciOiJSUzI1NiJ9.eyJpc3MiOiIxIn0.c2ln-_x"] +SECRETS = [ + "sk-ant-oat01-Ab_c-D", + "ghp_" + "a1" * 18, + "gho_X1", + "ghs_y2", + "ghu_z3", + "ghr_w4", + "github_pat_11AB_cd9", + "eyJhbGciOiJSUzI1NiJ9.eyJpc3MiOiIxIn0.c2ln-_x", +] @pytest.mark.parametrize("secret", SECRETS) @@ -258,8 +340,10 @@ def test_neutralize_redacts_secret_shapes(secret): @respx.mock async def test_agent_text_cannot_leak_the_github_token(tmp_path): """A prompt-injected agent may print credentials; the worker's own token is redacted too, whatever its shape.""" + async def no_sleep(_): pass + gh = GitHub("o/r", lambda: "Current.Token-42", writes_per_second=1000, sleep=no_sleep) a, ws, d = await _prepared(tmp_path, gh) (d / "result.json").write_text(json.dumps({"result": "token Current.Token-42 " + SECRETS[0]})) @@ -280,8 +364,11 @@ async def no_sleep(_): async def test_outsider_marker_does_not_suppress_summary(tmp_path, gh): a, ws, d = await _prepared(tmp_path, gh) (d / "result.json").write_text(json.dumps({"result": "ok"})) - respx.get(f"{API}{R}/issues/7/comments").mock(return_value=httpx.Response(200, json=[ - {"id": 1, "user": {"login": "mallory"}, "body": f""}])) + respx.get(f"{API}{R}/issues/7/comments").mock( + return_value=httpx.Response( + 200, json=[{"id": 1, "user": {"login": "mallory"}, "body": f""}] + ) + ) post = respx.post(f"{API}{R}/issues/7/comments").mock(return_value=httpx.Response(201, json={"id": 2})) await a.publish_round(PublishInput(7, K, 1, ws, None, None)) assert post.called @@ -302,14 +389,22 @@ async def test_push_failure_is_retryable(tmp_path, gh): async def test_prompt_drops_outsider_comments(tmp_path, gh): origin = make_origin(tmp_path) a = acts(tmp_path, gh, origin=origin) - respx.get(f"{API}{R}/issues/7").mock(return_value=httpx.Response(200, json={ - "number": 7, "title": "T", "body": "b", "labels": []})) - respx.get(f"{API}{R}/issues/7/comments").mock(return_value=httpx.Response(200, json=[ - {"id": 1, "user": {"login": "alice"}, "body": "writer-says-hi"}, - {"id": 2, "user": {"login": "mallory"}, "body": "outsider-injection"}])) + respx.get(f"{API}{R}/issues/7").mock( + return_value=httpx.Response(200, json={"number": 7, "title": "T", "body": "b", "labels": []}) + ) + respx.get(f"{API}{R}/issues/7/comments").mock( + return_value=httpx.Response( + 200, + json=[ + {"id": 1, "user": {"login": "alice"}, "body": "writer-says-hi"}, + {"id": 2, "user": {"login": "mallory"}, "body": "outsider-injection"}, + ], + ) + ) for who, perm in (("alice", "write"), ("mallory", "read")): respx.get(f"{API}{R}/collaborators/{who}/permission").mock( - return_value=httpx.Response(200, json={"permission": perm})) + return_value=httpx.Response(200, json={"permission": perm}) + ) ws = await a.prepare_round(RoundInput(7, K, 1, None, None)) prompt = (Path(ws.dir) / "prompt.md").read_text() assert "writer-says-hi" in prompt and "outsider-injection" not in prompt @@ -327,8 +422,13 @@ async def test_prepare_refuses_a_key_that_is_not_8_hex(tmp_path, gh, key): async def test_poll_continues_after_one_issue_fails(tmp_path, gh): respx.get(f"{API}{R}/issues").mock(return_value=httpx.Response(200, json=[{"number": 1}, {"number": 2}])) respx.get(f"{API}{R}/issues/1/events").mock(return_value=httpx.Response(500, json={})) - respx.get(f"{API}{R}/issues/2/events").mock(return_value=httpx.Response(200, json=[ - {"event": "labeled", "label": {"name": "autodev:go"}, "actor": {"login": "alice"}}])) - respx.get(f"{API}{R}/collaborators/alice/permission").mock(return_value=httpx.Response(200, json={"permission": "write"})) + respx.get(f"{API}{R}/issues/2/events").mock( + return_value=httpx.Response( + 200, json=[{"event": "labeled", "label": {"name": "autodev:go"}, "actor": {"login": "alice"}}] + ) + ) + respx.get(f"{API}{R}/collaborators/alice/permission").mock( + return_value=httpx.Response(200, json={"permission": "write"}) + ) temporal = FakeTemporal() assert await acts(tmp_path, gh, temporal).poll_issues() == [2] diff --git a/tests/test_autodev_agent.py b/tests/test_autodev_agent.py index 621a669..9b83baf 100644 --- a/tests/test_autodev_agent.py +++ b/tests/test_autodev_agent.py @@ -20,8 +20,10 @@ def test_prompt_has_issue_and_human_discussion_only(tmp_path): cfg = make_cfg(tmp_path) - comments = [{"user": {"login": "alice"}, "body": "use lowercase"}, - {"user": {"login": "bot"}, "body": "Started "}] + comments = [ + {"user": {"login": "alice"}, "body": "use lowercase"}, + {"user": {"login": "bot"}, "body": "Started "}, + ] p = render_prompt(cfg, ISSUE, comments, "autodev/issue-7", "abc", None, None) assert "#7 in o/r" in p and "Add feature" in p and "Please add feature.txt" in p assert "use lowercase" in p and "Started" not in p and "Requested changes" not in p @@ -34,8 +36,10 @@ def test_job_script_runs_claude_and_bundles(tmp_path): bin_dir = tmp_path / "bin" bin_dir.mkdir() fake = bin_dir / "claude" - fake.write_text('#!/bin/sh\necho x > feature.txt\ngit add feature.txt\ngit commit -qm "add feature"\n' - 'echo \'{"type":"result","result":"Added feature.txt","total_cost_usd":0.12}\'\n') + fake.write_text( + '#!/bin/sh\necho x > feature.txt\ngit add feature.txt\ngit commit -qm "add feature"\n' + 'echo \'{"type":"result","result":"Added feature.txt","total_cost_usd":0.12}\'\n' + ) fake.chmod(0o755) cfg = make_cfg(tmp_path, agent={"path_prepend": [str(bin_dir)], "model": "it's-quoted"}) m = Mirror(tmp_path / "mirror.git", str(make_origin(tmp_path))) @@ -88,8 +92,15 @@ def status(self, ids=None): if ids is None: return self.existing st = self.states.pop(0) if len(self.states) > 1 else self.states[0] - return [SimpleNamespace(job_id=ids[0], state=st, exit_code=0 if st == "DONE" else None, name=None, - terminal=st in ("DONE", "EXIT", "UNKNOWN"))] + return [ + SimpleNamespace( + job_id=ids[0], + state=st, + exit_code=0 if st == "DONE" else None, + name=None, + terminal=st in ("DONE", "EXIT", "UNKNOWN"), + ) + ] def _jobs(tmp_path, fake, **agent): @@ -147,8 +158,9 @@ async def test_wait_heartbeats_until_terminal(tmp_path): async def test_unknown_state_uses_exit_code_file(tmp_path): (tmp_path / "exit_code").write_text("0\n") - out = await _jobs(tmp_path, FakeCsub(states=["UNKNOWN"])).wait("777", Workspace(str(tmp_path), "a", "b"), - lambda s: None) + out = await _jobs(tmp_path, FakeCsub(states=["UNKNOWN"])).wait( + "777", Workspace(str(tmp_path), "a", "b"), lambda s: None + ) assert (out.state, out.exit_code) == ("DONE", 0) @@ -222,8 +234,12 @@ async def test_submit_reuses_job_when_work_root_is_symlinked(tmp_path): real = tmp_path / "real" real.mkdir() (tmp_path / "link").symlink_to(real) - fake = FakeCsub(existing=[SimpleNamespace(job_id="555", name="x", cwd=str(real.resolve() / "r1"), state="RUN"), - SimpleNamespace(job_id="556", name="y", state="RUN")]) + fake = FakeCsub( + existing=[ + SimpleNamespace(job_id="555", name="x", cwd=str(real.resolve() / "r1"), state="RUN"), + SimpleNamespace(job_id="556", name="y", state="RUN"), + ] + ) cfg = make_cfg(tmp_path, paths={"state_dir": str(tmp_path / "state"), "work_root": str(tmp_path / "link")}) j = AgentJobs(cfg, client_factory=lambda: fake) ws = Workspace(str(j.cfg.paths.work_root / "r1"), "abc", "b") @@ -232,13 +248,16 @@ async def test_submit_reuses_job_when_work_root_is_symlinked(tmp_path): assert ref.job_id == "555" and fake.submitted == [] -@pytest.mark.parametrize("err,permanent", [ - (CsubError("policy_violation", "mount not under allowed_roots"), True), - (CsubError("invalid_request", "bad spec"), True), - (ProtocolError("response: not valid JSON"), True), - (TransportError("cannot reach broker socket"), False), - (CsubError("lsf_error", "bsub timed out"), False), -]) +@pytest.mark.parametrize( + "err,permanent", + [ + (CsubError("policy_violation", "mount not under allowed_roots"), True), + (CsubError("invalid_request", "bad spec"), True), + (ProtocolError("response: not valid JSON"), True), + (TransportError("cannot reach broker socket"), False), + (CsubError("lsf_error", "bsub timed out"), False), + ], +) async def test_submit_rejection_is_permanent_transport_error_is_not(tmp_path, err, permanent): j = _jobs(tmp_path, FakeCsub(error=err)) a = Activities(j.cfg, None, None, j, "bot") @@ -254,7 +273,9 @@ async def test_submit_rejection_is_permanent_transport_error_is_not(tmp_path, er async def test_submit_refuses_a_key_that_is_not_8_hex(tmp_path, key): j = _jobs(tmp_path, FakeCsub()) with pytest.raises(ApplicationError) as e: - await Activities(j.cfg, None, None, j, "bot").submit_agent(SubmitInput(7, key, 1, Workspace(str(tmp_path), "a", "b"))) + await Activities(j.cfg, None, None, j, "bot").submit_agent( + SubmitInput(7, key, 1, Workspace(str(tmp_path), "a", "b")) + ) assert e.value.non_retryable and not (tmp_path / "state" / "jobs").exists() diff --git a/tests/test_autodev_auth.py b/tests/test_autodev_auth.py index 4774a9a..873f102 100644 --- a/tests/test_autodev_auth.py +++ b/tests/test_autodev_auth.py @@ -21,8 +21,9 @@ def key(): def _pem(key) -> bytes: - return key.private_bytes(serialization.Encoding.PEM, serialization.PrivateFormat.PKCS8, - serialization.NoEncryption()) + return key.private_bytes( + serialization.Encoding.PEM, serialization.PrivateFormat.PKCS8, serialization.NoEncryption() + ) def test_app_jwt_is_signed_and_backdated(key): @@ -37,10 +38,12 @@ def test_app_jwt_is_signed_and_backdated(key): def test_app_tokens_are_cached_until_near_expiry(key, tmp_path): kp = tmp_path / "app.pem" kp.write_bytes(_pem(key)) - route = respx.post(f"{API}/app/installations/7/access_tokens").mock(side_effect=[ - httpx.Response(201, json={"token": "t1", "expires_at": "2026-10-10T12:00:00Z"}), - httpx.Response(201, json={"token": "t2", "expires_at": "2026-10-10T13:00:00Z"}), - ]) + route = respx.post(f"{API}/app/installations/7/access_tokens").mock( + side_effect=[ + httpx.Response(201, json={"token": "t1", "expires_at": "2026-10-10T12:00:00Z"}), + httpx.Response(201, json={"token": "t2", "expires_at": "2026-10-10T13:00:00Z"}), + ] + ) now = [1791633600.0 - 3600] # 2026-10-10T11:00:00Z tokens = AppTokens(42, 7, kp, clock=lambda: now[0]) assert tokens() == "t1" and tokens() == "t1" and route.call_count == 1 @@ -78,5 +81,6 @@ async def test_bot_login_app_mode(tmp_path, key): @respx.mock async def test_noreply_email_uses_the_bot_user_id(tmp_path): respx.get(f"{API}/users/autodev-bot%5Bbot%5D").mock(return_value=httpx.Response(200, json={"id": 123})) - assert await noreply_email(make_cfg(tmp_path), "autodev-bot[bot]") == \ - "123+autodev-bot[bot]@users.noreply.github.com" + assert ( + await noreply_email(make_cfg(tmp_path), "autodev-bot[bot]") == "123+autodev-bot[bot]@users.noreply.github.com" + ) diff --git a/tests/test_autodev_config.py b/tests/test_autodev_config.py index ef65d66..499ca36 100644 --- a/tests/test_autodev_config.py +++ b/tests/test_autodev_config.py @@ -40,13 +40,24 @@ def test_deployment_name_is_restricted(tmp_path): make_cfg(tmp_path, deployment="Auto Dev") -@pytest.mark.parametrize("agent", [ - {"allow_hosts": ["github.com"]}, {"allow_hosts": [" api.github.com"]}, {"allow_hosts": ["github.com "]}, - {"extra_mounts": ["tools:ro"]}, {"extra_mounts": ["~/tools:ro"]}, {"allow_hosts": ["api.github.com"]}, - {"allow_hosts": ["Raw.GithubUserContent.com."]}, {"extra_mounts": ["/tools"]}, - {"extra_mounts": ["/tools:rw"]}, {"extra_mounts": ["{state}:ro"]}, {"extra_mounts": ["{state}/sub:ro"]}, - {"extra_mounts": ["{tmp}:ro"]}, {"extra_mounts": ["{tmp}/tok:ro"]}, -]) +@pytest.mark.parametrize( + "agent", + [ + {"allow_hosts": ["github.com"]}, + {"allow_hosts": [" api.github.com"]}, + {"allow_hosts": ["github.com "]}, + {"extra_mounts": ["tools:ro"]}, + {"extra_mounts": ["~/tools:ro"]}, + {"allow_hosts": ["api.github.com"]}, + {"allow_hosts": ["Raw.GithubUserContent.com."]}, + {"extra_mounts": ["/tools"]}, + {"extra_mounts": ["/tools:rw"]}, + {"extra_mounts": ["{state}:ro"]}, + {"extra_mounts": ["{state}/sub:ro"]}, + {"extra_mounts": ["{tmp}:ro"]}, + {"extra_mounts": ["{tmp}/tok:ro"]}, + ], +) def test_agent_constraints_rejected(tmp_path, agent): sub = {"state": str(tmp_path / "state"), "tmp": str(tmp_path)} agent = {k: [v.format(**sub) for v in vs] for k, vs in agent.items()} @@ -63,8 +74,11 @@ def test_paths_are_canonical(tmp_path): for name in ("work", "state", "tools"): (tmp_path / f"real-{name}").mkdir() (tmp_path / f"link-{name}").symlink_to(tmp_path / f"real-{name}") - cfg = make_cfg(tmp_path, agent={"extra_mounts": [f"{tmp_path}/link-tools/../link-tools:ro"]}, - paths={"state_dir": str(tmp_path / "link-state"), "work_root": str(tmp_path / "link-work")}) + cfg = make_cfg( + tmp_path, + agent={"extra_mounts": [f"{tmp_path}/link-tools/../link-tools:ro"]}, + paths={"state_dir": str(tmp_path / "link-state"), "work_root": str(tmp_path / "link-work")}, + ) assert cfg.paths.work_root == (tmp_path / "real-work").resolve() assert cfg.paths.state_dir == (tmp_path / "real-state").resolve() assert cfg.agent.extra_mounts == [f"{(tmp_path / 'real-tools').resolve()}:ro"] diff --git a/tests/test_autodev_github.py b/tests/test_autodev_github.py index 85ac587..cfc7b08 100644 --- a/tests/test_autodev_github.py +++ b/tests/test_autodev_github.py @@ -24,11 +24,13 @@ async def fake_sleep(s): @respx.mock async def test_secondary_rate_limit_backoff(gh): - route = respx.post(f"{API}{R}/issues/1/comments").mock(side_effect=[ - httpx.Response(403, json={"message": "You have exceeded a secondary rate limit"}), - httpx.Response(403, headers={"retry-after": "7"}, json={"message": "slow down"}), - httpx.Response(201, json={"id": 1}), - ]) + route = respx.post(f"{API}{R}/issues/1/comments").mock( + side_effect=[ + httpx.Response(403, json={"message": "You have exceeded a secondary rate limit"}), + httpx.Response(403, headers={"retry-after": "7"}, json={"message": "slow down"}), + httpx.Response(201, json={"id": 1}), + ] + ) assert (await gh.comment(1, "hi"))["id"] == 1 assert route.call_count == 3 assert gh.sleeps[:1] == [60.0] and 7.0 in gh.sleeps @@ -38,10 +40,12 @@ async def test_secondary_rate_limit_backoff(gh): @respx.mock async def test_primary_rate_limit_waits_for_reset(gh, monkeypatch): monkeypatch.setattr("time.time", lambda: 1000.0) - respx.get(f"{API}{R}/issues/5").mock(side_effect=[ - httpx.Response(403, headers={"x-ratelimit-remaining": "0", "x-ratelimit-reset": "1030"}), - httpx.Response(200, json={"number": 5}), - ]) + respx.get(f"{API}{R}/issues/5").mock( + side_effect=[ + httpx.Response(403, headers={"x-ratelimit-remaining": "0", "x-ratelimit-reset": "1030"}), + httpx.Response(200, json={"number": 5}), + ] + ) assert (await gh.get_issue(5))["number"] == 5 assert gh.sleeps == [31.0] @@ -70,10 +74,16 @@ async def fake_sleep(s): @respx.mock async def test_label_event_and_actor(gh): - respx.get(f"{API}{R}/issues/3/events").mock(return_value=httpx.Response(200, json=[ - {"id": 1, "event": "labeled", "label": {"name": "x"}, "actor": {"login": "a"}, "created_at": "t1"}, - {"id": 2, "event": "labeled", "label": {"name": "x"}, "actor": {"login": "b"}, "created_at": "t2"}, - {"id": 3, "event": "labeled", "label": {"name": "y"}, "actor": {"login": "c"}, "created_at": "t3"}])) + respx.get(f"{API}{R}/issues/3/events").mock( + return_value=httpx.Response( + 200, + json=[ + {"id": 1, "event": "labeled", "label": {"name": "x"}, "actor": {"login": "a"}, "created_at": "t1"}, + {"id": 2, "event": "labeled", "label": {"name": "x"}, "actor": {"login": "b"}, "created_at": "t2"}, + {"id": 3, "event": "labeled", "label": {"name": "y"}, "actor": {"login": "c"}, "created_at": "t3"}, + ], + ) + ) assert await gh.label_event(3, "x") == ("b", "2", "t2") assert await gh.label_actor(3, "x") == "b" assert await gh.label_event(3, "z") == (None, None, None) @@ -88,9 +98,17 @@ async def test_create_issue(gh): @respx.mock async def test_find_issue_by_marker(gh): - route = respx.get(f"{API}{R}/issues").mock(return_value=httpx.Response(200, json=[ - {"number": 1, "body": "nope"}, {"number": 2, "body": "x y"}, {"number": 3, "body": ""}, - {"number": 4, "body": "", "pull_request": {}}])) + route = respx.get(f"{API}{R}/issues").mock( + return_value=httpx.Response( + 200, + json=[ + {"number": 1, "body": "nope"}, + {"number": 2, "body": "x y"}, + {"number": 3, "body": ""}, + {"number": 4, "body": "", "pull_request": {}}, + ], + ) + ) assert (await gh.find_issue_by_marker(""))["number"] == 2 assert route.calls[0].request.url.params["state"] == "all" assert await gh.find_issue_by_marker("zzz") is None @@ -100,8 +118,9 @@ async def test_find_issue_by_marker(gh): @respx.mock async def test_find_and_edit_comment(gh): - respx.get(f"{API}{R}/issues/4/comments").mock(return_value=httpx.Response(200, json=[ - {"id": 1, "body": "a"}, {"id": 2, "body": "has MK"}])) + respx.get(f"{API}{R}/issues/4/comments").mock( + return_value=httpx.Response(200, json=[{"id": 1, "body": "a"}, {"id": 2, "body": "has MK"}]) + ) assert (await gh.find_comment(4, "MK"))["id"] == 2 assert await gh.find_comment(4, "nope") is None route = respx.patch(f"{API}{R}/issues/comments/2").mock(return_value=httpx.Response(200, json={"id": 2})) @@ -121,16 +140,25 @@ async def fake_sleep(s): await g.set_status("abc", "ctx", "success", "d" * 200, "http://u") await g.set_status("abc", "ctx", "failure", "d", None) assert json.loads(route.calls[0].request.content) == { - "state": "success", "context": "ctx", "description": "d" * 140, "target_url": "http://u"} + "state": "success", + "context": "ctx", + "description": "d" * 140, + "target_url": "http://u", + } assert len(sleeps) == 1 - respx.get(f"{API}{R}/commits/abc/status").mock(return_value=httpx.Response(200, json={"statuses": [{"context": "c"}]})) + respx.get(f"{API}{R}/commits/abc/status").mock( + return_value=httpx.Response(200, json={"statuses": [{"context": "c"}]}) + ) assert await g.statuses("abc") == [{"context": "c"}] @respx.mock async def test_dir_sha(gh): - route = respx.get(f"{API}{R}/contents/a/b").mock(return_value=httpx.Response(200, json=[ - {"name": "c", "type": "dir", "sha": "S1"}, {"name": "d", "type": "file", "sha": "S2"}])) + route = respx.get(f"{API}{R}/contents/a/b").mock( + return_value=httpx.Response( + 200, json=[{"name": "c", "type": "dir", "sha": "S1"}, {"name": "d", "type": "file", "sha": "S2"}] + ) + ) assert await gh.dir_sha("a/b/c", "main") == "S1" assert route.calls[0].request.url.params["ref"] == "main" assert await gh.dir_sha("a/b/d", "main") is None # a file, not a dir @@ -156,15 +184,18 @@ async def test_ref_prs_and_files(gh): assert await gh.list_prs() == [{"number": 1}] p = route.calls[0].request.url.params assert (p["state"], p["sort"], p["direction"], p["per_page"]) == ("all", "updated", "desc", "50") - f = respx.get(f"{API}{R}/pulls/7/files").mock(return_value=httpx.Response(200, json=[{"filename": "snippets/a.py"}])) + f = respx.get(f"{API}{R}/pulls/7/files").mock( + return_value=httpx.Response(200, json=[{"filename": "snippets/a.py"}]) + ) assert await gh.pr_files(7) == ["snippets/a.py"] assert f.calls[0].request.url.params["per_page"] == "100" @respx.mock async def test_ensure_branch_noop_second_time(gh): - respx.get(f"{API}{R}/git/ref/heads/g").mock(side_effect=[ - httpx.Response(404, json={}), httpx.Response(200, json={"object": {"sha": "c"}})]) + respx.get(f"{API}{R}/git/ref/heads/g").mock( + side_effect=[httpx.Response(404, json={}), httpx.Response(200, json={"object": {"sha": "c"}})] + ) respx.post(f"{API}{R}/git/trees").mock(return_value=httpx.Response(201, json={"sha": "t"})) commits = respx.post(f"{API}{R}/git/commits").mock(return_value=httpx.Response(201, json={"sha": "c"})) refs = respx.post(f"{API}{R}/git/refs").mock(return_value=httpx.Response(201, json={})) @@ -175,8 +206,9 @@ async def test_ensure_branch_noop_second_time(gh): @respx.mock async def test_upsert_file_skips_existing(gh): - respx.get(f"{API}{R}/contents/a/f.json").mock(side_effect=[ - httpx.Response(404, json={}), httpx.Response(200, json={"sha": "x"})]) + respx.get(f"{API}{R}/contents/a/f.json").mock( + side_effect=[httpx.Response(404, json={}), httpx.Response(200, json={"sha": "x"})] + ) put = respx.put(f"{API}{R}/contents/a/f.json").mock(return_value=httpx.Response(201, json={})) assert await gh.upsert_file("a/f.json", b"{}", "b", "m") is True assert await gh.upsert_file("a/f.json", b"{}", "b", "m") is False diff --git a/tests/test_autodev_jobs.py b/tests/test_autodev_jobs.py index 1ff7974..746c387 100644 --- a/tests/test_autodev_jobs.py +++ b/tests/test_autodev_jobs.py @@ -21,7 +21,9 @@ async def test_submit_once_per_key(tmp_path): async def test_reattach_by_cwd(tmp_path): ws = tmp_path / "ws" - fake = FakeCsub(existing=[SimpleNamespace(job_id="555", cwd="/elsewhere"), SimpleNamespace(job_id="556", cwd=str(ws))]) + fake = FakeCsub( + existing=[SimpleNamespace(job_id="555", cwd="/elsewhere"), SimpleNamespace(job_id="556", cwd=str(ws))] + ) assert await _jobs(tmp_path, fake).submit("k1", JobSpec("j", "true", str(ws), 10)) == "556" assert fake.submitted == [] and (tmp_path / "ids" / "k1.id").read_text() == "556" @@ -29,22 +31,46 @@ async def test_reattach_by_cwd(tmp_path): async def test_plain_job_spec(tmp_path): fake = FakeCsub() ws = str(tmp_path / "ws") - await _jobs(tmp_path, fake).submit("k1", JobSpec("harness", "echo hi", ws, 30, cpus=2, mem_mb=8000, - allow_hosts=["pypi.org"], ro_mounts=["/data", "/tools"], - env={"A": "1"})) + await _jobs(tmp_path, fake).submit( + "k1", + JobSpec( + "harness", + "echo hi", + ws, + 30, + cpus=2, + mem_mb=8000, + allow_hosts=["pypi.org"], + ro_mounts=["/data", "/tools"], + env={"A": "1"}, + ), + ) s = fake.submitted[0] assert s["agent"] is None and s["env"] == {"A": "1"} and s["session"] == "dep" and s["cwd"] == ws - assert s["mounts"] == [{"path": ws, "mode": "rw"}, {"path": "/data", "mode": "ro"}, {"path": "/tools", "mode": "ro"}] - assert (s["name"], s["command"], s["cpus"], s["mem_mb"], s["walltime_min"], s["allow_hosts"]) == \ - ("harness", "echo hi", 2, 8000, 30, ["pypi.org"]) + assert s["mounts"] == [ + {"path": ws, "mode": "rw"}, + {"path": "/data", "mode": "ro"}, + {"path": "/tools", "mode": "ro"}, + ] + assert (s["name"], s["command"], s["cpus"], s["mem_mb"], s["walltime_min"], s["allow_hosts"]) == ( + "harness", + "echo hi", + 2, + 8000, + 30, + ["pypi.org"], + ) async def test_rw_mounts_follow_the_workspace(tmp_path): fake = FakeCsub() ws = str(tmp_path / "ws") await _jobs(tmp_path, fake).submit("k1", JobSpec("j", "true", ws, 10, ro_mounts=["/ro"], rw_mounts=["/cache"])) - assert fake.submitted[0]["mounts"] == [{"path": ws, "mode": "rw"}, {"path": "/cache", "mode": "rw"}, - {"path": "/ro", "mode": "ro"}] + assert fake.submitted[0]["mounts"] == [ + {"path": ws, "mode": "rw"}, + {"path": "/cache", "mode": "rw"}, + {"path": "/ro", "mode": "ro"}, + ] @pytest.mark.parametrize("key", ["../x", "", "a/b", "x" * 121]) @@ -67,7 +93,9 @@ async def test_recorded_id_skips_building_the_spec(tmp_path, monkeypatch): from autodev.models import Workspace monkeypatch.delenv("UNSET_KEY", raising=False) - j = AgentJobs(make_cfg(tmp_path, agent={"api_key_env": "UNSET_KEY"}), client_factory=lambda: pytest.fail("client used")) + j = AgentJobs( + make_cfg(tmp_path, agent={"api_key_env": "UNSET_KEY"}), client_factory=lambda: pytest.fail("client used") + ) (j.cfg.paths.state_dir / "jobs").mkdir(parents=True) (j.cfg.paths.state_dir / "jobs" / f"issue-7-{K}-r1.id").write_text("444") assert (await j.submit(7, K, 1, Workspace(str(tmp_path), "a", "b"))).job_id == "444" diff --git a/tests/test_autodev_ops.py b/tests/test_autodev_ops.py index bbe0b16..f5e9f77 100644 --- a/tests/test_autodev_ops.py +++ b/tests/test_autodev_ops.py @@ -68,6 +68,8 @@ def test_logs_missing_file_exits(tmp_path): def test_cli_reads_deployment_from_config(tmp_path, tmux): y = tmp_path / "a.yaml" - y.write_text(f"deployment: autodev-zed\ngithub: {{repo: o/r}}\npaths: {{work_root: {tmp_path}/w, state_dir: {tmp_path}/s}}\n") + y.write_text( + f"deployment: autodev-zed\ngithub: {{repo: o/r}}\npaths: {{work_root: {tmp_path}/w, state_dir: {tmp_path}/s}}\n" + ) assert main(["--config", str(y), "down"]) == 0 assert ["tmux", "kill-session", "-t", "=autodev-zed"] in tmux.calls diff --git a/tests/test_autodev_review.py b/tests/test_autodev_review.py index ad00ec2..96cbd5e 100644 --- a/tests/test_autodev_review.py +++ b/tests/test_autodev_review.py @@ -13,8 +13,7 @@ def rv(id, login, state, commit=HEAD, body="", at="2026-10-10T01:00:00Z"): - return {"id": id, "user": {"login": login}, "state": state, "commit_id": commit, "body": body, - "submitted_at": at} + return {"id": id, "user": {"login": login}, "state": state, "commit_id": commit, "body": body, "submitted_at": at} def cm(id, login, body, at="2026-10-10T02:00:00Z"): @@ -60,41 +59,54 @@ def test_approve_comment_must_follow_latest_round(): def test_latest_round_at_is_the_bots_latest_round_marker(): - comments = [cm(1, "bot", f"r1 {MP} round k-1 -->", at="2026-10-10T01:00:00Z"), - cm(2, "bot", f"r2 {MP} round k-2 -->", at="2026-10-10T03:00:00Z"), - cm(3, "mallory", f"{MP} round k-3 -->", at="2026-10-10T04:00:00Z"), - cm(4, "bot", f"{MP} started k-3 -->", at="2026-10-10T05:00:00Z")] + comments = [ + cm(1, "bot", f"r1 {MP} round k-1 -->", at="2026-10-10T01:00:00Z"), + cm(2, "bot", f"r2 {MP} round k-2 -->", at="2026-10-10T03:00:00Z"), + cm(3, "mallory", f"{MP} round k-3 -->", at="2026-10-10T04:00:00Z"), + cm(4, "bot", f"{MP} started k-3 -->", at="2026-10-10T05:00:00Z"), + ] assert latest_round_at(comments, MP, "bot") == "2026-10-10T03:00:00Z" assert latest_round_at([], MP, "bot") is None def test_feedback_from_reviews_and_revise_comments_oldest_first(): - reviews = [rv(3, "alice", "CHANGES_REQUESTED", body="rename foo", at="2026-10-10T01:00:00Z"), - rv(4, "bob", "CHANGES_REQUESTED", body="ignored: reader")] - inline = [{"pull_request_review_id": 3, "path": "a.py", "line": 4, "body": "here"}, - {"pull_request_review_id": 99, "path": "b.py", "line": 1, "body": "other review"}] - comments = [cm(10, "admin", "/revise also update docs", at="2026-10-10T00:30:00Z"), - cm(11, "alice", "looks good"), cm(12, "bot", f"/revise {MP} x -->")] + reviews = [ + rv(3, "alice", "CHANGES_REQUESTED", body="rename foo", at="2026-10-10T01:00:00Z"), + rv(4, "bob", "CHANGES_REQUESTED", body="ignored: reader"), + ] + inline = [ + {"pull_request_review_id": 3, "path": "a.py", "line": 4, "body": "here"}, + {"pull_request_review_id": 99, "path": "b.py", "line": 1, "body": "other review"}, + ] + comments = [ + cm(10, "admin", "/revise also update docs", at="2026-10-10T00:30:00Z"), + cm(11, "alice", "looks good"), + cm(12, "bot", f"/revise {MP} x -->"), + ] fb = feedback(reviews, inline, comments, W, "bot", False, MP) - assert fb == [Feedback("comment-10", "also update docs"), - Feedback("review-3", "rename foo\n- `a.py` line 4: here")] + assert fb == [Feedback("comment-10", "also update docs"), Feedback("review-3", "rename foo\n- `a.py` line 4: here")] def test_handled_ids_come_from_our_markers(): - comments = [cm(1, "bot", "done\n\n"), - cm(2, "alice", ""), # a person pasting our marker does not count - cm(3, "bot", "see and handled review-7 -->")] # not anchored to the prefix + comments = [ + cm(1, "bot", "done\n\n"), + cm(2, "alice", ""), # a person pasting our marker does not count + cm(3, "bot", "see and handled review-7 -->"), + ] # not anchored to the prefix assert handled_ids(comments, MP, "bot") == {"review-3"} -@pytest.mark.parametrize("runs,statuses,want", [ - ([], [], "none"), - ([{"status": "completed", "conclusion": "success"}], [{"state": "success"}], "success"), - ([{"status": "in_progress", "conclusion": None}], [], "pending"), - ([{"status": "completed", "conclusion": "skipped"}], [{"state": "pending"}], "pending"), - ([{"status": "completed", "conclusion": "failure"}], [{"state": "pending"}], "failure"), - ([], [{"state": "error"}], "failure"), -]) +@pytest.mark.parametrize( + "runs,statuses,want", + [ + ([], [], "none"), + ([{"status": "completed", "conclusion": "success"}], [{"state": "success"}], "success"), + ([{"status": "in_progress", "conclusion": None}], [], "pending"), + ([{"status": "completed", "conclusion": "skipped"}], [{"state": "pending"}], "pending"), + ([{"status": "completed", "conclusion": "failure"}], [{"state": "pending"}], "failure"), + ([], [{"state": "error"}], "failure"), + ], +) def test_combine_checks(runs, statuses, want): assert combine_checks(runs, statuses) == want @@ -103,17 +115,23 @@ def test_combine_checks(runs, statuses, want): def gh(): async def no_sleep(_): pass + return GitHub("o/r", lambda: "tok", writes_per_second=1000, sleep=no_sleep) @respx.mock async def test_label_actor_is_latest_labeler(gh): - respx.get(f"{API}{R}/issues/1/events").mock(return_value=httpx.Response(200, json=[ - {"event": "labeled", "label": {"name": "autodev:go"}, "actor": {"login": "a"}}, - {"event": "unlabeled", "label": {"name": "autodev:go"}, "actor": {"login": "a"}}, - {"event": "labeled", "label": {"name": "autodev:go"}, "actor": {"login": "b"}}, - {"event": "labeled", "label": {"name": "other"}, "actor": {"login": "c"}}, - ])) + respx.get(f"{API}{R}/issues/1/events").mock( + return_value=httpx.Response( + 200, + json=[ + {"event": "labeled", "label": {"name": "autodev:go"}, "actor": {"login": "a"}}, + {"event": "unlabeled", "label": {"name": "autodev:go"}, "actor": {"login": "a"}}, + {"event": "labeled", "label": {"name": "autodev:go"}, "actor": {"login": "b"}}, + {"event": "labeled", "label": {"name": "other"}, "actor": {"login": "c"}}, + ], + ) + ) assert await gh.label_actor(1, "autodev:go") == "b" @@ -138,7 +156,8 @@ async def test_merge_refusal_is_reported_not_raised(gh): @respx.mock async def test_check_state_reads_both_apis(gh): - respx.get(f"{API}{R}/commits/{HEAD}/check-runs").mock(return_value=httpx.Response(200, json={ - "check_runs": [{"status": "completed", "conclusion": "success"}]})) + respx.get(f"{API}{R}/commits/{HEAD}/check-runs").mock( + return_value=httpx.Response(200, json={"check_runs": [{"status": "completed", "conclusion": "success"}]}) + ) respx.get(f"{API}{R}/commits/{HEAD}/status").mock(return_value=httpx.Response(200, json={"statuses": []})) assert await gh.check_state(HEAD) == "success" diff --git a/tests/test_autodev_workflows.py b/tests/test_autodev_workflows.py index 6040edb..5f2b20c 100644 --- a/tests/test_autodev_workflows.py +++ b/tests/test_autodev_workflows.py @@ -48,8 +48,19 @@ def snap(state="open", sha="h1", approved=False, checks="none", feedback=()): class Mocks: - def __init__(self, snaps, existing_pr=None, job_state="DONE", exit_code=0, publish_pr=5, await_failures=0, - merge_ok=True, observe_failures=0, report_failures=0, observe_fatal=False): + def __init__( + self, + snaps, + existing_pr=None, + job_state="DONE", + exit_code=0, + publish_pr=5, + await_failures=0, + merge_ok=True, + observe_failures=0, + report_failures=0, + observe_fatal=False, + ): self.observe_failures, self.report_failures, self.reported = observe_failures, report_failures, 0 self.observe_fatal = observe_fatal self.calls: list[tuple[str, object]] = [] @@ -137,8 +148,9 @@ async def start(env, m, inp=None, runner=None): main = Worker(env.client, task_queue=tq, workflows=[IssueWorkflow], activities=m.acts(), **kw) waits = Worker(env.client, task_queue=tq + "-await", activities=m.await_act()) async with main, waits: - h = await env.client.start_workflow(IssueWorkflow.execute, inp or IssueInput(7), id=f"wf-{uuid.uuid4()}", - task_queue=tq) + h = await env.client.start_workflow( + IssueWorkflow.execute, inp or IssueInput(7), id=f"wf-{uuid.uuid4()}", task_queue=tq + ) return await h.result(), h @@ -246,8 +258,12 @@ async def test_replay_recorded_history(env): AUTODEV_RECORD_HISTORY=1 pixi run pytest tests/test_autodev_workflows.py -k recorded_history""" if os.environ.get("AUTODEV_RECORD_HISTORY"): fb = Feedback("review-1", "rename foo") - _, h = await start(env, Mocks([snap(feedback=[fb]), snap(sha="h2", feedback=[fb]), - snap(sha="h2", approved=True, checks="success")])) + _, h = await start( + env, + Mocks( + [snap(feedback=[fb]), snap(sha="h2", feedback=[fb]), snap(sha="h2", approved=True, checks="success")] + ), + ) HISTORY.parent.mkdir(exist_ok=True) HISTORY.write_text(json.dumps(_scrub((await h.fetch_history()).to_json_dict()), indent=1) + "\n") history = WorkflowHistory.from_json("autodev-recorded", HISTORY.read_text()) diff --git a/tests/test_mia_activities.py b/tests/test_mia_activities.py index 73a7fe8..7486389 100644 --- a/tests/test_mia_activities.py +++ b/tests/test_mia_activities.py @@ -15,10 +15,17 @@ from mia_data.models import DatasetState, JobArgs, job_key REC_SHA, SNIP_SHA = "c" * 40, "a" * 40 -RECORD = {"data": {"download_url": "https://data.example.org/a.zip", "formats": ["tiff"]}, - "imaging": {"modality": ["confocal"], "organism": ["Homo sapiens"]}} -BINDING = {"record": "rec-1", "record_commit": REC_SHA, "snippets_commit": SNIP_SHA, "reasons": ["r"], - "raw": [{"snippet": "tiff-zip-member", "urls": ["https://data.example.org/a.zip::m.tif"], "params": {}}]} +RECORD = { + "data": {"download_url": "https://data.example.org/a.zip", "formats": ["tiff"]}, + "imaging": {"modality": ["confocal"], "organism": ["Homo sapiens"]}, +} +BINDING = { + "record": "rec-1", + "record_commit": REC_SHA, + "snippets_commit": SNIP_SHA, + "reasons": ["r"], + "raw": [{"snippet": "tiff-zip-member", "urls": ["https://data.example.org/a.zip::m.tif"], "params": {}}], +} ROUND = "i1-e99" @@ -34,9 +41,16 @@ def _setup(tmp_path, probe=None, csub=None): dl.mkdir() (dl / "conversion.json").write_text('{"axes": "zyx"}') downloads = {"downloaded": {"rec-1": {"at": "2026-10-01", "record_commit": REC_SHA, "dataset_dir": str(dl)}}} - cat = FakeGH("o/catalog", files={("state/downloads.json", "state"): json.dumps(downloads).encode(), - ("datasets/rec-1.yaml", REC_SHA): yaml.safe_dump(RECORD).encode()}) - data = FakeGH("o/data", issues={1: {"number": 1, "body": "\n\n- Record: x", "labels": []}}) + cat = FakeGH( + "o/catalog", + files={ + ("state/downloads.json", "state"): json.dumps(downloads).encode(), + ("datasets/rec-1.yaml", REC_SHA): yaml.safe_dump(RECORD).encode(), + }, + ) + data = FakeGH( + "o/data", issues={1: {"number": 1, "body": "\n\n- Record: x", "labels": []}} + ) snip = FakeGH("o/snippets", refs={"main": SNIP_SHA}) snap = cfg.paths.work_root / "_snapshots" / SNIP_SHA # as snapshot() leaves it for name in ("tiff-zip-member", "old-tiff"): @@ -46,8 +60,9 @@ def _setup(tmp_path, probe=None, csub=None): (snap / "SPEC.md").write_text("spec\n") csub = csub or FakeCsub() jobs = CsubJobs("mia-data", cfg.paths.state_dir / "jobs", client_factory=lambda: csub) - a = Activities(cfg, data, snip, cat, jobs, bot="bot", - probe=probe or (lambda: ["*.example.org", *cfg.network.package_hosts])) + a = Activities( + cfg, data, snip, cat, jobs, bot="bot", probe=probe or (lambda: ["*.example.org", *cfg.network.package_hosts]) + ) return SimpleNamespace(a=a, cfg=cfg, data=data, snip=snip, cat=cat, csub=csub) @@ -114,7 +129,9 @@ async def test_read_match_foreign_host_needs_human(tmp_path): async def test_prepare_match_malformed_snippet_yaml_is_permanent(tmp_path): t = _setup(tmp_path) pins = await t.a.admit(1, "99") - (t.cfg.paths.work_root / "_snapshots" / SNIP_SHA / "snippets" / "tiff-zip-member" / "snippet.yaml").write_text("a: [\n") + (t.cfg.paths.work_root / "_snapshots" / SNIP_SHA / "snippets" / "tiff-zip-member" / "snippet.yaml").write_text( + "a: [\n" + ) with pytest.raises(ApplicationError) as e: await t.a.prepare_match(1, ROUND, pins, 1) assert e.value.non_retryable and e.value.type in ("ParserError", "ScannerError") @@ -135,8 +152,10 @@ async def test_submit_refuses_moved_harness(tmp_path): async def test_submit_refuses_host_missing_from_probe(tmp_path): - t = _setup(tmp_path, probe=lambda: ["*.pypi.org", "pypi.org", "files.pythonhosted.org", "conda.anaconda.org", - "repo.prefix.dev"]) + t = _setup( + tmp_path, + probe=lambda: ["*.pypi.org", "pypi.org", "files.pythonhosted.org", "conda.anaconda.org", "repo.prefix.dev"], + ) with pytest.raises(ApplicationError) as e: await _submit_match(t) assert e.value.non_retryable and "data.example.org" in str(e.value) and t.csub.submitted == [] @@ -175,7 +194,9 @@ async def test_comment_can_carry_the_binding(tmp_path): b.write_text("record: rec-1\n") await t.a.comment(1, "Test crops: **passed**", "test r 1", "data", str(b), "cd" * 32) body = t.data.comments[1][0]["body"] - assert body.startswith("Test crops: **passed**\n\n**Binding** (sha256 `" + "cd" * 32 + "`):\n```yaml\nrecord: rec-1\n```") + assert body.startswith( + "Test crops: **passed**\n\n**Binding** (sha256 `" + "cd" * 32 + "`):\n```yaml\nrecord: rec-1\n```" + ) async def test_open_snippet_issue_twice_gives_one_issue(tmp_path): @@ -202,6 +223,7 @@ async def pr_files(n): async def dir_sha(path, ref): return "tree1" if (path, ref) == ("snippets/new-snip", SNIP_SHA) else None + t.snip.pr_files, t.snip.dir_sha = pr_files, dir_sha @@ -219,12 +241,15 @@ async def test_adopt_pr_binding_pins_it_to_the_sha(tmp_path): assert r["hash"] == hashlib.sha256(p.read_bytes()).hexdigest() -@pytest.mark.parametrize("b, files, why", [ - ({**NEW, "record": "rec-2"}, None, "rec-2"), - ({**NEW, "raw": [{**NEW["raw"][0], "urls": ["https://evil.example.com/a.tif"]}]}, None, "evil.example.com"), - (BINDING, None, "new-snip"), # the example doesn't use the PR's snippet - (NEW, ["snippets/new-snip/a.py", "snippets/other/b.py"], "exactly one"), -]) +@pytest.mark.parametrize( + "b, files, why", + [ + ({**NEW, "record": "rec-2"}, None, "rec-2"), + ({**NEW, "raw": [{**NEW["raw"][0], "urls": ["https://evil.example.com/a.tif"]}]}, None, "evil.example.com"), + (BINDING, None, "new-snip"), # the example doesn't use the PR's snippet + (NEW, ["snippets/new-snip/a.py", "snippets/other/b.py"], "exactly one"), + ], +) async def test_adopt_pr_binding_rejects(tmp_path, b, files, why): t = _setup(tmp_path) await t.a.admit(1, "99") @@ -243,8 +268,12 @@ async def test_comment_returns_its_url_once(tmp_path): def _crop(root, cid, n): d = root / "crops" / cid d.mkdir(parents=True) - files = {"raw.png": b"raw" + cid.encode(), "crop.json": json.dumps( - {"crop_id": cid, "plane": {"z": n, "c": 0}, "box": {"y": 10 * n, "x": 5, "h": 256, "w": 256}}).encode()} + files = { + "raw.png": b"raw" + cid.encode(), + "crop.json": json.dumps( + {"crop_id": cid, "plane": {"z": n, "c": 0}, "box": {"y": 10 * n, "x": 5, "h": 256, "w": 256}} + ).encode(), + } for rel, b in files.items(): (d / rel).write_bytes(b) return {"crop_id": cid, "files": {r: hashlib.sha256(b).hexdigest() for r, b in files.items()}} @@ -265,8 +294,9 @@ async def test_publish_gallery_verified_only(tmp_path): t.data.files[f"{pre}/c1/raw.png"] = b"already there" g = await t.a.publish_gallery(1, ROUND, SNIP_SHA, str(cw), str(vw), verified) assert g.prefix == pre - assert sorted(t.data.puts) == [f"{pre}/{r}" for r in - ("binding.yaml", "c1/overlay.png", "c2/overlay.png", "c2/raw.png", "verify.json")] + assert sorted(t.data.puts) == [ + f"{pre}/{r}" for r in ("binding.yaml", "c1/overlay.png", "c2/overlay.png", "c2/raw.png", "verify.json") + ] rows = [ln for ln in g.table.splitlines() if ln.startswith("| ![raw]")] assert len(rows) == 2 and all(ln.count("?raw=true") == 2 for ln in rows) and "c3" not in g.table assert "c2 · z=1 c=0 · (10, 5)" in rows[1] diff --git a/tests/test_mia_binding.py b/tests/test_mia_binding.py index 31cb84f..7c6d358 100644 --- a/tests/test_mia_binding.py +++ b/tests/test_mia_binding.py @@ -11,11 +11,25 @@ REC = yaml.safe_load((Path(__file__).parent / "fixtures/mia/records/bbbc034-hipsc-nuclei-3d.yaml").read_text()) SHA = "37e18f6e" + "0" * 32 ZIP = "https://data.broadinstitute.org/bbbc/BBBC034/" -B = {"record": "bbbc034-hipsc-nuclei-3d", "record_commit": SHA, "snippets_commit": "4c1d", - "raw": [{"snippet": "tiff-zip-member", "urls": [ZIP + "BBBC034_v1_dataset.zip::AICS_12_134_C=2.tif"], "params": {}}], - "labels": [{"snippet": "tiff-zip-member", "urls": [ZIP + "BBBC034_v1_DatasetGroundTruth.zip::ground_truth_segmented.tif"], - "params": {}, "pairs_with": 0, "label_key": "manual_gt-nucleus-instance", "segmentation_type": "instance"}], - "reasons": ["axes zyx"]} +B = { + "record": "bbbc034-hipsc-nuclei-3d", + "record_commit": SHA, + "snippets_commit": "4c1d", + "raw": [ + {"snippet": "tiff-zip-member", "urls": [ZIP + "BBBC034_v1_dataset.zip::AICS_12_134_C=2.tif"], "params": {}} + ], + "labels": [ + { + "snippet": "tiff-zip-member", + "urls": [ZIP + "BBBC034_v1_DatasetGroundTruth.zip::ground_truth_segmented.tif"], + "params": {}, + "pairs_with": 0, + "label_key": "manual_gt-nucleus-instance", + "segmentation_type": "instance", + } + ], + "reasons": ["axes zyx"], +} NAMES = {"tiff-zip-member"} @@ -27,8 +41,15 @@ def test_example_binding_passes(): assert check(copy.deepcopy(B)) == [] -@pytest.mark.parametrize("url", ["https://evil.example.org/x.zip", "http://data.broadinstitute.org/x.zip", - "ftp://data.broadinstitute.org/x.zip", "HTTPS://data.broadinstitute.org/x"]) +@pytest.mark.parametrize( + "url", + [ + "https://evil.example.org/x.zip", + "http://data.broadinstitute.org/x.zip", + "ftp://data.broadinstitute.org/x.zip", + "HTTPS://data.broadinstitute.org/x", + ], +) def test_bad_urls_rejected(url): b = copy.deepcopy(B) b["raw"][0]["urls"] = [url] @@ -66,8 +87,10 @@ def test_record_urls_include_sample_and_patterns(): def test_hosts_add_redirects_and_packages(tmp_path): - cfg = make_cfg(tmp_path, network={"redirect_hosts": {"data.broadinstitute.org": ["cdn.broad.example"]}, - "package_hosts": ["pypi.org"]}) + cfg = make_cfg( + tmp_path, + network={"redirect_hosts": {"data.broadinstitute.org": ["cdn.broad.example"]}, "package_hosts": ["pypi.org"]}, + ) urls = B["raw"][0]["urls"] + ["s3://bucket/key", "https://DATA.broadinstitute.org/y"] assert hosts(urls, cfg) == ["bucket.s3.amazonaws.com", "cdn.broad.example", "data.broadinstitute.org", "pypi.org"] with pytest.raises(ValueError): diff --git a/tests/test_mia_cli.py b/tests/test_mia_cli.py index fe2231c..ce2c310 100644 --- a/tests/test_mia_cli.py +++ b/tests/test_mia_cli.py @@ -20,8 +20,9 @@ def _storage(cfg, withdrawn=None): d = cfg.paths.storage_root / ds / cid d.mkdir(parents=True) (d / "crop.json").write_text(json.dumps({"crop_id": cid, "snippets": [{"name": snip, "commit": "1"}]})) - (cfg.paths.storage_root / ds / "manifest.json").write_text(json.dumps({"crops": [ - {"crop_id": c, "snippets": [{"name": n}], "withdrawn": False} for c, n in crops]})) + (cfg.paths.storage_root / ds / "manifest.json").write_text( + json.dumps({"crops": [{"crop_id": c, "snippets": [{"name": n}], "withdrawn": False} for c, n in crops]}) + ) def test_crops_by_snippet(monkeypatch, tmp_path, capsys): @@ -71,8 +72,14 @@ async def _aw(req): async def login(cfg): return "bot" - probe = SimpleNamespace(agents=["claude"], allowed_hosts=hosts, user="u", env_allow=list(env_allow), - allowed_roots=list(roots), queues=queues or {"short": {}, "local": {}}) + probe = SimpleNamespace( + agents=["claude"], + allowed_hosts=hosts, + user="u", + env_allow=list(env_allow), + allowed_roots=list(roots), + queues=queues or {"short": {}, "local": {}}, + ) monkeypatch.setattr(cli, "_lsf_queues", lambda: {"short", "local", "gpu_a100"}) monkeypatch.setattr(cli, "GitHub", _GH) monkeypatch.setattr(cli, "_client", client) @@ -103,8 +110,11 @@ def test_check_reports_every_row_and_fails_on_one(monkeypatch, tmp_path, capsys) def test_check_flags_unallowed_data_host(monkeypatch, tmp_path, capsys): - _fake_env(monkeypatch, [h for h in DATA_HOSTS if h != "zenodo.org"] + ["pypi.org", "files.pythonhosted.org", - "conda.anaconda.org", "repo.prefix.dev"]) + _fake_env( + monkeypatch, + [h for h in DATA_HOSTS if h != "zenodo.org"] + + ["pypi.org", "files.pythonhosted.org", "conda.anaconda.org", "repo.prefix.dev"], + ) monkeypatch.setattr(_GH, "request", lambda self, m, p: _ok(self)) for d in ("state", "work", "storage"): (tmp_path / d).mkdir() @@ -145,17 +155,24 @@ def test_check_flags_csub_env_roots_and_home(monkeypatch, tmp_path, capsys): def test_check_flags_policy_queue_lsf_does_not_have(monkeypatch, tmp_path, capsys): - _fake_env(monkeypatch, ["*"], queues={"short": {}, "long": {}, "gpu": {"gpu": True}}) # gpu tables are not LSF names + _fake_env( + monkeypatch, ["*"], queues={"short": {}, "long": {}, "gpu": {"gpu": True}} + ) # gpu tables are not LSF names cfg = make_cfg(tmp_path) monkeypatch.setattr(cli, "load_config", lambda _: cfg) monkeypatch.setattr(cli, "_setup_logging", lambda *a: None) assert main(["check"]) == 1 row = next(line for line in capsys.readouterr().out.split("\n") if "csub broker" in line) - assert row.startswith("BAD") and "not an LSF queue: long" in row and "gpu" not in row.split("not an LSF queue:")[1].split("(")[0] + assert ( + row.startswith("BAD") + and "not an LSF queue: long" in row + and "gpu" not in row.split("not an LSF queue:")[1].split("(")[0] + ) def test_help_lists_devops_commands(capsys): import pytest + with pytest.raises(SystemExit) as e: main(["--help"]) assert e.value.code == 0 @@ -168,11 +185,13 @@ def test_devops_commands_drive_the_mia_session(monkeypatch, tmp_path, capsys): import subprocess from autodev import ops + calls = [] def run(argv, **kw): calls.append((argv, kw)) return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") + monkeypatch.setattr(ops.subprocess, "run", run) cfg = make_cfg(tmp_path) monkeypatch.setattr(cli, "load_config", lambda _: cfg) diff --git a/tests/test_mia_config.py b/tests/test_mia_config.py index 7f3b8f6..d9429b0 100644 --- a/tests/test_mia_config.py +++ b/tests/test_mia_config.py @@ -18,23 +18,29 @@ def test_defaults(tmp_path): assert c.crops.licences[0] == "CC0-1.0" and c.gallery.branch == "galleries" -@pytest.mark.parametrize("override", [ - {"paths": {"state_dir": "{tmp}/work/s"}}, - {"network": {"package_hosts": ["api.github.com"]}}, - {"network": {"redirect_hosts": {"a.example": ["*.b.example"]}}}, - {"limits": {"max_pr_revisions": 0}}, - {"limits": {"max_stage_attempts": 2}}, # retries aren't implemented - {"crops": {"licences": []}}, -]) +@pytest.mark.parametrize( + "override", + [ + {"paths": {"state_dir": "{tmp}/work/s"}}, + {"network": {"package_hosts": ["api.github.com"]}}, + {"network": {"redirect_hosts": {"a.example": ["*.b.example"]}}}, + {"limits": {"max_pr_revisions": 0}}, + {"limits": {"max_stage_attempts": 2}}, # retries aren't implemented + {"crops": {"licences": []}}, + ], +) def test_rejected(tmp_path, override): - override = {k: {a: (b.replace("{tmp}", str(tmp_path)) if isinstance(b, str) else b) for a, b in v.items()} - for k, v in override.items()} + override = { + k: {a: (b.replace("{tmp}", str(tmp_path)) if isinstance(b, str) else b) for a, b in v.items()} + for k, v in override.items() + } with pytest.raises(ValidationError): make_cfg(tmp_path, **override) -@pytest.mark.parametrize("key, value", [("allow_hosts", ["x.example.org"]), ("extra_mounts", ["/data:ro"]), - ("api_key_env", "KEY")]) +@pytest.mark.parametrize( + "key, value", [("allow_hosts", ["x.example.org"]), ("extra_mounts", ["/data:ro"]), ("api_key_env", "KEY")] +) def test_rejects_autodev_only_agent_settings(tmp_path, key, value): with pytest.raises(ValidationError, match=f"agent.{key} is not used by mia-data"): make_cfg(tmp_path, agent={key: value}) @@ -46,8 +52,13 @@ def test_via_proxy_env_override(monkeypatch): def test_roots_take_a_path_or_a_path_with_browse_url(tmp_path): - c = make_cfg(tmp_path, paths={"work_root": {"path": str(tmp_path / "w"), "url": "https://fg.example/browse/w/"}, - "storage_root": str(tmp_path / "crops")}) + c = make_cfg( + tmp_path, + paths={ + "work_root": {"path": str(tmp_path / "w"), "url": "https://fg.example/browse/w/"}, + "storage_root": str(tmp_path / "crops"), + }, + ) assert c.paths.work_root == (tmp_path / "w").resolve() and c.paths.storage_root == (tmp_path / "crops").resolve() assert c.paths.link(c.paths.work_root / "issue-5" / "round x") == "https://fg.example/browse/w/issue-5/round%20x" assert c.paths.link(c.paths.work_root) == "https://fg.example/browse/w" diff --git a/tests/test_mia_gallery.py b/tests/test_mia_gallery.py index 77fd5fc..d7686fe 100644 --- a/tests/test_mia_gallery.py +++ b/tests/test_mia_gallery.py @@ -43,7 +43,14 @@ def _tree(tmp_path, ids=("c1", "c2")): def test_gallery_files_only_verified(tmp_path): cw, vw, verified = _tree(tmp_path, ("c1", "c2", "c3")) files = gallery_files(cw, vw, verified[:2]) - assert sorted(files) == ["binding.yaml", "c1/overlay.png", "c1/raw.png", "c2/overlay.png", "c2/raw.png", "verify.json"] + assert sorted(files) == [ + "binding.yaml", + "c1/overlay.png", + "c1/raw.png", + "c2/overlay.png", + "c2/raw.png", + "verify.json", + ] (cw / "crops" / "c1" / "raw.png").write_bytes(b"swapped") with pytest.raises(ValueError): gallery_files(cw, vw, verified[:1]) diff --git a/tests/test_mia_jobspec.py b/tests/test_mia_jobspec.py index 3db26c8..50c39e5 100644 --- a/tests/test_mia_jobspec.py +++ b/tests/test_mia_jobspec.py @@ -27,13 +27,17 @@ from mia_data.models import Pins RECORD = {"data": {"download_url": "https://data.example.org/a.zip"}} -BINDING = {"record": "rec-1", "record_commit": "c" * 40, - "raw": [{"snippet": "tiff-zip-member", "urls": ["https://data.example.org/a.zip::m.tif"], "params": {}}]} +BINDING = { + "record": "rec-1", + "record_commit": "c" * 40, + "raw": [{"snippet": "tiff-zip-member", "urls": ["https://data.example.org/a.zip::m.tif"], "params": {}}], +} def _cfg(tmp_path): - cfg = make_cfg(tmp_path, limits={"job_bytes": 1e6}, - network={"redirect_hosts": {"data.example.org": ["cdn.example.net"]}}) + cfg = make_cfg( + tmp_path, limits={"job_bytes": 1e6}, network={"redirect_hosts": {"data.example.org": ["cdn.example.net"]}} + ) h = cfg.harness.dir (h / "harness").mkdir() (h / "harness" / "pixi.lock").write_text("lock v1\n") @@ -77,11 +81,15 @@ def test_snapshot_extracts_once_read_only(tmp_path, monkeypatch): sha = run_git("rev-parse", "HEAD", cwd=src) archives = [] real = subprocess.run - monkeypatch.setattr(jobspec.subprocess, "run", lambda a, **k: (archives.append(a) if "archive" in a else None, real(a, **k))[1]) + monkeypatch.setattr( + jobspec.subprocess, "run", lambda a, **k: (archives.append(a) if "archive" in a else None, real(a, **k))[1] + ) d = snapshot(cfg, sha) assert snapshot(cfg, sha) == d == cfg.paths.work_root / "_snapshots" / sha and len(archives) == 1 assert sorted(p.name for p in d.iterdir()) == ["SPEC.md", "WITHDRAWN.yaml", "snippets"] - assert all(_readonly(p) for p in [d, d / "SPEC.md", d / "snippets" / "npy", d / "snippets" / "npy" / "snippet.yaml"]) + assert all( + _readonly(p) for p in [d, d / "SPEC.md", d / "snippets" / "npy", d / "snippets" / "npy" / "snippet.yaml"] + ) assert (cfg.paths.state_dir / "mirror" / "mia-snippets.git" / "HEAD").exists() # a commit pushed after the mirror was made is fetched on demand (src / "snippets" / "npy" / "README.md").write_text("new\n") @@ -102,6 +110,7 @@ def run(a, **k): archives.append(a) time.sleep(0.2) # widen the race return real(a, **k) + monkeypatch.setattr(jobspec.subprocess, "run", run) with ThreadPoolExecutor(2) as ex: a, b = ex.map(lambda _: snapshot(cfg, sha), range(2)) @@ -161,7 +170,9 @@ def test_match_job(tmp_path): @pytest.mark.parametrize("trusted", [False, True]) def test_crop_job_cache_mode(tmp_path, trusted): cfg, ws = _cfg(tmp_path), _ws(tmp_path) - s = crop_job(cfg, ws, _pins(cfg), "binding.yaml", tmp_path / "snap", "record.yaml", 4, "s-t", "i1-e2", 0, None, trusted) + s = crop_job( + cfg, ws, _pins(cfg), "binding.yaml", tmp_path / "snap", "record.yaml", 4, "s-t", "i1-e2", 0, None, trusted + ) cache = str(cfg.paths.work_root / "_cache" / "rec-1") assert (cache in s.rw_mounts, cache in s.ro_mounts) == (trusted, not trusted) assert s.env["MIA_HARNESS_CACHE_WRITABLE"] == ("1" if trusted else "0") and Path(cache).is_dir() @@ -198,10 +209,14 @@ def test_check_hosts_allowed(tmp_path): def _builders(cfg, ws, pins, tmp_path): snap = tmp_path / "snap" - return [lambda: match_job(cfg, ws, pins, "record.yaml"), - lambda: smoke_job(cfg, ws, pins, "binding.yaml", snap, "record.yaml"), - lambda: crop_job(cfg, ws, pins, "binding.yaml", snap, "record.yaml", 4, "s", "i1-e2", 1, tmp_path / "m.json", True), - lambda: verify_job(cfg, ws, pins, tmp_path / "crops", "binding.yaml", None)] + return [ + lambda: match_job(cfg, ws, pins, "record.yaml"), + lambda: smoke_job(cfg, ws, pins, "binding.yaml", snap, "record.yaml"), + lambda: crop_job( + cfg, ws, pins, "binding.yaml", snap, "record.yaml", 4, "s", "i1-e2", 1, tmp_path / "m.json", True + ), + lambda: verify_job(cfg, ws, pins, tmp_path / "crops", "binding.yaml", None), + ] def test_every_builder_sets_harness_env(tmp_path): diff --git a/tests/test_mia_publish.py b/tests/test_mia_publish.py index 419a88a..7df8368 100644 --- a/tests/test_mia_publish.py +++ b/tests/test_mia_publish.py @@ -17,9 +17,13 @@ def _src(tmp_path, ids=("c1", "c2"), snippet="tiff-zip-member", name="src"): for c in ids: d = src / c (d / "labels").mkdir(parents=True) - files = {"raw.png": b"png" + c.encode(), "labels/k-x-instance.png": b"lab" + c.encode(), - "crop.json": json.dumps({"crop_id": c, "plane": {"z": 0, "c": 0}, - "snippets": [{"name": snippet, "commit": "4c1d"}]}).encode()} + files = { + "raw.png": b"png" + c.encode(), + "labels/k-x-instance.png": b"lab" + c.encode(), + "crop.json": json.dumps( + {"crop_id": c, "plane": {"z": 0, "c": 0}, "snippets": [{"name": snippet, "commit": "4c1d"}]} + ).encode(), + } for rel, data in files.items(): (d / rel).write_bytes(data) verified.append({"crop_id": c, "files": {r: hashlib.sha256(v).hexdigest() for r, v in files.items()}}) @@ -72,6 +76,7 @@ def test_concurrent_publishers(tmp_path, monkeypatch): def lockstep(a, b): # both attempts have copied the crop; both now rename it barrier.wait(timeout=10) rename(a, b) + monkeypatch.setattr("mia_data.publish.os.rename", lockstep) def run(src, verified): @@ -79,6 +84,7 @@ def run(src, verified): results.append(publish(ds, src, verified, {}, tmp_max_age_s=AGE)) except Exception as e: # noqa: BLE001 errors.append(e) + ts = [threading.Thread(target=run, args=s) for s in srcs] [t.start() for t in ts], [t.join() for t in ts] assert errors == [] diff --git a/tests/test_mia_seed.py b/tests/test_mia_seed.py index 3a1bce1..8aaadc8 100644 --- a/tests/test_mia_seed.py +++ b/tests/test_mia_seed.py @@ -13,13 +13,16 @@ API = "https://api.github.com" FIX = Path(__file__).parent / "fixtures" / "mia" LIC = ["CC0-1.0", "CC-BY-4.0", "CC-BY-3.0", "MIT"] -PATHS = {"bbbc034-hipsc-nuclei-3d": "datasets/3D/fluorescence-other", - "czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet": "datasets/3D/cryo-ET", - "hf-pytc-zebrafinch-j0126-ffn-em": "datasets/3D/SBF-SEM"} +PATHS = { + "bbbc034-hipsc-nuclei-3d": "datasets/3D/fluorescence-other", + "czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet": "datasets/3D/cryo-ET", + "hf-pytc-zebrafinch-j0126-ffn-em": "datasets/3D/SBF-SEM", +} def _gh(repo): async def nosleep(s): ... + return GitHub(repo, lambda: "tok", writes_per_second=1000, sleep=nosleep) @@ -41,13 +44,19 @@ def mock_catalog(ids=None, existing=()): del paths["synthetic-gpl"] for f in ("downloads", "downloads-native"): respx.get(f"{API}/repos/o/catalog/contents/state/{f}.json", params={"ref": "state"}).respond( - 200, content=(FIX / f"{f}.json").read_bytes()) - respx.get(f"{API}/repos/o/catalog/git/trees/main").respond(200, json={"tree": [ - {"type": "blob", "path": p} for p in paths.values()] + [{"type": "tree", "path": "datasets"}]}) + 200, content=(FIX / f"{f}.json").read_bytes() + ) + respx.get(f"{API}/repos/o/catalog/git/trees/main").respond( + 200, + json={"tree": [{"type": "blob", "path": p} for p in paths.values()] + [{"type": "tree", "path": "datasets"}]}, + ) for i, p in paths.items(): respx.get(f"{API}/repos/o/catalog/contents/{p}", params={"ref": "main"}).respond( - 200, content=(FIX / "records" / f"{i}.yaml").read_bytes()) - respx.get(f"{API}/repos/o/data/issues").respond(200, json=[{"body": f""} for i in existing]) + 200, content=(FIX / "records" / f"{i}.yaml").read_bytes() + ) + respx.get(f"{API}/repos/o/data/issues").respond( + 200, json=[{"body": f""} for i in existing] + ) return respx.post(f"{API}/repos/o/data/issues").respond(201, json={"number": 1}) @@ -67,7 +76,10 @@ async def test_three_fixtures_make_three_issues(tmp_path, gh_data, gh_catalog): assert "native" in b["body"] and "tensorswitch" in b["body"] and "[52, 1024, 1024]" in b["body"] assert "blob/main/datasets/3D/fluorescence-other/bbbc034" in b["body"] c = by_id["czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet"] - assert c["title"] == "czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet: M. pneumoniae chloramphenicol ribosomes (cryo-ET)" + assert ( + c["title"] + == "czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet: M. pneumoniae chloramphenicol ribosomes (cryo-ET)" + ) assert c["body"].startswith("") assert set(c["labels"]) == {"data:new", "modality:cryo-ET", "has-labels"} z = by_id["hf-pytc-zebrafinch-j0126-ffn-em"] @@ -121,15 +133,21 @@ async def test_downloaded_union_keeps_newest_and_both_downloaders(gh_catalog): d = await catalog.downloaded(gh_catalog) e = d["bbbc034-hipsc-nuclei-3d"] assert e["downloaders"] == ["tensorswitch", "native"] - ats = [json.loads((FIX / f).read_text())["downloaded"]["bbbc034-hipsc-nuclei-3d"]["at"] - for f in ("downloads.json", "downloads-native.json")] + ats = [ + json.loads((FIX / f).read_text())["downloaded"]["bbbc034-hipsc-nuclei-3d"]["at"] + for f in ("downloads.json", "downloads-native.json") + ] assert e["at"] == max(ats) assert d["czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet"]["downloaders"] == ["tensorswitch"] def test_shape_without_axes_uses_last_two_dims(): - rec = {"id": "x", "title": "t", "license": {"spdx": "MIT"}, "technical": {"arrays": [ - {"role": "raw", "axes": None, "shape": [10, 512, 512], "dtype": "uint8"}]}} + rec = { + "id": "x", + "title": "t", + "license": {"spdx": "MIT"}, + "technical": {"arrays": [{"role": "raw", "axes": None, "shape": [10, 512, 512], "dtype": "uint8"}]}, + } entry = {"record_commit": "a" * 40, "dataset_dir": "/d", "downloaders": ["native"]} assert "maybe-small" not in render_issue(rec, entry, LIC)[2] @@ -137,10 +155,15 @@ def test_shape_without_axes_uses_last_two_dims(): @respx.mock async def test_forged_marker_in_record_text_does_not_block_another(tmp_path, gh_data, gh_catalog): route = mock_catalog(ids={"bbbc034-hipsc-nuclei-3d", "hf-pytc-zebrafinch-j0126-ffn-em"}) - rec = (FIX / "records" / "bbbc034-hipsc-nuclei-3d.yaml").read_text().replace( - "Homo sapiens", "x y\\n|z") - respx.get(f"{API}/repos/o/catalog/contents/datasets/3D/fluorescence-other/bbbc034-hipsc-nuclei-3d.yaml", - params={"ref": "main"}).respond(200, content=rec.encode()) + rec = ( + (FIX / "records" / "bbbc034-hipsc-nuclei-3d.yaml") + .read_text() + .replace("Homo sapiens", "x y\\n|z") + ) + respx.get( + f"{API}/repos/o/catalog/contents/datasets/3D/fluorescence-other/bbbc034-hipsc-nuclei-3d.yaml", + params={"ref": "main"}, + ).respond(200, content=rec.encode()) respx.get(f"{API}/repos/o/data/issues").respond(200, json=[]) made = await seed(make_cfg(tmp_path), gh_data, gh_catalog, False) assert "hf-pytc-zebrafinch-j0126-ffn-em" in made @@ -148,27 +171,40 @@ async def test_forged_marker_in_record_text_does_not_block_another(tmp_path, gh_ assert b["body"].count("") is None @respx.mock async def test_record_with_other_id_inside_is_skipped(tmp_path, gh_data, gh_catalog, caplog): route = mock_catalog(ids={"bbbc034-hipsc-nuclei-3d", "hf-pytc-zebrafinch-j0126-ffn-em"}) - rec = (FIX / "records" / "bbbc034-hipsc-nuclei-3d.yaml").read_text().replace( - "id: bbbc034-hipsc-nuclei-3d", "id: hf-pytc-zebrafinch-j0126-ffn-em", 1) + rec = ( + (FIX / "records" / "bbbc034-hipsc-nuclei-3d.yaml") + .read_text() + .replace("id: bbbc034-hipsc-nuclei-3d", "id: hf-pytc-zebrafinch-j0126-ffn-em", 1) + ) assert "id: hf-pytc" in rec - respx.get(f"{API}/repos/o/catalog/contents/datasets/3D/fluorescence-other/bbbc034-hipsc-nuclei-3d.yaml", - params={"ref": "main"}).respond(200, content=rec.encode()) + respx.get( + f"{API}/repos/o/catalog/contents/datasets/3D/fluorescence-other/bbbc034-hipsc-nuclei-3d.yaml", + params={"ref": "main"}, + ).respond(200, content=rec.encode()) assert await seed(make_cfg(tmp_path), gh_data, gh_catalog, False) == ["hf-pytc-zebrafinch-j0126-ffn-em"] (b,) = bodies(route) assert b["title"].startswith("hf-pytc-zebrafinch") and b["body"].count("mia-data:record=") == 1 assert "bbbc034" in caplog.text -@pytest.mark.parametrize("rid, folder", [ - ("bbbc034-hipsc-nuclei-3d", "lm-human-bbbc034-hipsc-nuclei-3d"), # as demo-native's dataset_dir - ("hf-pytc-zebrafinch-j0126-ffn-em", "em-taeniopygia-guttata-hf-pytc-zebrafinch-j0126-ffn"), - ("czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet", "em-mycoplasmoides-pneumoniae-mpneumoniae-chloramphenicol-ribosomes")]) +@pytest.mark.parametrize( + "rid, folder", + [ + ("bbbc034-hipsc-nuclei-3d", "lm-human-bbbc034-hipsc-nuclei-3d"), # as demo-native's dataset_dir + ("hf-pytc-zebrafinch-j0126-ffn-em", "em-taeniopygia-guttata-hf-pytc-zebrafinch-j0126-ffn"), + ( + "czcdp-10003-mpneumoniae-chloramphenicol-ribosomes-cryoet", + "em-mycoplasmoides-pneumoniae-mpneumoniae-chloramphenicol-ribosomes", + ), + ], +) def test_dataset_folder_follows_miao_layout(rid, folder): rec = yaml.safe_load((FIX / "records" / f"{rid}.yaml").read_text()) assert catalog.dataset_folder(rec, rid) == folder diff --git a/tests/test_mia_status.py b/tests/test_mia_status.py index 40662e5..df61787 100644 --- a/tests/test_mia_status.py +++ b/tests/test_mia_status.py @@ -2,9 +2,16 @@ def rows(n, cid="i1-e2-b1-{:03d}"): - return [{"crop_id": cid.format(i), "raw_url": f"https://github.com/o/d/blob/g/p/{i}/raw.png?raw=true", - "overlay_url": f"https://github.com/o/d/blob/g/p/{i}/overlay.png?raw=true", - "plane": {"z": i, "c": 0}, "box": {"y": 10, "x": 20}} for i in range(n)] + return [ + { + "crop_id": cid.format(i), + "raw_url": f"https://github.com/o/d/blob/g/p/{i}/raw.png?raw=true", + "overlay_url": f"https://github.com/o/d/blob/g/p/{i}/overlay.png?raw=true", + "plane": {"z": i, "c": 0}, + "box": {"y": 10, "x": 20}, + } + for i in range(n) + ] def test_crops_table_small_plain_large_details(): @@ -28,7 +35,9 @@ def test_binding_block_fences_and_neutralizes(tmp_path): def test_links_line_lists_only_given_urls(): - assert render_links({"Workspace": None, "Crops": "https://fg.example/b/c d", "Gallery": "https://g/x"}) == \ - "**Folders:** [Crops](https://fg.example/b/c%20d) · [Gallery](https://g/x)\n" + assert ( + render_links({"Workspace": None, "Crops": "https://fg.example/b/c d", "Gallery": "https://g/x"}) + == "**Folders:** [Crops](https://fg.example/b/c%20d) · [Gallery](https://g/x)\n" + ) assert render_links({"Workspace": None}) == "" assert "") diff --git a/tests/test_mia_test_pr.py b/tests/test_mia_test_pr.py index d2515d9..7fa6bdb 100644 --- a/tests/test_mia_test_pr.py +++ b/tests/test_mia_test_pr.py @@ -25,9 +25,22 @@ def submit(self, spec): else: (ws / "preview" / "c1").mkdir(parents=True) (ws / "preview" / "c1" / "overlay.png").write_bytes(b"ov") - (ws / "verify.json").write_text(json.dumps({"n": 1, "errors": [], "crops": [ - {"crop_id": "c1", "ok": self.verify_ok, "errors": [] if self.verify_ok else ["overlay empty"], - "files": self.files}]})) + (ws / "verify.json").write_text( + json.dumps( + { + "n": 1, + "errors": [], + "crops": [ + { + "crop_id": "c1", + "ok": self.verify_ok, + "errors": [] if self.verify_ok else ["overlay empty"], + "files": self.files, + } + ], + } + ) + ) return super().submit(spec) @@ -51,7 +64,9 @@ async def test_test_pr_passes(tmp_path, capsys): assert log[-1][4] == c["html_url"] and f"" in c["body"] assert "1 of 1 crops verified" in c["body"] and "?raw=true" in c["body"] pre = f"galleries/_test-pr/rec-1/{PR}/{SNIP_SHA}" - assert sorted(t.data.puts) == [f"{pre}/{r}" for r in ("binding.yaml", "c1/overlay.png", "c1/raw.png", "verify.json")] + assert sorted(t.data.puts) == [ + f"{pre}/{r}" for r in ("binding.yaml", "c1/overlay.png", "c1/raw.png", "verify.json") + ] crop = t.csub.submitted[0] assert f"--n 4 --seed test-pr-{PR}-{SNIP_SHA}" in crop["command"] assert crop["cwd"].startswith(str(t.cfg.paths.work_root / "test-pr" / str(PR) / SNIP_SHA)) diff --git a/tests/test_mia_workflows.py b/tests/test_mia_workflows.py index 5591e9d..65a9858 100644 --- a/tests/test_mia_workflows.py +++ b/tests/test_mia_workflows.py @@ -54,9 +54,22 @@ def shift(iso, seconds): class Mocks: - def __init__(self, outcome="matched", smoke=None, states=None, test_ok=4, prod_ok=16, - n_effective=16, await_failures=0, gate=None, gate_on="", test_ok_by=None, pr_now=None, trees=None, - revisions=5): + def __init__( + self, + outcome="matched", + smoke=None, + states=None, + test_ok=4, + prod_ok=16, + n_effective=16, + await_failures=0, + gate=None, + gate_on="", + test_ok_by=None, + pr_now=None, + trees=None, + revisions=5, + ): self.calls: list[tuple[str, tuple]] = [] self.outcome, self.smoke, self.states = outcome, smoke or SmokeResult(True, True), states or {} self.test_ok, self.prod_ok, self.n_effective = test_ok, prod_ok, n_effective @@ -77,13 +90,20 @@ async def limits() -> RoundLimits: return RoundLimits(4, 16, 120, self.revisions) @activity.defn(name="set_stage") - async def set_stage(issue: int, stage: str, state: DatasetState | None = None, detail: str = "", - extra: str = "") -> None: + async def set_stage( + issue: int, stage: str, state: DatasetState | None = None, detail: str = "", extra: str = "" + ) -> None: rec("set_stage", stage, detail, extra) @activity.defn(name="comment") - async def comment(issue: int, body: str, marker: str, repo: str = "data", binding_path: str | None = None, - binding_hash: str | None = None) -> str | None: + async def comment( + issue: int, + body: str, + marker: str, + repo: str = "data", + binding_path: str | None = None, + binding_hash: str | None = None, + ) -> str | None: rec("comment", body, marker, repo, binding_path) return f"https://github.com/o/{repo}/issues/{issue}#{marker}" if repo == "snippets" else None @@ -109,8 +129,13 @@ async def submit(key: str, kind: str, a: JobArgs) -> str: @activity.defn(name="read_match") async def read_match(ws: str) -> MatchResult: rec("read_match") - return MatchResult(self.outcome, reason="label class undecidable", binding_path="/s/binding-1.yaml", - binding_hash="bh", reasons=["axes from conversion.json"]) + return MatchResult( + self.outcome, + reason="label class undecidable", + binding_path="/s/binding-1.yaml", + binding_hash="bh", + reasons=["axes from conversion.json"], + ) @activity.defn(name="read_smoke") async def read_smoke(ws: str) -> SmokeResult: @@ -123,13 +148,19 @@ async def read_verify(ws: str, crop_ws: str | None = None) -> VerifyResult: prod = "production" in ws test_ok = next((v for s, v in self.test_ok_by.items() if s in ws), self.test_ok) # by tested SHA k, n = (self.prod_ok, 16) if prod else (test_ok, 4) - return VerifyResult(k, n, [] if k == n else ["c9: overlaps c1"], - [{"crop_id": f"c{i}", "files": {}} for i in range(k)], self.n_effective if prod else 4, - bytes_fetched=1234) + return VerifyResult( + k, + n, + [] if k == n else ["c9: overlaps c1"], + [{"crop_id": f"c{i}", "files": {}} for i in range(k)], + self.n_effective if prod else 4, + bytes_fetched=1234, + ) @activity.defn(name="publish_gallery") - async def publish_gallery(issue: int, round_id: str, kind: str, crop_ws: str, verify_ws: str, - verified: list[dict]) -> GalleryResult: + async def publish_gallery( + issue: int, round_id: str, kind: str, crop_ws: str, verify_ws: str, verified: list[dict] + ) -> GalleryResult: rec("publish_gallery", kind, len(verified)) return GalleryResult(f"galleries/rec-1/{round_id}/{kind}", {}, f"table {kind}") @@ -162,9 +193,26 @@ async def adopt_pr_binding(round_id: str, pr: int, sha: str) -> dict: rec("adopt_pr_binding", pr, sha) return {"path": f"/s/binding-pr-{sha[:7]}.yaml", "hash": "h" + sha[:7], "name": "new-snip", "errors": []} - return [admit, limits, set_stage, comment, refuse_label, prepare_match, prepare_job, submit, read_match, - read_smoke, read_verify, publish_gallery, publish_crops, open_snippet_issue, pr_state, set_crop_status, - tree_sha, adopt_pr_binding] + return [ + admit, + limits, + set_stage, + comment, + refuse_label, + prepare_match, + prepare_job, + submit, + read_match, + read_smoke, + read_verify, + publish_gallery, + publish_crops, + open_snippet_issue, + pr_state, + set_crop_status, + tree_sha, + adopt_pr_binding, + ] def await_act(self): @activity.defn(name="await_job") @@ -217,10 +265,18 @@ async def idle(h, m, stage): async def started(env, m, first=None, runner=None): tq = f"tq-{uuid.uuid4()}" kw = {"workflow_runner": runner} if runner else {} - async with Worker(env.client, task_queue=tq, workflows=[MiaDataDataset], activities=m.acts(), **kw), \ - Worker(env.client, task_queue=tq + "-await", activities=m.await_act()): - h0 = await env.client.start_workflow(MiaDataDataset.run, DatasetState(7), id=f"wf-{uuid.uuid4()}", - task_queue=tq, start_signal="go", start_signal_args=[first or ev("go", "11")]) + async with ( + Worker(env.client, task_queue=tq, workflows=[MiaDataDataset], activities=m.acts(), **kw), + Worker(env.client, task_queue=tq + "-await", activities=m.await_act()), + ): + h0 = await env.client.start_workflow( + MiaDataDataset.run, + DatasetState(7), + id=f"wf-{uuid.uuid4()}", + task_queue=tq, + start_signal="go", + start_signal_args=[first or ev("go", "11")], + ) h = env.client.get_workflow_handle(h0.id) # follows continue-as-new h.first_run_id = h0.run_id try: @@ -245,8 +301,16 @@ async def test_match_test_review_approve_crop_done(env): assert m.stages == ["matching", "testing", "review", "cropping", "done"] assert [a[1] for a in m.of("submit")] == ["match", "smoke", "crop", "verify", "crop", "verify"] test, prod = [a for a in m.jobs() if a.n_crops] - assert (test.n_crops, test.batch, test.trusted, test.sha, prod.n_crops, prod.batch, prod.trusted, prod.existing) == \ - (4, 0, True, SHA, 16, 1, True, True) + assert ( + test.n_crops, + test.batch, + test.trusted, + test.sha, + prod.n_crops, + prod.batch, + prod.trusted, + prod.existing, + ) == (4, 0, True, SHA, 16, 1, True, True) assert [a[0] for a in m.of("publish_gallery")] == [SHA, "production"] assert m.of("publish_crops") == [("/w/i7-e11/crop-production-1", 16, SHA)] seq = [n for n in m.names() if n in ("read_verify", "publish_gallery", "publish_crops", "comment")] @@ -288,7 +352,7 @@ async def test_stale_approve_is_refused(env): assert m.stages[-1] == "review" await approve_review(h, m, "13") await idle(h, m, "done") - (label, reason, eid), = m.of("refuse_label") + ((label, reason, eid),) = m.of("refuse_label") assert (label, eid) == ("data:approve", "12") and "older" in reason @@ -313,7 +377,7 @@ async def test_go_while_busy_is_rejected_with_comment(env): await idle(h, m, "ineligible") await h.signal(MiaDataDataset.go, ev("go", "13")) # arrives while idle: a new round await until(lambda: len(m.of("admit")) == 2) - (label, reason, eid), = m.of("refuse_label") + ((label, reason, eid),) = m.of("refuse_label") assert (label, eid) == ("data:go", "12") and "busy" in reason assert m.of("admit") == [("11",), ("13",)] @@ -342,12 +406,15 @@ async def test_blocked_goes_needs_human(env): assert [a[1] for a in m.of("submit")] == ["match"] -@pytest.mark.parametrize("states, stages, published", [ - ({"match": ("EXIT", 1)}, ["matching", "needs-human"], []), - ({"smoke": ("EXIT", 1)}, ["matching", "needs-human"], []), # even though smoke.json says ok - ({f"verify-{SHA}": ("EXIT", 1)}, ["matching", "testing", "needs-human"], []), - ({"crop-production": ("EXIT", 1)}, ["matching", "testing", "review", "cropping", "needs-human"], [16]), -]) +@pytest.mark.parametrize( + "states, stages, published", + [ + ({"match": ("EXIT", 1)}, ["matching", "needs-human"], []), + ({"smoke": ("EXIT", 1)}, ["matching", "needs-human"], []), # even though smoke.json says ok + ({f"verify-{SHA}": ("EXIT", 1)}, ["matching", "testing", "needs-human"], []), + ({"crop-production": ("EXIT", 1)}, ["matching", "testing", "review", "cropping", "needs-human"], [16]), + ], +) async def test_failed_job_needs_human_no_retry(env, states, stages, published): m = Mocks(states=states) async with started(env, m) as h: @@ -371,6 +438,7 @@ async def test_production_verify_exit_is_judged_by_its_report(env): async def test_rollover_in_review_then_approve(env, monkeypatch): from temporalio import workflow + monkeypatch.setattr(workflows, "_rollover_due", lambda: workflow.info().continued_run_id is None) # once m = Mocks() async with started(env, m, runner=UnsandboxedWorkflowRunner()) as h: @@ -384,7 +452,9 @@ async def test_rollover_in_review_then_approve(env, monkeypatch): st = await h.query(MiaDataDataset.status) assert (await h.describe()).run_id != h.first_run_id assert m.stages == ["matching", "testing", "review", "cropping", "done"] and len(m.of("admit")) == 1 - assert [a.seed for a in m.jobs() if a.n_crops] == [st.seed + "-t", st.seed + "-p"] and "16 new" in m.of("set_stage")[-1][2] + assert [a.seed for a in m.jobs() if a.n_crops] == [st.seed + "-t", st.seed + "-p"] and "16 new" in m.of( + "set_stage" + )[-1][2] async def test_test_crop_failure_never_produces(env): @@ -561,12 +631,16 @@ async def test_merge_signal_before_test_finishes(env): await idle(h, m, "done") assert m.stages == ["matching", "developing", "testing", "cropping", "done"] assert statuses(m) == [(H1, "pending"), (H1, "success")] - assert [(a.sha, a.trusted) for a in m.jobs() if a.n_crops] == [(H1, False), (MC, True)] # h1's test carried the merge + assert [(a.sha, a.trusted) for a in m.jobs() if a.n_crops] == [ + (H1, False), + (MC, True), + ] # h1's test carried the merge assert m.names().index("tree_sha") > m.names().index("read_verify") # --- the poll activity ------------------------------------------------------------------------- + class PollGH(FakeGH): def __init__(self, repo, issues=None, events=None, perms=None, prs=None, changed=None, status=None): super().__init__(repo, issues=issues) @@ -608,6 +682,7 @@ def get_workflow_handle(self, wid): class H: async def signal(self, name, arg): signals.append((wid, name, arg)) + return H() @@ -616,8 +691,14 @@ def issue(n, *labels, body="", user="bot"): def pr(n, ref, sha, state="open", repo="o/snippets"): - return {"number": n, "state": state, "updated_at": "2099-01-01T00:00:00Z", "merged_at": None, - "merge_commit_sha": None, "head": {"ref": ref, "sha": sha, "repo": {"full_name": repo}}} + return { + "number": n, + "state": state, + "updated_at": "2099-01-01T00:00:00Z", + "merged_at": None, + "merge_commit_sha": None, + "head": {"ref": ref, "sha": sha, "repo": {"full_name": repo}}, + } def _poll(tmp_path, data, snip): @@ -627,15 +708,26 @@ def _poll(tmp_path, data, snip): async def test_poll_refuses_go_on_licence_excluded(tmp_path): at = "2026-10-12T10:00:00Z" - data = PollGH("o/data", issues={1: issue(1, "data:go", "licence-excluded"), 2: issue(2, "data:go"), - 3: issue(3, "data:approve")}, - events={(1, "data:go"): ("alice", "51", at), (2, "data:go"): ("alice", "52", at), - (3, "data:approve"): ("mallory", "53", at)}, perms={"alice": "write"}) + data = PollGH( + "o/data", + issues={1: issue(1, "data:go", "licence-excluded"), 2: issue(2, "data:go"), 3: issue(3, "data:approve")}, + events={ + (1, "data:go"): ("alice", "51", at), + (2, "data:go"): ("alice", "52", at), + (3, "data:approve"): ("mallory", "53", at), + }, + perms={"alice": "write"}, + ) a, fc = _poll(tmp_path, data, PollGH("o/snippets")) await a.poll() - (wf, arg, kw), = fc.started - assert (wf, arg, kw["id"], kw["task_queue"], kw["start_signal"]) == \ - ("MiaDataDataset", DatasetState(2), "mia-data/o/data/issue-2", "mia-data", "go") + ((wf, arg, kw),) = fc.started + assert (wf, arg, kw["id"], kw["task_queue"], kw["start_signal"]) == ( + "MiaDataDataset", + DatasetState(2), + "mia-data/o/data/issue-2", + "mia-data", + "go", + ) assert kw["start_signal_args"] == [LabelEvent("go", "52", "alice", at)] assert sorted(data.removed) == [(1, "data:go"), (2, "data:go"), (3, "data:approve")] assert "licence" in data.comments[1][0]["body"] and "mallory" in data.comments[3][0]["body"] @@ -644,14 +736,28 @@ async def test_poll_refuses_go_on_licence_excluded(tmp_path): async def test_poll_sets_status_for_non_snippet_pr(tmp_path): h = ["1" * 40, "2" * 40, "3" * 40, "4" * 40] - snip = PollGH("o/snippets", - issues={100: issue(100, body="\n\nNew snippet"), - 101: issue(101, body="", user="mallory")}, - prs=[pr(5, "docs", h[0]), pr(6, "feature", h[1]), pr(7, "docs2", h[2]), - pr(8, "snippets/issue-100", h[3]), pr(9, "snippets/issue-101", h[3])], - changed={5: ["README.md"], 6: ["snippets/x/snippet.py"], 7: ["SPEC.md"], - 8: ["snippets/x/snippet.py"], 9: ["snippets/y/snippet.py"]}, - status={h[2]: [{"context": "mia-data/crops", "state": "success"}]}) + snip = PollGH( + "o/snippets", + issues={ + 100: issue(100, body="\n\nNew snippet"), + 101: issue(101, body="", user="mallory"), + }, + prs=[ + pr(5, "docs", h[0]), + pr(6, "feature", h[1]), + pr(7, "docs2", h[2]), + pr(8, "snippets/issue-100", h[3]), + pr(9, "snippets/issue-101", h[3]), + ], + changed={ + 5: ["README.md"], + 6: ["snippets/x/snippet.py"], + 7: ["SPEC.md"], + 8: ["snippets/x/snippet.py"], + 9: ["snippets/y/snippet.py"], + }, + status={h[2]: [{"context": "mia-data/crops", "state": "success"}]}, + ) a, fc = _poll(tmp_path, PollGH("o/data"), snip) await a.poll() assert snip.set == [(h[0], "mia-data/crops", "success", "no snippet in this PR")] @@ -660,8 +766,13 @@ async def test_poll_sets_status_for_non_snippet_pr(tmp_path): async def test_poll_signals_a_pr_only_when_it_changes(tmp_path): k = issue(100, body="") - snip = PollGH("o/snippets", issues={100: k}, prs=[pr(8, "snippets/issue-100", H1)], - changed={8: ["snippets/x/a.py"]}, status={H1: [{"context": "mia-data/crops", "state": "pending"}]}) + snip = PollGH( + "o/snippets", + issues={100: k}, + prs=[pr(8, "snippets/issue-100", H1)], + changed={8: ["snippets/x/a.py"]}, + status={H1: [{"context": "mia-data/crops", "state": "pending"}]}, + ) a, fc = _poll(tmp_path, PollGH("o/data"), snip) await a.poll() await a.poll() # nothing changed: no signal From d12371cdd6eafd8a9ecda531c144058250f75fcb Mon Sep 17 00:00:00 2001 From: Konrad Rokicki Date: Tue, 13 Oct 2026 09:37:02 -0400 Subject: [PATCH 5/6] CI: lint and test on pull requests and pushes to main Co-Authored-By: Claude Fable 5.1 --- .github/workflows/ci.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..dfd8b49 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,17 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: prefix-dev/setup-pixi@v0.8.10 + with: + cache: true + - run: pixi run lint + - run: pixi run test From a95bfb4eedffe1d5f0465827b62e1d1e1a7a6b92 Mon Sep 17 00:00:00 2001 From: Konrad Rokicki Date: Tue, 13 Oct 2026 09:37:14 -0400 Subject: [PATCH 6/6] ruff: leave the plan notes alone Co-Authored-By: Claude Fable 5.1 --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index feb7ba0..c6126d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,6 +63,7 @@ md = "mia-data" [tool.ruff] line-length = 120 +extend-exclude = ["notes"] [tool.ruff.lint] select = ["E", "F", "W", "I", "UP", "B"]