Merge pull request #34 from 12vectors/task/42-the-report-keeps-its-headline
42 — A long closing report loses its head, which is where it says what happened
This commit is contained in:
@@ -44,11 +44,13 @@ Drivers know apps, adapters know vendors, `local/` knows this project.
|
||||
Module map for `manager/core/` (dependencies flow strictly left to right):
|
||||
|
||||
```
|
||||
config → state → taskfiles → events / github / drive / sync → agents → watch / httpd → board.py
|
||||
config → state / reports → taskfiles → events / github / drive / sync → agents → watch / httpd → board.py
|
||||
```
|
||||
|
||||
- `config.py` — paths, stages, settings, prompt/adapter/driver resolution
|
||||
- `state.py` — shared registries, event log persistence, SSE fan-out
|
||||
- `reports.py` — what the record keeps of an agent's closing report: one
|
||||
cap, one clip, for the task file and the PR body alike
|
||||
- `taskfiles.py` — reading and moving task files; the only code touching tasks/
|
||||
- `events.py` — ingests NORMALIZED events (the adapter contract), session registry
|
||||
- `github.py` — PR opening, Copilot requests, review/CI polling
|
||||
@@ -331,6 +333,20 @@ worktree must not already exist when it starts.
|
||||
review/ is how a broken launch hides. Stdout is kept in
|
||||
`local/state/agent/logs/`.
|
||||
|
||||
### What the record keeps of a report
|
||||
|
||||
The agent's closing report is the permanent record: it is appended to the
|
||||
task file, shown as the session's last entry, and carried into the PR
|
||||
body — the same text in all three, from one helper (`reports.py`) with
|
||||
one cap. A report that fits arrives whole. One that doesn't keeps **both
|
||||
ends** — the headline the report contract puts first, and the pointer
|
||||
that closes it — and loses the middle, cut on line boundaries, with one
|
||||
line of prose in its place saying how much went and naming the log under
|
||||
`local/state/agent/logs/` that still holds all of it. The reader is never
|
||||
left to infer that something was removed. A *failed* run is the deliberate
|
||||
exception: its excerpt keeps the log's tail, because for a crash the end
|
||||
is the story.
|
||||
|
||||
### A run that died
|
||||
|
||||
An agent that exits non-zero is the one outcome a person must not miss, so
|
||||
|
||||
+17
-15
@@ -20,16 +20,22 @@ from pathlib import Path
|
||||
|
||||
import config
|
||||
import events
|
||||
import reports
|
||||
import state
|
||||
from taskfiles import actor_name, find_stage_of, move_task, read_task, set_assignee
|
||||
|
||||
|
||||
def _clean_log(text: str, cap: int = 3000) -> str:
|
||||
"""An agent's -p output is its final report; strip the hook-failure
|
||||
noise other tools may have interleaved."""
|
||||
lines = [l for l in text.strip().splitlines()
|
||||
if not ("hook" in l and "failed" in l)]
|
||||
return "\n".join(lines).strip()[-cap:]
|
||||
def _report_of(record: dict, text: str | None = None) -> str:
|
||||
"""What the record keeps of a run's report: cleaned, and clipped by
|
||||
reports.report — head first, because the report's own first line is
|
||||
where it says what happened. `text` names the part after a marker when
|
||||
there is one; otherwise the whole log."""
|
||||
if text is None:
|
||||
try:
|
||||
text = Path(record["log"]).read_text(encoding="utf-8", errors="replace")
|
||||
except OSError:
|
||||
return ""
|
||||
return reports.report(text, log_path=record.get("log"))
|
||||
|
||||
|
||||
def _file_report(record: dict, heading: str, report: str) -> None:
|
||||
@@ -383,7 +389,7 @@ def _failure_excerpt(log_path: str | None, lines: int = 6, cap: int = 600) -> st
|
||||
text = Path(log_path).read_text(encoding="utf-8", errors="replace")
|
||||
except OSError:
|
||||
text = ""
|
||||
kept = [line.rstrip() for line in _clean_log(text, cap=8000).splitlines()
|
||||
kept = [line.rstrip() for line in reports.tail(text, 8000).splitlines()
|
||||
if line.strip()]
|
||||
if not kept:
|
||||
return "no output — the run died before the agent said anything"
|
||||
@@ -479,11 +485,7 @@ def _reap_agent(agent_id: str, proc: subprocess.Popen, log_file) -> None:
|
||||
summary = (f"{name} declined {filename} — not ready: {declined}"
|
||||
+ ("" if cleaned else f" (worktree {record['worktree']} kept: it has commits)"))
|
||||
elif rc == 0 and not stopped:
|
||||
try:
|
||||
report = _clean_log(Path(record["log"]).read_text(encoding="utf-8",
|
||||
errors="replace"))
|
||||
except OSError:
|
||||
report = ""
|
||||
report = _report_of(record)
|
||||
_file_report(record, "Work report", report)
|
||||
_session_report(record, report)
|
||||
if _no_new_commits(record):
|
||||
@@ -614,7 +616,7 @@ def _reap_pr_fix(agent_id: str, proc: subprocess.Popen, log_file) -> None:
|
||||
except OSError:
|
||||
text = ""
|
||||
idx = text.find("ADDRESSED:")
|
||||
report = text[idx:].strip() if idx >= 0 else text.strip()[-1500:]
|
||||
report = _report_of(record, text[idx:] if idx >= 0 else text)
|
||||
_file_report(record, "PR update", report)
|
||||
_session_report(record, report)
|
||||
summary = f"{name} acted on {filename}'s PR — re-review when ready"
|
||||
@@ -640,7 +642,7 @@ def _reap_pr_review(agent_id: str, proc: subprocess.Popen, log_file) -> None:
|
||||
except OSError:
|
||||
text = ""
|
||||
idx = text.find("PR REVIEW:")
|
||||
report = text[idx:].strip() if idx >= 0 else text.strip()[-1500:]
|
||||
report = _report_of(record, text[idx:] if idx >= 0 else text)
|
||||
match = re.search(r"^PR REVIEW:\s*(APPROVE|REQUEST CHANGES)", report)
|
||||
verdict = match.group(1) if match else None
|
||||
_file_report(record, "PR review", report)
|
||||
@@ -672,7 +674,7 @@ def _reap_review(agent_id: str, proc: subprocess.Popen, log_file) -> None:
|
||||
except OSError:
|
||||
text = ""
|
||||
idx = text.find("RELEVANCE REVIEW")
|
||||
report = text[idx:].strip() if idx >= 0 else text.strip()[-1500:]
|
||||
report = _report_of(record, text[idx:] if idx >= 0 else text)
|
||||
verdict = report.splitlines()[0] if report else None
|
||||
_file_report(record, "Relevance review", report)
|
||||
_session_report(record, report)
|
||||
|
||||
+10
-6
@@ -25,6 +25,7 @@ from pathlib import Path
|
||||
|
||||
import config
|
||||
import drive as drive_mod
|
||||
import reports
|
||||
import state
|
||||
from taskfiles import STATUS_RE, commit_edit, find_stage_of, move_task, read_task
|
||||
|
||||
@@ -147,9 +148,9 @@ def _open_pr(filename: str) -> str:
|
||||
|
||||
body = (f"Task: `{filename}` — tracked in `.task-manager/tasks/review/`.\n\n"
|
||||
f"Opened by the board when the card moved to review.")
|
||||
log_tail = _agent_log_tail(filename)
|
||||
if log_tail:
|
||||
body += f"\n\n## Agent summary\n\n{log_tail}"
|
||||
summary = _agent_report(filename)
|
||||
if summary:
|
||||
body += f"\n\n## Agent summary\n\n{summary}"
|
||||
result = _run([config.GH_BIN, "pr", "create", "--head", branch, "--base", "main",
|
||||
"--title", task["title"], "--body", body], timeout=120)
|
||||
if result.returncode != 0:
|
||||
@@ -190,7 +191,10 @@ def _existing_pr(branch: str) -> str:
|
||||
return next((l.strip() for l in found.stdout.splitlines() if "/pull/" in l), "")
|
||||
|
||||
|
||||
def _agent_log_tail(filename: str, cap: int = 1500) -> str:
|
||||
def _agent_report(filename: str) -> str:
|
||||
"""The work agent's closing report, clipped exactly as the task file
|
||||
clips it (reports.report — head first, one cap for both). The PR and
|
||||
the record must never tell different stories about the same run."""
|
||||
with state.LOCK:
|
||||
records = [r for r in state.AGENTS.values()
|
||||
if r["task"] == filename and r.get("mode") == "work"]
|
||||
@@ -198,10 +202,10 @@ def _agent_log_tail(filename: str, cap: int = 1500) -> str:
|
||||
return ""
|
||||
latest = max(records, key=lambda r: r["started"])
|
||||
try:
|
||||
text = Path(latest["log"]).read_text(encoding="utf-8", errors="replace").strip()
|
||||
text = Path(latest["log"]).read_text(encoding="utf-8", errors="replace")
|
||||
except OSError:
|
||||
return ""
|
||||
return text[-cap:]
|
||||
return reports.report(text, log_path=latest.get("log"))
|
||||
|
||||
|
||||
def request_copilot(filename: str) -> str:
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
"""Keeping an agent's closing report — one clip, every surface.
|
||||
|
||||
A report is the permanent record: it is appended to the task file, shown
|
||||
as the session's last entry, and carried into the PR body. When one is
|
||||
too long to keep whole, *which end survives* is the whole question. The
|
||||
report contract asks the agent to lead with the state of the work, so the
|
||||
first lines are the part a reader must not lose — a tail slice drops
|
||||
exactly the sentence that says what happened.
|
||||
|
||||
So the clip here keeps both ends and cuts the middle, on line boundaries,
|
||||
and says so in words on its own line, naming the log that still holds the
|
||||
whole thing. The failure excerpt is the one deliberate exception and does
|
||||
not come through here: for a run that died, the *end* is the story.
|
||||
|
||||
Sits left of both consumers in the module map (config → state →
|
||||
taskfiles → events / github / … → agents), so agents.py and github.py
|
||||
clip identically instead of reaching sideways for each other's helper.
|
||||
Depends on config alone, for the repo root a log path is named against.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import config
|
||||
|
||||
# What the permanent record keeps of one report, in characters. Roughly
|
||||
# 1,800 words — enough for the report the prompt contract asks for
|
||||
# (state of the work, what to do, what to know, review-first pointer)
|
||||
# with room to spare, so in practice reports arrive whole and this is
|
||||
# the backstop rather than the norm. One number, both consumers: the
|
||||
# task file and the PR body must never tell different stories about the
|
||||
# same run.
|
||||
CAP = 12000
|
||||
|
||||
# Of the budget, how much goes to the head. The headline is the contract;
|
||||
# the tail is the "review first" pointer that closes it. Two to one.
|
||||
HEAD_SHARE = 2 / 3
|
||||
|
||||
|
||||
def clean(text: str) -> str:
|
||||
"""An agent's -p output is its final report; strip the hook-failure
|
||||
noise other tools may have interleaved. No capping — that is `report`
|
||||
(head + tail) or `tail` (a dead run's ending)."""
|
||||
lines = [l for l in (text or "").strip().splitlines()
|
||||
if not ("hook" in l and "failed" in l)]
|
||||
return "\n".join(lines).strip()
|
||||
|
||||
|
||||
def tail(text: str, cap: int) -> str:
|
||||
"""The last `cap` characters, cleaned. For a crash, where the end is
|
||||
the story — see `_failure_excerpt` in agents.py."""
|
||||
return clean(text)[-cap:]
|
||||
|
||||
|
||||
def report(text: str, log_path: str | None = None, cap: int = CAP) -> str:
|
||||
"""The report as the record should keep it.
|
||||
|
||||
Shorter than the cap: returned cleaned and otherwise byte for byte.
|
||||
Longer: the leading lines and the trailing lines survive, separated by
|
||||
one line of prose saying how much was cut and where the whole report
|
||||
still lives. Cuts land on line boundaries, so no line of the record is
|
||||
half a line — the single exception is a report that is one enormous
|
||||
line, which is cut at a space rather than not shown at all.
|
||||
"""
|
||||
text = clean(text)
|
||||
if len(text) <= cap:
|
||||
return text
|
||||
|
||||
# Two passes over the elision line: the first counts what it costs at
|
||||
# its longest (every character dropped), the second states the truth.
|
||||
# Digits can only shrink, so the result never exceeds the cap. The 4
|
||||
# is the blank line either side of it.
|
||||
budget = cap - len(_elision(len(text), cap, log_path)) - 4
|
||||
if budget <= 0:
|
||||
# A cap too small to hold even the notice: keep the head, say
|
||||
# nothing else. Nothing in bench configures one this small.
|
||||
return _split_head(text, cap)[0]
|
||||
|
||||
head, rest = _split_head(text, int(budget * HEAD_SHARE))
|
||||
end = _take_tail(rest, budget - len(head)).lstrip()
|
||||
head = head.rstrip()
|
||||
dropped = len(text) - len(head) - len(end)
|
||||
kept = [head, _elision(dropped, cap, log_path)]
|
||||
if end:
|
||||
kept.append(end)
|
||||
return "\n\n".join(kept)
|
||||
|
||||
|
||||
def _elision(dropped: int, cap: int, log_path: str | None) -> str:
|
||||
"""The line that stands where the middle was. A reader must never have
|
||||
to infer that something was removed, nor go looking for the rest."""
|
||||
return (f"… {dropped} characters of this report were cut here to keep the "
|
||||
f"record within {cap} characters. The whole report is in "
|
||||
f"{_log_reference(log_path)}.")
|
||||
|
||||
|
||||
def _log_reference(log_path: str | None) -> str:
|
||||
"""Where the unclipped report still is, repo-relative when it can be."""
|
||||
logs = "manager/local/state/agent/logs/"
|
||||
if not log_path:
|
||||
return f"this run's log under `{logs}`"
|
||||
path = Path(log_path)
|
||||
try:
|
||||
return f"`{path.resolve().relative_to(config.REPO)}`"
|
||||
except (ValueError, OSError):
|
||||
return f"`{logs}{path.name}`"
|
||||
|
||||
|
||||
def _split_head(text: str, budget: int) -> tuple[str, str]:
|
||||
"""Leading whole lines fitting the budget, and everything after them.
|
||||
|
||||
A first line longer than the entire budget is the one case a line gets
|
||||
cut: at its last space, so the record ends on a word rather than on
|
||||
`four" — they are`. A line with no space in it at all (one long token)
|
||||
is cut where the budget runs out; there is no better place.
|
||||
"""
|
||||
kept, used = [], 0
|
||||
for line in text.split("\n"):
|
||||
cost = len(line) + (1 if kept else 0)
|
||||
if used + cost > budget:
|
||||
break
|
||||
kept.append(line)
|
||||
used += cost
|
||||
if kept:
|
||||
return "\n".join(kept), text[used:].lstrip("\n")
|
||||
piece = _cut_at_space(text[:budget])
|
||||
return piece, text[len(piece):].lstrip("\n")
|
||||
|
||||
|
||||
def _take_tail(text: str, budget: int) -> str:
|
||||
"""Trailing whole lines fitting the budget — the mirror of the head,
|
||||
including how it treats one line too long to keep whole."""
|
||||
kept, used = [], 0
|
||||
for line in reversed(text.split("\n")):
|
||||
cost = len(line) + (1 if kept else 0)
|
||||
if used + cost > budget:
|
||||
break
|
||||
kept.insert(0, line)
|
||||
used += cost
|
||||
if kept:
|
||||
return "\n".join(kept)
|
||||
return _cut_at_space(text[-budget:], from_start=True)
|
||||
|
||||
|
||||
def _cut_at_space(piece: str, from_start: bool = False) -> str:
|
||||
"""Trim a fragment back to a word boundary, if one is near enough to
|
||||
the edge to be worth losing. Half the fragment is the limit: past that
|
||||
the cut is doing more harm than the broken word it avoids."""
|
||||
if not piece:
|
||||
return ""
|
||||
if from_start:
|
||||
space = piece.find(" ")
|
||||
if 0 <= space < len(piece) // 2:
|
||||
piece = piece[space + 1:]
|
||||
else:
|
||||
space = piece.rfind(" ")
|
||||
if space > len(piece) // 2:
|
||||
piece = piece[:space]
|
||||
return piece.strip()
|
||||
Vendored
+22
@@ -0,0 +1,22 @@
|
||||
Work is committed on `task/32-serve-bench-12vectors-com-from-a-worker` in two commits and the suite is green, but **nothing has been deployed** — this headless run had no Cloudflare credentials and no network for `npx`, so `wrangler deploy`, `wrangler dev` and every live-response check are still outstanding.
|
||||
|
||||
## What to do
|
||||
|
||||
1. **Deploy it**: `cd site && npx wrangler deploy` from a shell logged in to Cloudflare, then confirm that `https://bench.12vectors.com/` answers with the built landing page now.
|
||||
2. **Tick or reject the acceptance boxes I could not reach** — the ones I marked "one, two and four" — they are acceptance criteria 1, 2 and 4, and they are the ones no test in this repo can reach.
|
||||
3. **Replace the account line** in `site/README.md`'s "Where the site lives" table with what `wrangler whoami` prints. I described the account by its defining property (the one holding the `12vectors.com` zone) rather than inventing a name or ID.
|
||||
4. **Open the follow-up card** for a GitHub Action on merge, if you still want one.
|
||||
|
||||
## What to know
|
||||
|
||||
The card's open question carried the author's own recommendation — manual deploy for v1 — so I built that rather than sending the card back: `site/wrangler.jsonc` and a documented command sequence, no workflow file, no Cloudflare token in repository secrets.
|
||||
|
||||
Serving the site needed three things from the builder first, which is the first commit (`ad639a3`):
|
||||
|
||||
- **A real 404.** Cloudflare's `not_found_handling: "404-page"` wants a literal `404.html` at the root of the assets directory, so a manifest route may now name an `.html` file instead of ending in `/`. `/404.html` is a normal entry with a new `notfound` layout — the site's design and nav, a link back to the landing page, `noindex`, no canonical, and a `null` section that keeps it off the nav it renders.
|
||||
- **`site/root/`**, copied verbatim to the top of the build the way `static/` is copied into a subdirectory. It holds `_headers`, which the host reads from the root and nowhere else. A file in `root/` that a route would also write stops the build.
|
||||
- **Fingerprinted asset urls.** The templates link the stylesheet and icon through `$stylesheet`/`$icon`, carrying `?v=<hash>` of the file's contents. Nothing in `static/` is renamed, so the tree and the `/static/*` glob are unchanged — but the url moves when the file does, which is what makes the year-long `immutable` cache safe rather than a way to ship an invisible deploy.
|
||||
|
||||
Two judgement calls worth a look. I added a `default-src 'none'` **Content-Security-Policy** to `site/root/_headers` beyond the three headers the card named: it turns "no analytics, no third-party anything" into something the browser enforces, and a test asserts no built page contains an inline `<script>` or `style=` that the policy would block. And I could not reach the Cloudflare docs from this run, so two behaviours are asserted from memory: that Workers static assets reads `_headers` at all, and that a later rule wins on a header it repeats. Both are written to fail safe — if either is wrong, HTML still revalidates and the static assets merely revalidate more often than needed.
|
||||
|
||||
**Review first:** `site/wrangler.jsonc` and `site/root/_headers` — everything the live site does is decided in those two files. Then `site/build.py`'s `target_for`, `stamp` and `root_files`, which are the builder's side of them. `tests/test_site_deploy.py` covers the config, the built artefacts, and the fact that `wrangler.jsonc`, `pages.json` and `README.md` cannot drift apart about which domain this is; its docstring says plainly what it does not cover.
|
||||
@@ -0,0 +1,276 @@
|
||||
"""What the record keeps of a long closing report (task 42).
|
||||
|
||||
The cap used to keep the *last* 3,000 characters, so a report longer than
|
||||
that lost exactly the sentence the prompt contract puts first — the one
|
||||
saying what happened — and the task file began mid-word. The clip now
|
||||
keeps the head, keeps the tail, cuts the middle on line boundaries, and
|
||||
says so in words while naming the log that still holds the whole thing.
|
||||
|
||||
The regression fixture is card 32's report, `tests/fixtures/
|
||||
32-work-report.log` (3,619 bytes, the size the live one was). Its run's
|
||||
log is gitignored state and did not survive into this worktree, so the
|
||||
fixture is that report reassembled: the 3,000 characters the old clip
|
||||
kept are verbatim from `tasks/done/32-serve-bench-12vectors-com-from-a-
|
||||
worker.md`, and the 619 the old clip discarded are rebuilt from the
|
||||
quotation in card 42, which is where they were preserved. Its first
|
||||
sentence and its first two action items are the bytes this card exists
|
||||
to stop losing.
|
||||
|
||||
python3 -m unittest discover -s tests -v
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import sys
|
||||
import tempfile
|
||||
import time
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
sys.path.insert(0, str(REPO / "manager" / "core"))
|
||||
|
||||
import agents # noqa: E402
|
||||
import github # noqa: E402
|
||||
import reports # noqa: E402
|
||||
import state # noqa: E402
|
||||
|
||||
FIXTURE = Path(__file__).resolve().parent / "fixtures" / "32-work-report.log"
|
||||
OLD_CAP = 3000 # what the bug shipped with, and what card 32 met
|
||||
|
||||
# A report that is unmistakably longer than any cap under test, with a
|
||||
# contract-shaped head and tail: the headline first, the pointer last.
|
||||
HEADLINE = "Work is committed on `task/x` and the suite is green, but nothing is deployed."
|
||||
CLOSER = "**Review first:** `manager/core/reports.py`, then its tests."
|
||||
|
||||
|
||||
def long_report(paragraphs: int = 400) -> str:
|
||||
middle = "\n\n".join(f"Paragraph {i} of the narrative, which is the part "
|
||||
f"a clip may drop without costing the reader a decision."
|
||||
for i in range(paragraphs))
|
||||
return f"{HEADLINE}\n\n{middle}\n\n{CLOSER}"
|
||||
|
||||
|
||||
class ShortReportsAreUntouched(unittest.TestCase):
|
||||
def test_passed_through_byte_for_byte(self):
|
||||
text = "ADDRESSED: fixed the two comments.\n\nBoth were naming.\n"
|
||||
self.assertEqual(reports.report(text), text.strip())
|
||||
|
||||
def test_no_elision_line(self):
|
||||
self.assertNotIn("were cut here", reports.report("A one-line report."))
|
||||
|
||||
def test_exactly_the_cap_is_whole(self):
|
||||
text = "x" * 40 + "\n" + "y" * 59 # 100 characters
|
||||
self.assertEqual(reports.report(text, cap=100), text)
|
||||
|
||||
def test_hook_noise_still_stripped(self):
|
||||
text = "PostToolUse hook failed with status 1\nThe actual report.\n"
|
||||
self.assertEqual(reports.report(text), "The actual report.")
|
||||
|
||||
|
||||
class ClippedReportsKeepTheirHead(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.clipped = reports.report(long_report(), log_path="/tmp/logs/09-x-1203.log",
|
||||
cap=OLD_CAP)
|
||||
|
||||
def test_begins_with_the_reports_own_first_line(self):
|
||||
self.assertEqual(self.clipped.splitlines()[0], HEADLINE)
|
||||
|
||||
def test_keeps_the_tail_too(self):
|
||||
self.assertTrue(self.clipped.rstrip().endswith(CLOSER), self.clipped[-200:])
|
||||
|
||||
def test_says_in_words_that_it_was_clipped(self):
|
||||
self.assertIn("were cut here", self.clipped)
|
||||
self.assertRegex(self.clipped, r"… \d+ characters of this report were cut here")
|
||||
|
||||
def test_names_the_log_holding_the_whole_thing(self):
|
||||
self.assertIn("09-x-1203.log", self.clipped)
|
||||
|
||||
def test_the_elision_is_on_its_own_line(self):
|
||||
line = next(l for l in self.clipped.splitlines() if "were cut here" in l)
|
||||
self.assertTrue(line.startswith("… "), line)
|
||||
self.assertTrue(line.endswith("."), line)
|
||||
|
||||
def test_stays_within_the_cap(self):
|
||||
self.assertLessEqual(len(self.clipped), OLD_CAP)
|
||||
|
||||
def test_cuts_only_on_line_boundaries(self):
|
||||
source = set(long_report().splitlines())
|
||||
for line in self.clipped.splitlines():
|
||||
if "were cut here" in line:
|
||||
continue
|
||||
self.assertIn(line, source, f"line was cut mid-line: {line!r}")
|
||||
|
||||
def test_nothing_is_kept_twice(self):
|
||||
for line in self.clipped.splitlines():
|
||||
if line.strip() and "were cut here" not in line:
|
||||
self.assertEqual(self.clipped.count(line), 1,
|
||||
f"duplicated into the record: {line!r}")
|
||||
|
||||
def test_without_a_log_path_it_still_points_at_the_log_directory(self):
|
||||
clipped = reports.report(long_report(), cap=OLD_CAP)
|
||||
self.assertIn("manager/local/state/agent/logs/", clipped)
|
||||
|
||||
|
||||
class Card32Regression(unittest.TestCase):
|
||||
"""The live case: 3,619 bytes met a 3,000-character cap and the record
|
||||
lost the sentence saying nothing had been deployed."""
|
||||
|
||||
def setUp(self):
|
||||
self.text = FIXTURE.read_text(encoding="utf-8")
|
||||
|
||||
def test_the_fixture_is_the_size_the_report_was(self):
|
||||
self.assertEqual(len(self.text.encode("utf-8")), 3619)
|
||||
|
||||
def test_it_opens_with_the_state_of_the_work(self):
|
||||
clipped = reports.report(self.text, log_path="/tmp/logs/32-…-113412.log",
|
||||
cap=OLD_CAP)
|
||||
self.assertTrue(clipped.startswith("Work is committed on"), clipped[:120])
|
||||
self.assertIn("nothing has been deployed", clipped)
|
||||
|
||||
def test_it_retains_action_items_one_and_two(self):
|
||||
clipped = reports.report(self.text, cap=OLD_CAP)
|
||||
self.assertIn("**Deploy it**", clipped)
|
||||
self.assertIn("acceptance boxes I could not reach", clipped)
|
||||
|
||||
def test_it_keeps_the_review_first_pointer(self):
|
||||
clipped = reports.report(self.text, cap=OLD_CAP)
|
||||
self.assertIn("**Review first:**", clipped)
|
||||
|
||||
def test_the_old_clip_is_what_lost_it(self):
|
||||
"""The bug, stated as a test: the tail slice this replaces drops
|
||||
the headline and starts mid-word."""
|
||||
old = self.text.strip()[-OLD_CAP:]
|
||||
self.assertNotIn("nothing has been deployed", old)
|
||||
self.assertTrue(old.startswith('four"'), old[:40])
|
||||
|
||||
def test_the_shipped_cap_keeps_this_report_whole(self):
|
||||
self.assertEqual(reports.report(self.text), self.text.strip())
|
||||
|
||||
|
||||
class MarkersSurviveTheClip(unittest.TestCase):
|
||||
"""The board parses these out of the report it kept, so keeping the
|
||||
head is what keeps them — all three sit on the first line."""
|
||||
|
||||
def clipped(self, marker: str) -> str:
|
||||
return reports.report(f"{marker}\n\n{long_report()}", cap=OLD_CAP)
|
||||
|
||||
def test_not_ready(self):
|
||||
clipped = self.clipped("NOT READY: the deploy target is still undecided")
|
||||
match = re.search(r"^NOT READY:\s*(.*)$", clipped, re.MULTILINE)
|
||||
self.assertEqual(match.group(1), "the deploy target is still undecided")
|
||||
|
||||
def test_pr_review(self):
|
||||
clipped = self.clipped("PR REVIEW: REQUEST CHANGES")
|
||||
self.assertRegex(clipped, r"^PR REVIEW:\s*(APPROVE|REQUEST CHANGES)")
|
||||
|
||||
def test_addressed(self):
|
||||
self.assertTrue(self.clipped("ADDRESSED: renamed the helper")
|
||||
.startswith("ADDRESSED: renamed the helper"))
|
||||
|
||||
def test_relevance_verdict_is_still_the_first_line(self):
|
||||
clipped = self.clipped("RELEVANCE REVIEW: Still relevant")
|
||||
self.assertEqual(clipped.splitlines()[0], "RELEVANCE REVIEW: Still relevant")
|
||||
|
||||
|
||||
class OneEnormousLine(unittest.TestCase):
|
||||
"""A run whose whole output is a single line has no boundary to cut
|
||||
on. It still has to say something — and still must not end mid-word."""
|
||||
|
||||
def setUp(self):
|
||||
self.words = " ".join(f"word{i}" for i in range(4000))
|
||||
self.clipped = reports.report(self.words, cap=OLD_CAP)
|
||||
|
||||
def test_produces_something_rather_than_nothing(self):
|
||||
self.assertTrue(self.clipped.startswith("word0 word1 "))
|
||||
self.assertLessEqual(len(self.clipped), OLD_CAP)
|
||||
|
||||
def test_the_elision_still_explains_itself(self):
|
||||
self.assertIn("were cut here", self.clipped)
|
||||
|
||||
def test_both_ends_end_on_a_whole_word(self):
|
||||
head, end = self.clipped.split("\n\n")[0], self.clipped.split("\n\n")[-1]
|
||||
self.assertIn(f" {head.split()[-1]} ", self.words)
|
||||
self.assertIn(f" {end.split()[0]} ", self.words)
|
||||
self.assertTrue(self.words.endswith(end))
|
||||
|
||||
def test_a_single_token_with_no_spaces_is_still_reported(self):
|
||||
blob = "z" * 9000
|
||||
clipped = reports.report(blob, cap=500)
|
||||
self.assertTrue(clipped.startswith("zzz"))
|
||||
self.assertIn("were cut here", clipped)
|
||||
self.assertLessEqual(len(clipped), 500)
|
||||
|
||||
|
||||
class TheWindowsMeet(unittest.TestCase):
|
||||
"""Where an off-by-one would duplicate a paragraph into the permanent
|
||||
record: one character over the cap, and caps small enough that the
|
||||
head, the notice and the tail have to share almost nothing."""
|
||||
|
||||
def test_one_character_over_the_cap(self):
|
||||
lines = [f"line {i} of the report" for i in range(60)]
|
||||
text = "\n".join(lines)
|
||||
clipped = reports.report(text, cap=len(text) - 1)
|
||||
kept = [l for l in clipped.splitlines() if l.strip() and "were cut here" not in l]
|
||||
self.assertEqual(len(kept), len(set(kept)), "a line landed twice")
|
||||
self.assertTrue(set(kept) <= set(lines), "a line was cut mid-line")
|
||||
self.assertIn("were cut here", clipped)
|
||||
|
||||
def test_a_range_of_caps_never_duplicates_or_overflows(self):
|
||||
text = "\n".join(f"line {i} of the report" for i in range(200))
|
||||
for cap in range(200, 1200, 37):
|
||||
clipped = reports.report(text, cap=cap)
|
||||
self.assertLessEqual(len(clipped), cap, f"cap={cap}")
|
||||
body = [l for l in clipped.splitlines()
|
||||
if l.strip() and "were cut here" not in l]
|
||||
self.assertEqual(len(body), len(set(body)), f"cap={cap} duplicated a line")
|
||||
|
||||
def test_a_cap_too_small_for_the_notice_still_returns_the_head(self):
|
||||
clipped = reports.report(long_report(), cap=40)
|
||||
self.assertTrue(HEADLINE.startswith(clipped.split("\n")[0][:20]))
|
||||
self.assertLessEqual(len(clipped), 40)
|
||||
|
||||
|
||||
class FailedRunsStillKeepTheirTail(unittest.TestCase):
|
||||
"""The one deliberate exception: for a run that died, the end is the
|
||||
story. This card does not invert that."""
|
||||
|
||||
def test_excerpt_is_the_end_of_the_log(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
log = Path(tmp) / "run.log"
|
||||
log.write_text("\n".join([f"chatter {i}" for i in range(500)]
|
||||
+ ["API Error: 500 {\"type\":\"error\"}"]),
|
||||
encoding="utf-8")
|
||||
excerpt = agents._failure_excerpt(str(log))
|
||||
self.assertTrue(excerpt.endswith("API Error: 500 {\"type\":\"error\"}"))
|
||||
self.assertNotIn("chatter 0\n", excerpt)
|
||||
self.assertNotIn("were cut here", excerpt)
|
||||
|
||||
|
||||
class OneClipForBothConsumers(unittest.TestCase):
|
||||
"""The PR body and the task file must never tell different stories
|
||||
about the same run — same helper, same cap, same text."""
|
||||
|
||||
def test_pr_body_matches_the_task_file(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
log = Path(tmp) / "42-x-1203.log"
|
||||
log.write_text(long_report(), encoding="utf-8")
|
||||
record = {"id": "work-42-x-1203", "task": "42-x.md", "mode": "work",
|
||||
"started": time.time(), "log": str(log)}
|
||||
with state.LOCK:
|
||||
state.AGENTS[record["id"]] = record
|
||||
try:
|
||||
self.assertEqual(github._agent_report("42-x.md"),
|
||||
agents._report_of(record))
|
||||
finally:
|
||||
with state.LOCK:
|
||||
state.AGENTS.pop(record["id"], None)
|
||||
|
||||
def test_one_documented_cap(self):
|
||||
self.assertIsInstance(reports.CAP, int)
|
||||
self.assertGreater(reports.CAP, OLD_CAP)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user