Skip to content

Add bounded Hermes failure diagnosis - #17

Open
divo12 wants to merge 3 commits into
codex/paired-statistical-evidencefrom
codex/hermes-agent-diagnoser
Open

Add bounded Hermes failure diagnosis#17
divo12 wants to merge 3 commits into
codex/paired-statistical-evidencefrom
codex/hermes-agent-diagnoser

Conversation

@divo12

@divo12 divo12 commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a typed Hermes diagnoser at the existing subprocess boundary
  • send one bounded, content-verified evidence packet over stdin through an audited Hermes 0.20.0 Python bridge
  • isolate HERMES_HOME, enforce the installed version, and expose no filesystem, terminal, browser, web, memory, skill, plugin, or subagent tools
  • fail closed on stale assets, oversized prompts, timeouts, malformed output, invalid anchors, wrong traces, and unconnected components
  • document Judgment Labs/Hermes research and Azure operator mapping without reading or persisting secrets

Verification

  • 166 tests passed
  • Ruff and mypy --strict clean
  • Bandit and pip-audit clean
  • wheel and sdist build
  • forbidden dynamic/raw type scan clean
  • independent reviewer: APPROVE

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/research/2026-08-22-failure-mining-evals-tuning-ab-research.md">

<violation number="1" location="docs/research/2026-08-22-failure-mining-evals-tuning-ab-research.md:153">
P3: This PR adds a distinct Agentic diagnosis gap and PR16 to implement it, so the stacked plan now covers four work items, but the Executive conclusion and Methodology still claim \"Three gaps are important enough to implement now\" / \"the three immediate gaps\". Update the summary to count the new Hermes diagnoser gap, or the plan under-describes the scope being shipped.</violation>
</file>

<file name="src/ofw/diagnosis.py">

<violation number="1" location="src/ofw/diagnosis.py:238">
P2: When a model fingerprint field contains a NUL byte, `HermesDiagnoser.diagnose` raises `ValueError` while spawning the subprocess instead of abstaining. Reject NUL bytes in all model fields before constructing this command, or catch the spawn error and return an abstention.</violation>
</file>

<file name="src/ofw/_hermes_diagnosis_runner.py">

<violation number="1" location="src/ofw/_hermes_diagnosis_runner.py:28">
P2: When connected components use arbitrary filenames, Hermes receives no component kind or tool/subagent name for their contents, so its component attribution is guesswork and can pass parent validation for the wrong connected component. Preserve component kind and asset name in the evidence packet before prompting Hermes.</violation>

<violation number="2" location="src/ofw/_hermes_diagnosis_runner.py:108">
P2: When a registered asset contains non-UTF-8 bytes, `payload.decode()` aborts the entire Hermes diagnosis run even though harness assets accept arbitrary file bytes. Encode non-text assets losslessly in the evidence packet, or enforce a UTF-8-only asset contract before this runner.</violation>
</file>

You're on the cubic free plan with 19 free PR reviews remaining this month. Upgrade for unlimited reviews.

Re-trigger cubic

