Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/quality-gate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ jobs:
scripts.tests.test_bump_version \
scripts.tests.test_lint_embedded \
scripts.tests.test_bench_collect \
scripts.tests.test_capture_hook
scripts.tests.test_capture_hook \
scripts.tests.test_derive_prompt_shapes
coverage report

test:
Expand Down Expand Up @@ -209,7 +210,8 @@ jobs:
scripts.tests.test_bump_version \
scripts.tests.test_lint_embedded \
scripts.tests.test_bench_collect \
scripts.tests.test_capture_hook
scripts.tests.test_capture_hook \
scripts.tests.test_derive_prompt_shapes
coverage report

# The threshold lives in pyproject.toml ([tool.coverage.report]
Expand Down
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ coverage run -m unittest discover -s cargento/skills/cargento/tests -t .
coverage run -a -m unittest \
scripts.tests.test_validate_plugins scripts.tests.test_bump_version \
scripts.tests.test_lint_embedded scripts.tests.test_bench_collect \
scripts.tests.test_capture_hook scripts.tests.test_bench_event_latency
scripts.tests.test_capture_hook scripts.tests.test_bench_event_latency \
scripts.tests.test_derive_prompt_shapes
coverage report # enforces the fail_under threshold from pyproject.toml
# Native validators, if the CLIs are installed (they are not available on stock runners):
claude plugin validate ./cargento --strict
Expand Down
15 changes: 15 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ and how long the hook itself took. It never records a prompt, a tool argument, a
path. A research tool that captured those would be a worse leak than the thing it is researching,
because it writes to disk and accumulates.

`scripts/derive_prompt_shapes.py` re-derives the counts written into the harness-injected-prompt
comment block in `cargento_runtime/records.py`: which markup tag leads a harness's own machinery,
how often, and which of those a turn scanner already refuses. Point it at your own store with
`--claude-root` and `--codex-root`, or run it bare for the defaults. Every count in that comment
block is one of its outputs, so a reviewer can check the numbers instead of trusting them, and a new
harness build that changes a tag shows up as a name the vocabulary does not list.

It obeys the same rule `capture_hook.py` does, and for a sharper reason: a derivation script reads
every prompt anyone ever typed, credentials included. It prints counts and shape names only, and the
guard is a whitelist rather than a filter, so a label that is not a short markup name, a literal
already in the source, or one of a fixed set of column headings raises instead of printing. Its test
seeds a distinctive prompt into a fixture store and asserts the whole output carries none of it.
Discovering a new prose prefix is out of scope for the same reason: it cannot be done without
printing prose.

### Tests

Every behavior change to `server.py` or `cargento_runtime/` needs a regression test in
Expand Down
133 changes: 116 additions & 17 deletions cargento/skills/cargento/cargento_runtime/observer.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,36 @@
# of them hit the 200-character cap with the triggering phrase truncated away,
# so the rendered card showed a block whose visible text contained no block
# language at all. The first-person forms above already carry the real case.
#
# `not permitted`, `permission denied` and `waiting for your` are the remainder,
# and they go the same way. Re-measured over the whole local Claude corpus (3,774
# transcripts, 2,828 with an assistant message) the table produced 7 blocks and
# those three supplied 4 of them — every one inside a quoted or fenced span, two
# of them again truncated away by the 200-character cap. They are replaced below
# by the self-state forms that carry the real case; those forms match 0 records
# today, which is the point. An indicator that never fires costs nothing, and
# these three cost a wrong answer each.
_BLOCK_INDICATORS = (
"i'm blocked",
"i am blocked",
"i'm stuck",
"i am stuck",
"waiting for you",
"waiting for your",
"waiting for approval",
"not permitted",
"permission denied",
"i'm waiting for your",
"i am waiting for your",
"i'm not permitted",
"i am not permitted",
"i don't have permission",
"i do not have permission",
)

# What may not follow an indicator. `waiting for you` is a prefix of `waiting for
# your`, so without this the bare phrase would keep matching the possessive the
# line above removes — and the two are not the same claim: "waiting for you." is
# a hand-off, "waiting for your PR to land" is a report about someone else.
_BLOCK_TRAILING_RE = re.compile(r"[A-Za-z0-9]")


class ModelCaller(Protocol):
"""A cheap model invocation that derives a goal line, or None on failure.
Expand All @@ -100,7 +118,14 @@ def __call__(self, recent_text: str, entity_stage: str) -> str | None: ...


def _is_generic_opener(text: str) -> bool:
"""Whether a user message is a generic skill-load directive, not a goal."""
"""Whether a user message is a generic skill-load directive, not a goal.

One concept with ``records.injected_prompt`` and two disjoint lists: both ask
"is this the harness talking". They are kept apart because this is a *goal*
rule and that is a *record* rule — a generic opener is a real user message
that states nothing, so no other reader should be made to drop it — but a
phrase that belongs on both has to be added to both.
"""
stripped = text.strip().lower()
return any(stripped.startswith(prefix) for prefix in _GENERIC_OPENER_PREFIXES)

Expand All @@ -111,6 +136,11 @@ def _is_generic_opener(text: str) -> bool:
# shape with no single name to hand the injected-tag lookup, which is exactly
# the case `records.injected_prompt` documents: a harness it has no measured
# vocabulary for gets the union of every measured set.
#
# So this value is deliberately NOT a key of `records._INJECTED_TAGS`, and the
# dict miss is the mechanism rather than an accident. The Droid half of the name
# is aspirational: `resolve_transcript` answers for claude, codex and pi only, so
# no Droid transcript reaches this module today.
_SHARED_MESSAGE_HARNESS = "pi-or-droid"


Expand Down Expand Up @@ -222,6 +252,16 @@ def _dedup_key(record: dict[str, Any]) -> str:
degraded silently to the message text and a prompt repeated verbatim later
in the session kept its first, oldest position. Claude spells it ``uuid``
and Codex ``payload.id``; Pi and Droid do spell it ``id``.

Empty is a real answer and not a failure: ``payload.id`` is absent on 500 of
the 655 Codex user-message records (76.3%) the DISJOINT windows this module
now cuts return, measured over the whole local rollout store. The count
matters less than the instrument: the same measurement on the old
``head + read_tail`` concatenation gives 599 of 785, and the surplus is the
overlap region counted twice, which is the bug ``_window_lines`` fixes. The
share is 76.3% either way, so the load-bearing claim (about three quarters
carry no id) does not rest on which instrument you use. The caller's fallback
is positional for that reason — see ``_window_lines``.
"""
for value in (
record.get("uuid"),
Expand All @@ -233,12 +273,54 @@ def _dedup_key(record: dict[str, Any]) -> str:
return ""


def _window_lines(config: RuntimeConfig, path: str) -> list[str]:
"""The head window's lines then the tail window's, with no line in both.

Cut apart on byte offsets rather than deduped afterwards, because the two
windows overlap on any file under ``observer_head_bytes + tail_bytes`` —
completely, on any file the tail read swallows whole. The old concatenation
leaned on the dedup key to collapse that overlap, and the key falls back to
the message TEXT on the 76.4% of Codex user records carrying no
``payload.id``: a prompt repeated verbatim later in the session was then
dropped as a duplicate of its own first occurrence, and the goal stayed on
whatever came between them. Disjoint windows make that fallback positional,
so the only thing the key still has to collapse is a genuine replay — a
resumed transcript rewriting earlier records, which carry their original ids.
"""
tail_lines = runtime_io.read_tail(config, path)
try:
size = os.path.getsize(path)
except OSError:
return tail_lines
if size <= config.tail_bytes:
# The tail read took the whole file, so the head window is a prefix of
# what is already here.
return tail_lines
try:
head = runtime_io.read_prefix_bytes(path, max_bytes=config.observer_head_bytes)
except OSError:
return tail_lines
# The first byte the tail read covers. A head line starting at or after it is
# in `tail_lines` already; one starting before it cannot be, because the tail
# read drops its own partial first line.
floor = size - config.tail_bytes
head_lines: list[str] = []
offset = 0
for raw in head.split(b"\n"):
if offset >= floor:
break
head_lines.append(raw.decode("utf-8", "replace"))
offset += len(raw) + 1
return head_lines + tail_lines


def _extract_messages(config: RuntimeConfig, path: str) -> list[dict[str, str]]:
"""User and assistant texts from a JSONL transcript, head + tail bounded.

The head carries the opening directive; the tail carries the recent window.
Records are deduped by their own id so the overlap region between head and
tail does not double-count, and returned in **record-timestamp** order.
``_window_lines`` keeps the two disjoint, and records are deduped by their
own id so a resumed transcript's replayed block does not double-count.
Returned in **record-timestamp** order.

Ordering by timestamp and not by list position, because list position is
not record order. The concatenation itself is fine — the head is read
Expand All @@ -256,18 +338,12 @@ def _extract_messages(config: RuntimeConfig, path: str) -> list[dict[str, str]]:
fires. File order breaks ties, which is what keeps a transcript whose
records carry no stamp reading exactly as it did before.
"""
try:
head = runtime_io.read_prefix_bytes(path, max_bytes=config.observer_head_bytes)
except OSError:
head = b""
head_lines = head.decode("utf-8", "replace").split("\n")
tail_lines = runtime_io.read_tail(config, path)
ordered: list[tuple[float, int, dict[str, str]]] = []
seen: set[str] = set()
# Carried forward so a stampless record sorts beside the stamped one before
# it rather than ahead of the whole file.
last_ts = 0.0
for position, raw in enumerate(head_lines + tail_lines):
for position, raw in enumerate(_window_lines(config, path)):
if not raw or not raw.lstrip().startswith("{"):
continue
try:
Expand All @@ -282,7 +358,7 @@ def _extract_messages(config: RuntimeConfig, path: str) -> list[dict[str, str]]:
parsed = _parse_message_record(record, config.observer_model_context_chars)
if parsed is None:
continue
key = _dedup_key(record) or parsed["text"]
key = _dedup_key(record) or f"#{position}"
if key in seen:
continue
seen.add(key)
Expand Down Expand Up @@ -344,11 +420,19 @@ def _derive_goal_deterministic(
# it reads as `<command-message>…`, which was 60 of 400 Claude sessions and
# 5 of 457 Codex rollouts. `prompt_title`
# already owns that rendering (`/review 1287 — with fresh eyes`), and
# strips the wrapper tags off everything else.
# strips the wrapper tags off everything else. It also collapses a long
# absolute path to its basename (`transcripts.shorten_paths`), so a goal
# naming a temp file reads as the file rather than as the path to it.
goal = transcripts.prompt_title(config, directives[-1], limit=config.observer_goal_cap_chars)
if not goal:
return NO_GOAL, None
return records.safe_text(goal, config.observer_goal_cap_chars), None
# Cap plus one, for the reason `records.instruction_line` carries the same
# `+ 1`: `transcripts.clip` appends its ellipsis AFTER cutting to the cap, so
# a clipped goal is cap + 1 characters and a scrub at the cap took the `…`
# straight back off — an unmarked mid-token cut on 8 of the 1,295 goals the
# local Claude corpus publishes (269 of which clip at all). `safe_text` only
# ever shortens, so this cannot lengthen what rendering already bounded.
return records.safe_text(goal, config.observer_goal_cap_chars + 1), None


def _derive_stage(
Expand Down Expand Up @@ -390,6 +474,21 @@ def _derive_stage(
return entities[0][1] if entities else ""


def _indicator_hit(lower: str, indicator: str) -> int:
"""Where one indicator matches as a whole phrase in lowercased text, or -1.

Scans past a rejected hit rather than stopping at it: `str.find` returns the
first occurrence, and "waiting for your PR" earlier in a message must not
hide a genuine "waiting for you." later in it.
"""
start = 0
while (pos := lower.find(indicator, start)) >= 0:
if not _BLOCK_TRAILING_RE.match(lower, pos + len(indicator)):
return pos
start = pos + 1
return -1


def _derive_block(
config: RuntimeConfig,
messages: list[dict[str, str]],
Expand All @@ -411,7 +510,7 @@ def _derive_block(
text = msg["text"]
lower = text.lower()
for indicator in _BLOCK_INDICATORS:
pos = lower.find(indicator)
pos = _indicator_hit(lower, indicator)
if pos < 0:
continue
# Extract the sentence around the indicator.
Expand Down
Loading