Skip to content

fix(provider): OpenRouter generation ID collection and provider attribution #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
astefanutti merged 3 commits into from
Nov 5, 2026
+564 −131
Merged

fix(provider): OpenRouter generation ID collection and provider attribution #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83cac99
Fix OpenRouter generation ID collection and provider attribution
Aliciapet11 Nov 4, 2026
95797a0
chore: trigger CI
Aliciapet11 Nov 4, 2026
d895f34
fix(provider): address OpenRouter generation ID review
Aliciapet11 Nov 5, 2026
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
8 changes: 3 additions & 5 deletions agent_eval/agent/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,10 @@ class RunResult:
per_model_turns: Optional[dict] = None # Per-model assistant turn count
permission_denials: Optional[list] = None # [{tool_name, tool_use_id, tool_input}]
raw_output: Optional[dict] = None # Runner-specific parsed output
# Cost provenance (spec 014). `message_ids` is the cost-truth key set: every
# assistant message id in the root stream and the subagent transcripts —
# on a direct OpenRouter connection these are `gen-…` generation ids that
# the backfill prices. The other fields are filled by reconcile (or by a
# runner that knows its own number is an estimate); None = not applicable.
# Claude message ids, used for assistant-turn accounting. OpenRouter
# generation ids live separately in `generation_ids`.
message_ids: Optional[list] = None
generation_ids: Optional[list] = None
cost_source: Optional[str] = None
cost_usd_estimate: Optional[float] = None
providers: Optional[dict] = None
Expand Down
57 changes: 42 additions & 15 deletions agent_eval/agent/claude_code.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
make_prompt_event, inject_timestamp, extract_usage,
count_subagent_turns, combine_per_model_turns,
setup_subagent_hook,
assistant_message_id,
assistant_event_ids, extract_generation_ids,
claude_session_dir_name, claude_session_generation_ids, extract_session_id,
classify_stream_errors,
message_ids_for_run,
generation_ids_for_run, message_ids_for_run,
)
from agent_eval.tools.permissions import compile_permission_rules
from agent_eval.config import (
Expand Down Expand Up @@ -413,7 +414,9 @@ def execute(
cleanup_settings = temp_settings_file
cost_source = None
proc = None
stream_ids_seen = [] # every assistant id parsed, sighted or not
stream_ids_seen = [] # Claude message ids, for turn accounting
stream_message_ids_seen = set()
stream_generation_ids_seen = set()

try:
# Inside the try: the overlay already holds the key, so a failure
Expand Down Expand Up @@ -478,12 +481,16 @@ def _watchdog():
if self._binding is not None:
# Sight generation ids as the stream is read, so
# the backfill starts before the process exits.
mid, mecho = assistant_message_id(obj)
if mid:
message_id, generation_id, mecho = assistant_event_ids(obj)
if message_id and message_id not in stream_message_ids_seen:
msg_index += 1
stream_ids_seen.append(mid) # before sight: it is billed either way
self._binding.sight(mid, message_index=msg_index,
model_echo=mecho)
stream_message_ids_seen.add(message_id)
stream_ids_seen.append(message_id)
if generation_id:
stream_generation_ids_seen.add(generation_id)
self._binding.sight(
generation_id, message_index=msg_index or None,
model_echo=mecho)
msg = _extract_progress(obj, estimate=plan is not None)
if msg:
if msg.startswith("PERMISSION DENIED"):
Expand Down Expand Up @@ -533,6 +540,11 @@ def _watchdog():
timeout_stderr += (f"\nWARNING: {len(denial_list)} permission "
f"denial(s) detected during execution")
message_ids = message_ids_for_run(stream_ids, workspace / "subagents")
session_generation_ids = claude_session_generation_ids(
workspace, extract_session_id(stdout_lines))
generation_ids = generation_ids_for_run(
extract_generation_ids(stdout_lines) | stream_generation_ids_seen | session_generation_ids,
workspace / "subagents")

return RunResult(
exit_code=-1,
Expand All @@ -548,24 +560,31 @@ def _watchdog():
per_model_turns=per_model_turns,
permission_denials=denial_list,
message_ids=message_ids,
**self._provenance(stdout_lines, message_ids, cost_usd, cost_source),
generation_ids=generation_ids,
**self._provenance(stdout_lines, generation_ids, cost_usd, cost_source),
)
except Exception as e:
duration = time.monotonic() - start
# Stop and reap the CLI first: a child left running would keep
# producing billed generations nobody reads (CWE-772). Drain what
# it already wrote — those ids are billed too — then keep every
# streamed id as the result's cost-truth key set and let the
# streamed generation id as the result's cost-truth key set and let the
# binding finish (subagent transcripts, hook ids), or the partial
# run's spend would stay unpriced and unaudited.
if proc is not None:
stdout_lines.extend(_reap(proc))
stream_ids = set(extract_usage(stdout_lines)[3]) | set(stream_ids_seen)
message_ids = message_ids_for_run(stream_ids, workspace / "subagents")
session_generation_ids = claude_session_generation_ids(
workspace, extract_session_id(stdout_lines))
generation_ids = generation_ids_for_run(
extract_generation_ids(stdout_lines) | stream_generation_ids_seen | session_generation_ids,
workspace / "subagents")
return RunResult(
exit_code=-1, stdout="", stderr=str(e), duration_s=duration,
message_ids=message_ids,
**self._provenance(stdout_lines, message_ids, None, cost_source),
generation_ids=generation_ids,
**self._provenance(stdout_lines, generation_ids, None, cost_source),
)
finally:
# The overlay may hold the plan's literal key: gone on every path
Expand All @@ -578,6 +597,8 @@ def _watchdog():

duration = time.monotonic() - start
stdout_text = "\n".join(stdout_lines)
session_generation_ids = claude_session_generation_ids(
workspace, extract_session_id(stdout_lines))

# Clean up session directory now that SubagentStop hooks have fired
# and copied transcripts. Without this, session files accumulate
Expand Down Expand Up @@ -646,6 +667,9 @@ def _watchdog():
)

message_ids = message_ids_for_run(stream_ids, workspace / "subagents")
generation_ids = generation_ids_for_run(
extract_generation_ids(stdout_lines) | stream_generation_ids_seen | session_generation_ids,
workspace / "subagents")
return RunResult(
exit_code=exit_code,
stdout=stdout_text,
Expand All @@ -661,17 +685,18 @@ def _watchdog():
permission_denials=denial_list,
raw_output=raw_output,
message_ids=message_ids,
**self._provenance(stdout_lines, message_ids, cost_usd, cost_source),
generation_ids=generation_ids,
**self._provenance(stdout_lines, generation_ids, cost_usd, cost_source),
)

def _provenance(self, stdout_lines, message_ids, cost_usd, cost_source) -> dict:
def _provenance(self, stdout_lines, generation_ids, cost_usd, cost_source) -> dict:
"""The cost-provenance RunResult fields (spec 014): hand the binding
every id the stream did not show (subagent transcripts, the hook's own
ids), classify the run's visible provider errors, and label the CLI's
own number — an estimate under a plan (Claude Code prices at Anthropic
rates), ``runner:reported`` on Anthropic/Vertex."""
if self._binding is not None:
self._binding.after_run(message_ids)
self._binding.after_run(generation_ids)
error_class, budget = classify_stream_errors(stdout_lines)
return {
"cost_source": cost_source,
Expand Down Expand Up @@ -806,8 +831,10 @@ def _cleanup_session(workspace: Path) -> None:
projects_dir = Path.home() / ".claude" / "projects"
if not projects_dir.exists():
return
encoded = "-" + str(workspace).replace("/", "-")
encoded = claude_session_dir_name(workspace)

@qodo-code-review qodo-code-review Bot Nov 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Cleanup lacks root and home guards 📜 Skill insight ⛨ Security

_cleanup_session passes session_dir to shutil.rmtree after only an existence/type check,
without resolving it and rejecting / or Path.home() immediately before deletion. The changed
session-directory encoding feeds this deletion path during normal runs, so any unexpected path
resolution that widens session_dir reaches recursive cleanup without the required boundary check.
Agent Prompt 
## Issue description
`_cleanup_session` recursively deletes the computed Claude session directory without resolving the target and rejecting the filesystem root or the current user's home directory immediately before deletion.

## Fix Focus Areas
- agent_eval/agent/claude_code.py[831-837]

## Recommended Fix
Resolve `session_dir` immediately before `shutil.rmtree`, reject targets equal to `Path('/').resolve()` or `Path.home().resolve()`, and then perform the recursive deletion only after the safety check passes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

@astefanutti astefanutti Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good fix. For the record: the old encoding ("-" + str(workspace).replace("/", "-")) produced a double leading dash, so cleanup never matched a real session directory — the transcripts were not being deleted before extraction, they were never read, and session directories accumulated. With this change cleanup really deletes, which is why reading the transcript first now matters. Worth a sentence in the description; users will see long-accumulated session directories disappear per run.

session_dir = projects_dir / encoded
if session_dir == projects_dir:
return
if session_dir.exists() and session_dir.is_dir():
shutil.rmtree(session_dir, ignore_errors=True)

Expand Down
5 changes: 4 additions & 1 deletion agent_eval/agent/fullsend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from .base import EvalRunner, RunResult, terminate_process_group
from .stream_capture import (
count_subagent_turns, extract_usage, message_ids_for_run,
combine_per_model_turns,
combine_per_model_turns, extract_generation_ids, generation_ids_for_run,
)

_print_lock = threading.Lock()
Expand Down Expand Up @@ -464,6 +464,8 @@ def _map_run_dir(self, output_dir, workspace, exit_code, stderr, duration,
per_model_turns = combine_per_model_turns(
invocation_subagents, stream_ids_by_model)
message_ids = message_ids_for_run(stream_ids, invocation_subagents)
generation_ids = generation_ids_for_run(
extract_generation_ids(stdout.splitlines()), invocation_subagents)

resolved_model = None
raw_output = {"run_dir": str(run_dir) if run_dir else None}
Expand Down Expand Up @@ -507,6 +509,7 @@ def _map_run_dir(self, output_dir, workspace, exit_code, stderr, duration,
permission_denials=denials,
raw_output=raw_output,
message_ids=message_ids,
generation_ids=generation_ids,
)


Expand Down
Loading
Loading