Comment thread src/ofw/diagnosis.py
(
"hermes-diagnoser-v2",
*self.command.arguments,
self.model.provider,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a model fingerprint field contains a NUL byte, HermesDiagnoser.diagnose raises ValueError while spawning the subprocess instead of abstaining. Reject NUL bytes in all model fields before constructing this command, or catch the spawn error and return an abstention.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/diagnosis.py, line 238:

<comment>When a model fingerprint field contains a NUL byte, `HermesDiagnoser.diagnose` raises `ValueError` while spawning the subprocess instead of abstaining. Reject NUL bytes in all model fields before constructing this command, or catch the spawn error and return an abstention.</comment>

<file context>
@@ -199,6 +216,67 @@ def diagnose(
+                (
+                    "hermes-diagnoser-v2",
+                    *self.command.arguments,
+                    self.model.provider,
+                    self.model.model,
+                    self.model.reasoning,
</file context>

payload = source.read_bytes()
if digest_bytes(payload) != asset.digest:
raise ValueError("asset digest changed")
evidence.append(ConnectedAssetEvidence(relative, payload.decode()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a registered asset contains non-UTF-8 bytes, payload.decode() aborts the entire Hermes diagnosis run even though harness assets accept arbitrary file bytes. Encode non-text assets losslessly in the evidence packet, or enforce a UTF-8-only asset contract before this runner.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/_hermes_diagnosis_runner.py, line 108:

<comment>When a registered asset contains non-UTF-8 bytes, `payload.decode()` aborts the entire Hermes diagnosis run even though harness assets accept arbitrary file bytes. Encode non-text assets losslessly in the evidence packet, or enforce a UTF-8-only asset contract before this runner.</comment>

<file context>
@@ -0,0 +1,141 @@
+        payload = source.read_bytes()
+        if digest_bytes(payload) != asset.digest:
+            raise ValueError("asset digest changed")
+        evidence.append(ConnectedAssetEvidence(relative, payload.decode()))
+    return tuple(evidence)
+
</file context>



@dataclass(frozen=True, slots=True)
class ConnectedAssetEvidence:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When connected components use arbitrary filenames, Hermes receives no component kind or tool/subagent name for their contents, so its component attribution is guesswork and can pass parent validation for the wrong connected component. Preserve component kind and asset name in the evidence packet before prompting Hermes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/_hermes_diagnosis_runner.py, line 28:

<comment>When connected components use arbitrary filenames, Hermes receives no component kind or tool/subagent name for their contents, so its component attribution is guesswork and can pass parent validation for the wrong connected component. Preserve component kind and asset name in the evidence packet before prompting Hermes.</comment>

<file context>
@@ -0,0 +1,141 @@
+
+
+@dataclass(frozen=True, slots=True)
+class ConnectedAssetEvidence:
+    relative_path: Path
+    content: str
</file context>

Comment thread docs/specs/2026-08-23-hermes-failure-mining.md Outdated
| Trials | Repeat index and paired attempt identity exist | No uncertainty or discordant-pair report | PR15: exact paired evidence |
| Candidate tuning | File allowlist, manifest, expected effects, full result manifests | No layered next-iteration index of feedback/raw evidence | PR16: experience index |
| Candidate tuning | File allowlist, manifest, expected effects, full result manifests | No layered next-iteration index of feedback/raw evidence | PR17: experience index |
| Agentic diagnosis | Python diagnoser over one immutable snapshot | No safe external agent adapter for targeted component inspection | PR16: sandboxed Hermes diagnoser |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This PR adds a distinct Agentic diagnosis gap and PR16 to implement it, so the stacked plan now covers four work items, but the Executive conclusion and Methodology still claim "Three gaps are important enough to implement now" / "the three immediate gaps". Update the summary to count the new Hermes diagnoser gap, or the plan under-describes the scope being shipped.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/research/2026-08-22-failure-mining-evals-tuning-ab-research.md, line 153:

<comment>This PR adds a distinct Agentic diagnosis gap and PR16 to implement it, so the stacked plan now covers four work items, but the Executive conclusion and Methodology still claim \"Three gaps are important enough to implement now\" / \"the three immediate gaps\". Update the summary to count the new Hermes diagnoser gap, or the plan under-describes the scope being shipped.</comment>

<file context>
@@ -123,7 +149,8 @@ LangSmith’s comparative evaluation API can randomize answer order to mitigate
 | Trials | Repeat index and paired attempt identity exist | No uncertainty or discordant-pair report | PR15: exact paired evidence |
-| Candidate tuning | File allowlist, manifest, expected effects, full result manifests | No layered next-iteration index of feedback/raw evidence | PR16: experience index |
+| Candidate tuning | File allowlist, manifest, expected effects, full result manifests | No layered next-iteration index of feedback/raw evidence | PR17: experience index |
+| Agentic diagnosis | Python diagnoser over one immutable snapshot | No safe external agent adapter for targeted component inspection | PR16: sandboxed Hermes diagnoser |
 | Judge quality | Frozen Python/command verifiers | No calibrated model-judge adapter | Defer until a real adapter is requested |
 | Implicit dissatisfaction | Metadata/status/scores collected; content minimized | Corrections/rephrasing/abandonment unavailable and sensitive | Defer behind opt-in content policy |
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant