preserve(harness): checkpoint benchmark envelope work - #612
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
|
Tick the box to add this pull request to the merge queue (same as
|
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+175
to
+176
| from harness.benchmark_envelope import add_envelope | ||
| result = add_envelope(result, repo=repo, profile=profile, plan_hash=command_hash) |
There was a problem hiding this comment.
Suggestion: Wrapping the dry-run payload by replacing result with add_envelope(...) drops legacy top-level fields like result_code and plan_hash that existing consumers and tests still read. Keep those fields in the emitted JSON (for compatibility) or include them in the envelope output before writing the file. [api mismatch]
Severity Level: Critical 🚨
- ❌ Dry-run CLI output breaks test_harness_dry_run expectations.
- ❌ Consumers reading result_code from dry-run output fail.
- ⚠️ Replay logic using dry-run artifacts becomes incompatible.Steps of Reproduction ✅
1. Execute the dry-run CLI path as in `harness/tests/test_run_harness.py:46-61`, which
runs `python3 harness/scripts/run-harness.py run --repo <repo> --out <out_run> --dry-run
...` via `_run()`.
2. The `run_runner()` function in `harness/scripts/run-harness.py:101-165` builds `result`
with keys including `plan_hash`, `plan`, `command_count`, and `reproducibility`.
3. In the dry-run branch at `harness/scripts/run-harness.py:173-176`,
`result["result_code"]` is set and then `result = add_envelope(result, repo=repo,
profile=profile, plan_hash=command_hash)` replaces the original dict with the envelope
returned by `harness/src/harness/benchmark_envelope.py:15-45`, which does not expose
top-level `result_code` or `plan_hash`.
4. `Path(out).write_text(json.dumps(result, indent=2))` at
`harness/scripts/run-harness.py:177` writes this envelope; when the test at
`harness/tests/test_run_harness.py:63-66` reads `output["result_code"]` and
`output["plan_hash"]`, those keys are missing, causing assertion failure and breaking any
consumer expecting the legacy dry-run contract.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/scripts/run-harness.py
**Line:** 175:176
**Comment:**
*Api Mismatch: Wrapping the dry-run payload by replacing `result` with `add_envelope(...)` drops legacy top-level fields like `result_code` and `plan_hash` that existing consumers and tests still read. Keep those fields in the emitted JSON (for compatibility) or include them in the envelope output before writing the file.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+198
to
+199
| from harness.benchmark_envelope import add_envelope | ||
| payload = add_envelope(payload, repo=repo, profile=profile, plan_hash=command_hash) |
There was a problem hiding this comment.
Suggestion: Replacing the normal run payload with envelope-only JSON removes plan/plan_hash, so a later --replay run cannot recover the prior command plan and will compute same_plan/diffs from empty data. Preserve replay-critical fields in persisted output (or embed them in the envelope) so replay comparisons remain correct. [logic error]
Severity Level: Critical 🚨
- ❌ Replay CLI cannot compare plans from prior run envelope.
- ❌ second.json replay.same_plan check in tests fails.
- ⚠️ Plan_diff computed against empty prior_commands is misleading.Steps of Reproduction ✅
1. Run the first non-dry harness run as in `harness/tests/test_run_harness.py:75-88`,
which executes `python3 harness/scripts/run-harness.py run --repo <repo> --out <out_first>
--timeout 2` and triggers `run_runner()` normal path.
2. Inside `run_runner()` at `harness/scripts/run-harness.py:191-197`, `payload =
evidence_payload(discovery, runs, normalization)` builds the harness evidence (with
`commands`, `runs`, `result_code`, etc.), and `payload["plan_hash"] = command_hash` adds
the current plan hash.
3. At `harness/scripts/run-harness.py:198-199`, `payload = add_envelope(payload,
repo=repo, profile=profile, plan_hash=command_hash)` wraps this evidence into the envelope
defined in `harness/src/harness/benchmark_envelope.py:15-45`, discarding top-level `plan`,
`commands`, and `plan_hash` from the persisted JSON written at `run-harness.py:245`.
4. The second run with replay at `harness/tests/test_run_harness.py:90-104` passes
`--replay <out_first>`, so `run_runner()` reads the envelope via `prior =
json.loads(replay_path.read_text())` at `harness/scripts/run-harness.py:204`, then
attempts `prior_commands = prior.get("plan")` and `prior_commands = prior.get("commands",
[])` plus `prior_hash = prior.get("plan_hash")` at lines 205-208; because the envelope
lacks these keys, `prior_commands` becomes an empty list and `prior_hash` is recomputed
from an empty plan, making `same_plan = prior_hash == command_hash` at line 235 false and
`plan_diff` at line 236 a diff against empty commands, causing
`second_payload["replay"]["same_plan"]` checked in
`harness/tests/test_run_harness.py:108-109` to be incorrect and assertions to fail.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/scripts/run-harness.py
**Line:** 198:199
**Comment:**
*Logic Error: Replacing the normal run payload with envelope-only JSON removes `plan`/`plan_hash`, so a later `--replay` run cannot recover the prior command plan and will compute `same_plan`/diffs from empty data. Preserve replay-critical fields in persisted output (or embed them in the envelope) so replay comparisons remain correct.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6af3dceea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| events.append(event) | ||
| passed = payload.get("result_code") == "PASS" | ||
| legacy_digest = _digest(payload) | ||
| return { |
There was a problem hiding this comment.
When run_runner writes the return value from add_envelope(), this replacement object drops the existing top-level result_code, plan_hash, plan/commands, runs, and quality fields. Existing consumers still read those fields, so run --dry-run now raises KeyError in test_harness_dry_run_and_plan_hash, replay from a newly generated artifact compares against an empty prior plan, and commands/execute-phase-2-harness.sh summarizes successful runs as WARN with zero commands. Merge the envelope with the legacy payload or keep the legacy fields at top level.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| def _legacy_module() -> ModuleType: | ||
| script = Path(__file__).resolve().parents[3] / "scripts" / "run-harness.py" |
There was a problem hiding this comment.
In a non-editable wheel install, harness/pyproject.toml packages only src/harness, but this wrapper resolves scripts/run-harness.py outside that package. The helios-harness console script can import harness.scripts.run_harness, then fails with FileNotFoundError when main() or run_runner() tries to load the uninstalled legacy script; moving the runner implementation into the package or including the legacy script in the wheel avoids shipping a broken entrypoint.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| def add_envelope(payload: dict, *, repo: str, profile: str, plan_hash: str) -> dict: | ||
| identity = {"repo": repo, "commit": "0000000", "harness": "helios-harness", "model": "unknown", "task_id": f"plan:{plan_hash}", "hardware": "unknown"} |
There was a problem hiding this comment.
For benchmark runs on the same repo path with the same discovered plan but different Git revisions, this hard-coded commit keeps the deterministic_identity hash, run_id, session_id, and attempt_id identical. Because the payload already has manifest commit metadata before wrapping, the envelope should use that value instead of 0000000/unknown; otherwise benchmark provenance cannot distinguish results from different revisions.
Useful? React with 👍 / 👎.
| if event_type == "compaction": | ||
| event["details"] = {"tokens_before": 0, "tokens_after": 0, "retained_event_ids": [], "dropped_event_ids": []} | ||
| events.append(event) | ||
| passed = payload.get("result_code") == "PASS" |
There was a problem hiding this comment.
Suggestion: This maps every non-PASS result to failed, so WARN runs are incorrectly classified as failures. Add an explicit mapping for warning states so non-failing runs are not reported with failure semantics. [logic error]
Severity Level: Major ⚠️
- ❌ Dry-run benchmark envelopes misreport WARN states as failures.
- ⚠️ Downstream analytics treat advisory runs as failed executions.Steps of Reproduction ✅
1. Run the harness CLI in dry-run mode via the `helios-harness` script defined in
`/workspace/helios-cli/harness/pyproject.toml:3-4`, which ultimately executes
`run_runner()` in `/workspace/helios-cli/harness/scripts/run-harness.py:24-29` with
`args.dry_run` set to `True` (as in the test `test_run_runner_emits_benchmark_envelope` at
`/workspace/helios-cli/harness/tests/test_benchmark_envelope_direct.py:8-16`).
2. In the dry-run branch at `/workspace/helios-cli/harness/scripts/run-harness.py:24-26`,
the code sets `result["result_code"] = "WARN" if not commands else "PASS"` for empty
command plans, then imports `add_envelope` and calls `result = add_envelope(result,
repo=repo, profile=profile, plan_hash=command_hash)`.
3. Inside `add_envelope()` at
`/workspace/helios-cli/harness/src/harness/benchmark_envelope.py:32`, `passed` is computed
as `payload.get("result_code") == "PASS"`, so any non-`PASS` value, including the
intentional advisory `"WARN"` case from `_result_code()` in
`/workspace/helios-cli/harness/src/harness/schema.py:15-19`, is treated as not passed.
4. The envelope’s `result` block is built at
`/workspace/helios-cli/harness/src/harness/benchmark_envelope.py:42`, setting `"status":
"passed" if passed else "failed"` and `"failure_class": "none" if passed else "unknown"`.
For the dry-run WARN case described above, the outgoing envelope reports `"status":
"failed"` and `"failure_class": "unknown"` even though `_result_code()` and its tests in
`/workspace/helios-cli/harness/tests/test_schema.py:4-9` treat `"WARN"` as a non-failing,
advisory result, misclassifying warning-only benchmark runs as failures.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/src/harness/benchmark_envelope.py
**Line:** 32:32
**Comment:**
*Logic Error: This maps every non-`PASS` result to `failed`, so `WARN` runs are incorrectly classified as failures. Add an explicit mapping for warning states so non-failing runs are not reported with failure semantics.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+34
to
+45
| return { | ||
| "schema_version": "1.0.0", | ||
| "tenant_id": "phenotype", "session_id": session_id, "run_id": run_id, "attempt_id": attempt_id, | ||
| "deterministic_identity": {"algorithm": "sha256(canonical-json(inputs))", "canonical_json_sha256": digest, "inputs": identity}, | ||
| "subject": {"repo": repo, "commit": "unknown", "harness": "helios-harness", "runtime": "python", "model": "unknown", "hardware": "unknown"}, | ||
| "lease": {"lease_id": f"lease_{attempt_id[4:20]}", "owner": "helios-harness", "ttl_seconds": 120, "heartbeat_interval_seconds": 20}, | ||
| "task_manifest": {"task_id": f"plan:{plan_hash}", "input_sha256": plan_hash, "timeout_seconds": 1, "assertions": [{"id": "plan_discovered", "kind": "command_plan", "expected": True}], "judge": {"name": "helios-harness", "version": "0.1.0"}}, | ||
| "events": events, | ||
| "result": {"status": "passed" if passed else "failed", "outcome_sha256": legacy_digest, "replay_hash": _digest(events), "failure_class": "none" if passed else "unknown", "artifacts": [{"kind": "report", "uri": f"urn:helios:legacy-evidence:{legacy_digest}", "sha256": legacy_digest}]}, | ||
| "provenance": {"collector": "helios-harness", "collected_at": now, "source_hashes": {"plan": plan_hash}}, | ||
| "signature": {"algorithm": "placeholder", "key_id": "unconfigured", "signature_b64": ""}, | ||
| } |
There was a problem hiding this comment.
Suggestion: This returns a brand-new envelope and drops the original payload fields, which breaks existing consumers that still read top-level legacy keys like result_code and plan_hash. Preserve the original payload (for example under a nested field or by merging required legacy keys) so downstream contracts do not regress. [api mismatch]
Severity Level: Major ⚠️
- ❌ Replay logic misreads prior runs lacking top-level plan.
- ⚠️ External tools expecting result_code/plan_hash break on envelope.Steps of Reproduction ✅
1. Run the legacy harness runner via the CLI entrypoint defined in
`/workspace/helios-cli/harness/pyproject.toml:3-4` (`helios-harness =
"harness.scripts.run_harness:main"`), which calls `main()` in
`harness/src/harness/scripts/run_harness.py:20-22` and then `run_runner()` in
`harness/scripts/run-harness.py:150-52` for a normal run.
2. In `run_runner()` (see `/workspace/helios-cli/harness/scripts/run-harness.py:31-49`),
the code builds a legacy payload containing top-level keys such as `repo`, `profile`,
`plan_hash`, `plan`, `command_count`, and `reproducibility`, then at
`/workspace/helios-cli/harness/scripts/run-harness.py:49` imports `add_envelope` and calls
`payload = add_envelope(payload, repo=repo, profile=profile, plan_hash=command_hash)`.
3. Inspect `add_envelope()` in
`/workspace/helios-cli/harness/src/harness/benchmark_envelope.py:15-45`: at lines 34-45 it
returns a brand-new dictionary with only envelope fields (`schema_version`, `tenant_id`,
`subject`, `task_manifest`, `events`, `result`, `provenance`, `signature`) and does not
preserve the original top-level keys (`plan_hash`, `plan`, `result_code`,
`reproducibility`, etc.) from the incoming `payload`.
4. Later, when the same harness script is run with `--replay` pointing at a previously
produced run file, code at `/workspace/helios-cli/harness/scripts/run-harness.py:205-208`
expects legacy top-level keys: it reads `prior = json.loads(replay_path.read_text())`,
then accesses `prior.get("plan")`, `prior.get("commands", [])`, and
`prior.get("plan_hash")`. Because the new envelope structure returned from
`add_envelope()` at `benchmark_envelope.py:34-45` no longer exposes these keys at the top
level, `prior_plan` and `prior_hash` become empty/None, breaking replay/plan-diff behavior
and any other consumers that rely on legacy top-level fields.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/src/harness/benchmark_envelope.py
**Line:** 34:45
**Comment:**
*Api Mismatch: This returns a brand-new envelope and drops the original payload fields, which breaks existing consumers that still read top-level legacy keys like `result_code` and `plan_hash`. Preserve the original payload (for example under a nested field or by merging required legacy keys) so downstream contracts do not regress.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+58
to
+71
| def git_output(*args: str) -> str: | ||
| try: | ||
| return check_output(["git", "-C", root, *args], text=True, timeout=2).strip() | ||
| except (CalledProcessError, TimeoutExpired, FileNotFoundError): | ||
| return "" | ||
|
|
||
| root = str(repo_root.resolve()) | ||
| try: | ||
| remote = check_output(["git", "-C", root, "remote", "get-url", "origin"], text=True).strip() | ||
| except CalledProcessError: | ||
| remote = git_output("remote", "get-url", "origin") | ||
| except (CalledProcessError, TimeoutExpired): | ||
| remote = "(no-remote)" | ||
| try: | ||
| branch = check_output(["git", "-C", root, "branch", "--show-current"], text=True).strip() | ||
| commit = check_output(["git", "-C", root, "rev-parse", "--short", "HEAD"], text=True).strip() | ||
| except CalledProcessError: | ||
| branch = git_output("branch", "--show-current") | ||
| commit = git_output("rev-parse", "--short", "HEAD") |
There was a problem hiding this comment.
Suggestion: The helper swallows git exceptions and returns an empty string, so the surrounding fallback logic never triggers and sentinel values like (no-remote) / (no-git) are effectively dead code. Either let the helper raise, or move fallback assignment into the helper so failure states are represented consistently. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Repo discovery records empty remote instead of no-remote sentinel.
- ⚠️ Git failure states encoded inconsistently in RepoManifest metadata.Steps of Reproduction ✅
1. Invoke repository discovery using the harness API, e.g.
`Discoverer().discover(DiscoverInput(repo_root=<path>))`, where `Discoverer` is lazily
exported by `harness/src/harness/__init__.py:9-12` and its `discover()` method in
`/workspace/helios-cli/harness/src/harness/discoverer.py:9-12` calls
`RepoManifest.from_repo(root, root.name)`.
2. In `RepoManifest.from_repo()` at
`/workspace/helios-cli/harness/src/harness/interfaces.py:16-23`, `git_output()` is defined
to run `check_output(["git", "-C", root, *args], text=True, timeout=2)` and on any
`CalledProcessError`, `TimeoutExpired`, or `FileNotFoundError` it **swallows** the
exception and returns an empty string `""` instead.
3. The surrounding fallback logic at
`/workspace/helios-cli/harness/src/harness/interfaces.py:25-35` expects those git errors
to bubble up: it wraps `remote = git_output("remote", "get-url", "origin")` in a
`try:`/`except (CalledProcessError, TimeoutExpired)` to fall back to `remote =
"(no-remote)"`, and similarly wraps `branch = git_output("branch", "--show-current");
commit = git_output("rev-parse", "--short", "HEAD")` to fall back to sentinel values
`(no-git)` and empty commit on failure.
4. For a realistic non-git repository (or when `git` is missing), `check_output()` will
raise `CalledProcessError` or `FileNotFoundError`, but because `git_output()` at
interfaces.py:19-23 catches these and returns `""`, the outer `try` blocks never see an
exception, so `remote_url` is set to `""` and `branch`/`commit` become `""` instead of the
intended sentinel strings `(no-remote)` / `(no-git)`. This makes the fallback branches at
lines 28-35 effectively dead code and yields inconsistent failure metadata in the
`RepoManifest` returned to `Discoverer.discover()` at
`/workspace/helios-cli/harness/src/harness/discoverer.py:10-21`.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/src/harness/interfaces.py
**Line:** 58:71
**Comment:**
*Incorrect Condition Logic: The helper swallows git exceptions and returns an empty string, so the surrounding fallback logic never triggers and sentinel values like `(no-remote)` / `(no-git)` are effectively dead code. Either let the helper raise, or move fallback assignment into the helper so failure states are represented consistently.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
|
||
|
|
||
| def _legacy_module() -> ModuleType: | ||
| script = Path(__file__).resolve().parents[3] / "scripts" / "run-harness.py" |
There was a problem hiding this comment.
Suggestion: This wrapper hard-codes a filesystem jump to a top-level script that is not included in the wheel build (packages = ["src/harness"]), so the installed CLI entrypoint can fail at runtime with a missing file. Load the runner from package code (or include the legacy script in the package data) instead of traversing outside the installed package tree. [api mismatch]
Severity Level: Critical 🚨
- ❌ Installed helios-harness CLI fails with ImportError on start.
- ⚠️ Packaging excludes legacy runner required by entrypoint wrapper.Steps of Reproduction ✅
1. Build and install the harness wheel using the configuration in
`/workspace/helios-cli/harness/pyproject.toml:1-8`, which declares the script entrypoint
`helios-harness = "harness.scripts.run_harness:main"` and limits wheel contents to
`packages = ["src/harness"]`, excluding the top-level `harness/scripts` directory.
2. After installation, execute the CLI entrypoint `helios-harness`, which calls `main()`
in `/workspace/helios-cli/harness/src/harness/scripts/run_harness.py:20-22`. `main()`
immediately calls `_legacy_module()` defined at lines 10-17 in the same file.
3. `_legacy_module()` computes `script = Path(__file__).resolve().parents[3] / "scripts" /
"run-harness.py"` at
`/workspace/helios-cli/harness/src/harness/scripts/run_harness.py:11`, expecting to find
`harness/scripts/run-harness.py` three levels above the installed module path.
4. In the installed wheel, only `src/harness` is packaged, so the sibling
`scripts/run-harness.py` file does not exist at runtime.
`importlib.util.spec_from_file_location` at run_harness.py:12 either returns a spec with
`loader is None` or fails due to the missing file; `_legacy_module()` then raises
`ImportError(f"Unable to load harness runner: {script}")` at line 13-14, causing the
`helios-harness` CLI entrypoint to fail on startup in installed environments.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/src/harness/scripts/run_harness.py
**Line:** 11:11
**Comment:**
*Api Mismatch: This wrapper hard-codes a filesystem jump to a top-level script that is not included in the wheel build (`packages = ["src/harness"]`), so the installed CLI entrypoint can fail at runtime with a missing file. Load the runner from package code (or include the legacy script in the package data) instead of traversing outside the installed package tree.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+21
to
+23
| schema_path = Path(__file__).parents[3] / "docs/sessions/20260722-agent-harness-portfolio/artifacts/benchmark_run.schema.json" | ||
| jsonschema.Draft202012Validator(json.loads(schema_path.read_text())).validate(payload) | ||
| except ModuleNotFoundError: |
There was a problem hiding this comment.
Suggestion: This test only ignores missing jsonschema, but not a missing schema file; when jsonschema is installed and that docs path does not exist, the test fails with FileNotFoundError. Guard the schema read with an existence check (or also catch file-not-found) to avoid environment-dependent failures. [possible bug]
Severity Level: Major ⚠️
- ⚠️ Test suite may error when schema file is absent.
- ⚠️ CI reliability depends on non-packaged docs presence.Steps of Reproduction ✅
1. Run the harness test suite (e.g. `pytest`) so that
`test_run_runner_emits_benchmark_envelope` in
`/workspace/helios-cli/harness/tests/test_benchmark_envelope_direct.py:8-32` is executed;
this test drives `run_runner()` via the wrapper in
`harness/src/harness/scripts/run_harness.py:25-27` to produce a benchmark envelope
payload.
2. After reading the output JSON into `payload` at
`/workspace/helios-cli/harness/tests/test_benchmark_envelope_direct.py:17`, the test
enters a `try` block at lines 19-23 that imports `jsonschema` and constructs `schema_path
= Path(__file__).parents[3] /
"docs/sessions/20260722-agent-harness-portfolio/artifacts/benchmark_run.schema.json"`.
3. In an environment where the `jsonschema` package is installed but the
`docs/sessions/20260722-agent-harness-portfolio/artifacts/benchmark_run.schema.json` file
is not present (for example, when running tests against an installed wheel that only
packages `src/harness` as per `/workspace/helios-cli/harness/pyproject.toml:6-7`),
`schema_path.read_text()` at test_benchmark_envelope_direct.py:21-22 raises
`FileNotFoundError`.
4. The `except` block at
`/workspace/helios-cli/harness/tests/test_benchmark_envelope_direct.py:23-24` only catches
`ModuleNotFoundError`, so `FileNotFoundError` propagates, causing the test to error out
instead of being skipped when the external schema file is missing, leading to
environment-dependent test failures.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** harness/tests/test_benchmark_envelope_direct.py
**Line:** 21:23
**Comment:**
*Possible Bug: This test only ignores missing `jsonschema`, but not a missing schema file; when `jsonschema` is installed and that docs path does not exist, the test fails with `FileNotFoundError`. Guard the schema read with an existence check (or also catch file-not-found) to avoid environment-dependent failures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Preservation checkpoint
Pushes the previously local harness work into a reviewable branch without deleting or overwriting the source state.
Scope
Source branch is retained for provenance.
CodeAnt-AI Description
Wrap harness runs in a benchmark envelope and make the CLI importable
What Changed
Impact
✅ More reliable harness output✅ Fewer failures in repos with missing git metadata✅ Easier automated harness testing💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.