Repository navigation
feat(repo): Codex and Copilot CLI harness profiles, native review mode #116
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| ### Added | ||
|
|
||
| - **Codex CLI and Copilot CLI harness profiles for `repo run` and `repo reach`** — `--harness codex`, `codex-review`, `copilot` and `copilot-review` run oneDAL-style repository tasks under `codex exec`, Codex's native review mode (`codex exec review --commit HEAD`), `copilot -p` and `copilot -p` with a `/review`-prefixed prompt, so guidance arms can be compared per harness, not only under Claude Code. Their JSON traces convert to `events.jsonl` (`trace: codex-json`, `copilot-json`). A profile can now run the harness's own prompt (`native_prompt`) and convert its answer before grading (`answer_format: codex-review` appends a findings block parsed from Codex's review text); `repo reach` refuses a `native_prompt` profile. `bin_companions` mounts the helper files an executable needs beside it (Codex's `codex-code-mode-host`). | ||
|
|
||
| ### Fixed | ||
|
|
||
| - **`repo reach` writes the profile's `config_files`** — only `repo run` wrote them, so a profile whose endpoint or hooks live in a config file (a Codex `config.toml` naming a provider) probed a different setup than it ran. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| # Copyright (C) 2026 Intel Corporation | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Final-answer conversions a harness profile names in ``answer_format`` (contract §6). | ||
|
|
||
| A native review mode answers in its own format, not the ```` ```json ```` block | ||
| a review task asks for. The conversion keeps the harness's text and appends | ||
| that block, so a grader reading the last ```` ```json ```` block of the answer | ||
| reads the native findings; nothing is dropped from what the agent said. | ||
|
|
||
| Findings are ``{file, line, end_line, severity, priority, rule, message}``: | ||
| ``file`` relative to the workspace, ``line`` the first line of the range, | ||
| ``severity`` ``blocker`` (P0), ``major`` (P1) or ``minor`` (P2, P3) and ``None`` for a | ||
| priority this module does not know, ``rule`` the finding's title and ``message`` its body. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import logging | ||
| import re | ||
| from collections.abc import Callable, Sequence | ||
| from typing import Any | ||
|
|
||
| from .transcript import relative_path | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
| ANSWER_FORMATS = ("codex-review",) | ||
| _SEVERITY = {"P0": "blocker", "P1": "major", "P2": "minor", "P3": "minor"} | ||
|
antonwolfy marked this conversation as resolved.
|
||
| # a header may be indented (a finding in a list); the title may hold " — ": the path part is what follows | ||
| # the last one, holds none, and stays on the header's line | ||
| _CODEX_FINDING = re.compile(r"^[ \t]*- \[(P\d)\] (.+) — ([^\s—][^—\r\n]*?):(\d+)(?:-(\d+))?[ \t\r]*$", re.M) | ||
| # a marker is a [Pn] header in any shape outside a parsed finding: a list item at any indent (bold or | ||
| # not, with a path or not) or a line starting with [Pn]; every marker is parsed or no block | ||
| _BULLET_MARK = re.compile(r"^[ \t]*[-*][ \t]*(?:\*\*)?\[P\d\]", re.M | re.I) | ||
| _FINDING_MARK = re.compile(_BULLET_MARK.pattern + r"|^(?:\*\*)?\[P\d\]", re.M | re.I) | ||
|
|
||
|
|
||
| def codex_review_findings(text: str, workdir: str | None = None) -> list[dict[str, Any]]: | ||
| """Findings of Codex's native review text: ``- [P1] title — /abs/path:12-14`` then an indented body.""" | ||
| out = [] | ||
| for m, body_lines, _ in _finding_spans(text): | ||
| first = int(m.group(4)) | ||
| if m.group(1) not in _SEVERITY: | ||
| log.warning("unknown priority %s in a review finding; its severity is left unset", m.group(1)) | ||
| out.append({"file": relative_path(m.group(3), workdir), "line": first, | ||
| "end_line": int(m.group(5)) if m.group(5) else first, | ||
| "severity": _SEVERITY.get(m.group(1)), "priority": m.group(1), | ||
| "rule": m.group(2).strip(), "message": " ".join(body_lines)}) | ||
| return out | ||
|
|
||
|
|
||
| def _finding_spans(text: str) -> list[tuple[re.Match[str], list[str], int]]: | ||
| """Each parsed header, its body lines, and where its body ends in ``text``.""" | ||
| found = list(_CODEX_FINDING.finditer(text)) | ||
| out = [] | ||
| for i, m in enumerate(found): | ||
| end = found[i + 1].start() if i + 1 < len(found) else len(text) | ||
| body_lines: list[str] = [] | ||
| pos = m.end() | ||
| rest = text[m.end():end] | ||
| lines = rest.splitlines(keepends=True) | ||
| pos += len(lines[0]) if lines else 0 # [0] is the rest of the header line | ||
| for line in lines[1:]: | ||
| if line.strip(): # a blank line may separate paragraphs of the body | ||
| if not line[0].isspace(): # the body is indented; a flush line is prose after the findings | ||
| break | ||
| body_lines.append(line.strip()) | ||
|
Contributor
There was a problem hiding this comment. Nit: an indented, drifted The body loop takes every indented line, and This is a deeper case than the first thread's: the same input gave one finding at e60c7d0, so it is not a regression. Suggestion: in |
||
| pos += len(line) | ||
| out.append((m, body_lines, pos)) | ||
| return out | ||
|
|
||
|
|
||
| def _unparsed_marks(text: str, rx: re.Pattern[str] = _FINDING_MARK) -> int: | ||
| """Finding markers outside the parsed findings and their bodies (a ``[Pn]`` bullet in a body is prose).""" | ||
| rest, last = [], 0 | ||
| for m, _, end in _finding_spans(text): | ||
| rest.append(text[last:m.start()]) | ||
| last = end | ||
| rest.append(text[last:]) | ||
| return len(rx.findall("".join(rest))) | ||
|
|
||
|
|
||
| _CONVERTERS: dict[str, Callable[[str, str | None], list[dict[str, Any]]]] = { | ||
| "codex-review": codex_review_findings, | ||
| } | ||
|
|
||
|
|
||
| def convert_answer(fmt: str | None, answer: str, workdir: str | None = None, *, failed: bool = False, | ||
| messages: Sequence[str] = ()) -> str: | ||
| """``answer`` with the findings block of ``fmt`` appended; unchanged when ``fmt`` is ``None``. | ||
|
|
||
| ``messages`` are every agent message of the run, in order: a review may put its findings in an | ||
| earlier message than the last one, so the block is built from all of them (``answer`` is the last). | ||
| Without ``messages`` only ``answer`` is read. | ||
|
|
||
| A ``failed`` run or an empty answer stays as it is: a missing review, not a clean one. So does | ||
| an answer with more finding markers than findings parsed: no block is appended, so a grader | ||
| does not read a review that reported a problem as a clean one. | ||
| """ | ||
| texts = list(messages) or [answer] | ||
| if fmt is None or failed or not any(t.strip() for t in texts): | ||
| return answer | ||
| if fmt not in _CONVERTERS: | ||
| raise ValueError(f"unknown answer format {fmt!r} (known: {', '.join(ANSWER_FORMATS)})") | ||
| findings: list[dict[str, Any]] = [] | ||
| seen: set[tuple[Any, ...]] = set() | ||
| marked = 0 | ||
| for i, t in enumerate(texts): | ||
| parsed = _CONVERTERS[fmt](t, workdir) | ||
| # every marker counts in the last message and in a message that holds a finding; in an earlier | ||
| # message with none, only a [Pn] bullet does: a bare "[P1] means a blocker" line is narration | ||
| full = parsed or i == len(texts) - 1 | ||
| marked += len(parsed) - _repeats(parsed, seen) + _unparsed_marks(t, _FINDING_MARK if full else _BULLET_MARK) | ||
|
Contributor
There was a problem hiding this comment. The same finding printed twice in one message now withholds the whole findings block.
Reproduction,
At e60c7d0 this input gave a block with 2 findings. A summary that repeats a finding after the details is a normal shape, and the grader reads it as a missing review. Suggestion: dedup inside the loop and count markers from the findings that were kept, e.g. add each |
||
| for f in parsed: | ||
| key = (f["file"], f["line"], f["rule"], f["message"]) | ||
| if key not in seen: # a review posted in an interim message and repeated at the end counts once | ||
| seen.add(key) | ||
| findings.append(f) | ||
| if marked > len(findings): # format drift must not read as a clean review | ||
| log.warning("%s: %d finding marker(s) in the answer but %d parsed; no findings block", fmt, marked, | ||
| len(findings)) | ||
| return answer | ||
| return f"{answer}\n\n```json\n{json.dumps(findings, indent=1)}\n```\n" | ||
|
napetrov marked this conversation as resolved.
Comment on lines
+121
to
+125
There was a problem hiding this comment. When the answer has more [Pn] markers than parsed findings, this only logs a warning and still appends the block. An answer with "- [P1] Leak - a.c:2" (hyphen, not em dash) ends with
Contributor
Author
There was a problem hiding this comment. Fixed in b55ccad: an answer with more finding markers than parsed findings gets no findings block and is returned as is (with the warning), as for a failed run. The caplog test now asserts the unchanged answer. |
||
|
|
||
|
|
||
| def _repeats(parsed: list[dict[str, Any]], seen: set[tuple[Any, ...]]) -> int: | ||
| """How many of ``parsed`` were already collected: their markers are not counted twice.""" | ||
| return sum((f["file"], f["line"], f["rule"], f["message"]) in seen for f in parsed) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit (plausible, maintainability): Answer-format list is repeated in the schema and in code
ANSWER_FORMATS,_CONVERTERSand theanswer_formatenum inharness_profile.v1.jsoneach list the formats. Adding a format to one but not the others fails late, after the agent has run, as aValueErrorthatrun_cellrecords as a host-error failure. A small test that the three lists agree would catch this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in bc66cd0: test_answer_format_lists_agree asserts ANSWER_FORMATS, _CONVERTERS and the schema enum match, so drift fails in CI instead of after an agent run.