fix: use typed `ToolExecutionData` accessor in `_first_pass` (#1030) by microsasa · Pull Request #1053 · microsasa/cli-tools

microsasa · 2026-04-22T14:12:26Z

Summary

Replaces the raw ev.data.get("model") + isinstance(m, str) check in _first_pass with the typed ev.as_tool_execution().model accessor, aligning with the coding guidelines that prohibit isinstance in business logic.

Changes

src/copilot_usage/parser.py

Replace raw dict access with ev.as_tool_execution().model in the TOOL_EXECUTION_COMPLETE handler
Wrap with try/except (ValidationError, ValueError) matching the error handling pattern used for other event types in _first_pass

tests/copilot_usage/test_parser.py

Add test_first_pass_tool_model_from_typed_accessor: verifies _first_pass populates tool_model correctly via the typed accessor
Add test_first_pass_tool_model_validation_error: creates a malformed event whose model is a valid string (would pass the old isinstance check) but whose toolTelemetry is invalid — asserts tool_model remains None and no exception escapes
Remove test_no_pydantic_validation_for_tool_model: no longer applicable since the fix intentionally uses Pydantic validation

Why

The raw dict access with isinstance was a performance optimization that bypassed the ToolExecutionData model, creating a risk of silent contract divergence if the shape of ToolExecutionData.model ever changes. The coding guidelines permit isinstance only at I/O boundaries when coercing untyped external data into typed models — not in business logic like _first_pass.

Warning

⚠️ Firewall blocked 4 domains

The following domains were blocked by the firewall during workflow execution:

astral.sh
index.crates.io
pypi.org
releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "astral.sh"
    - "index.crates.io"
    - "pypi.org"
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by Issue Implementer · ● 19.9M · ◷

Replace raw dict access + isinstance check with the typed ev.as_tool_execution().model accessor in _first_pass, matching the pattern used by other event types in the same function. The raw dict access bypassed ToolExecutionData validation, risking silent contract divergence if the model field shape ever changes. The typed accessor ensures tool_model extraction follows the same Pydantic-validated path as all other event data. Wrap with try/except (ValidationError, ValueError) consistent with error handling used elsewhere in _first_pass for other event types. Tests: - Add test_first_pass_tool_model_from_typed_accessor: validates tool_model is correctly populated via the typed accessor. - Add test_first_pass_tool_model_validation_error: malformed data with a valid model string but invalid toolTelemetry is rejected by Pydantic validation, leaving tool_model as None. - Remove test_no_pydantic_validation_for_tool_model: no longer applicable since the fix intentionally uses Pydantic validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Updates the session summary “first pass” parsing to extract tool_model via the typed SessionEvent.as_tool_execution().model accessor (instead of raw ev.data inspection), aligning _first_pass with the project’s typed-event access patterns.

Changes:

Use ev.as_tool_execution().model in _first_pass for tool.execution_complete, with ValidationError/ValueError handling.
Update parser unit tests to validate the typed-accessor behavior, including the case where raw model looks valid but typed validation fails.
Remove the prior performance-focused test that asserted Pydantic validation was never invoked for tool model extraction.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/copilot_usage/parser.py`	Switch tool model extraction to typed accessor with exception handling in `_first_pass`.
`tests/copilot_usage/test_parser.py`	Add/adjust tests to enforce typed validation semantics for `tool_model` extraction.

+                except (ValidationError, ValueError):
+                    m = None


-                if isinstance(m, str) and m:
+                try:
+                    m = ev.as_tool_execution().model
+                except (ValidationError, ValueError):


- Narrow except clause to ValidationError only (ValueError is unreachable since the event type is already checked before calling as_tool_execution) - Add debug logging on validation failure, matching the pattern used by SESSION_START and SESSION_SHUTDOWN branches in _first_pass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-04-22T14:26:17Z

Commit pushed: 0cf3a59

Generated by Review Responder

The test_tool_execution_complete_bad_data_silently_skipped test asserted that no debug logs were emitted for malformed tool.execution_complete events. This was correct when _first_pass used raw dict access (ev.data.get("model")), but the PR switched to the typed as_tool_execution() accessor which now catches ValidationError and emits a debug log — matching the pattern used for session.start and session.shutdown handlers. Rename the test to test_tool_execution_complete_bad_data_logs_debug and flip the assertion from 'not any' to 'any' so it expects the debug message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-04-22T15:13:30Z

Commit pushed: b77dcca

Generated by CI Fixer

microsasa · 2026-04-22T15:13:31Z

CI fix pushed (18e5435)

The check job failed because test_tool_execution_complete_bad_data_silently_skipped asserted that no debug log was emitted for malformed tool.execution_complete events. This was correct for the original raw ev.data.get("model") path, but the review-response commit (38ceef8) added logger.debug(...) inside the new except ValidationError handler — matching the session.start / session.shutdown pattern. The test was never updated to expect the new log.

Fix: Renamed the test to test_tool_execution_complete_bad_data_logs_debug, updated the docstring, and flipped assert not any(...) → assert any(...) so it now expects the debug message.

Warning

⚠️ Firewall blocked 4 domains

The following domains were blocked by the firewall during workflow execution:

astral.sh
objects.githubusercontent.com
pypi.org
releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "astral.sh"
    - "objects.githubusercontent.com"
    - "pypi.org"
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by CI Fixer · ● 36.7M · ◷

microsasa · 2026-04-22T15:14:06Z

❌ Pipeline orchestrator: CI still failing after ci-fixer attempt. Marking as stuck for human review.

Copilot

Pull request overview

Updates the session summary “first pass” parsing to derive tool_model via the typed SessionEvent.as_tool_execution().model accessor, aligning _first_pass with the repository’s typed-event parsing approach and removing raw-dict + isinstance business-logic checks.

Changes:

Switch _first_pass’s TOOL_EXECUTION_COMPLETE handling from ev.data.get("model") to ev.as_tool_execution().model, with debug logging on Pydantic validation failure.
Update/replace tests to assert tool_model is populated via the typed accessor and remains unset when typed validation fails.
Adjust the existing debug-logging expectation for malformed tool.execution_complete events.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`src/copilot_usage/parser.py`	Uses `as_tool_execution().model` in `_first_pass` and logs+skips malformed tool events.
`tests/copilot_usage/test_parser.py`	Updates debug-log test and adds typed-accessor coverage for tool model extraction and validation failures.

+                try:
+                    m = ev.as_tool_execution().model
+                except ValidationError as exc:
+                    logger.debug(


Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

microsasa · 2026-04-23T05:28:25Z

Manual unstick: applied ruff format/ruff check --fix locally, CI should now pass. Stripped aw-pr-stuck:ci + aw-ci-fix-attempted so the pipeline can resume if another round is needed.

Add defensive ValueError catch to TOOL_EXECUTION_COMPLETE handler in _first_pass, matching the documented exceptions of as_tool_execution(). The etype guard makes ValueError currently unreachable, but catching it prevents escape if the guard is ever restructured. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-04-23T05:43:26Z

Commit pushed: c67de47

Generated by Review Responder

Copilot

Pull request overview

This PR updates the session summary parser to extract the tool model via the typed SessionEvent.as_tool_execution().model accessor instead of raw dict access, aligning _first_pass with the codebase’s typed-model parsing approach.

Changes:

Switch _first_pass’s tool.execution_complete handling from ev.data.get("model") to ev.as_tool_execution().model.
Add try/except (ValidationError, ValueError) around the typed accessor and emit debug logs on malformed tool payloads (consistent with other event handlers).
Update/replace tests to validate the typed accessor behavior and the ValidationError skip path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/copilot_usage/parser.py`	Uses `as_tool_execution().model` in `_first_pass` with ValidationError/ValueError handling and debug logging.
`tests/copilot_usage/test_parser.py`	Updates existing debug-log expectations for malformed tool events and adds direct `_first_pass` tests covering typed accessor success/failure paths.

The diff-coverage check (≥90% on new lines) failed because the except-ValueError handler in _first_pass is explicitly unreachable defensive code with no test exercising it. Add a test that patches as_tool_execution to raise ValueError, verifying the handler keeps tool_model as None. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-04-23T06:07:56Z

Commit pushed: bfd34eb

Generated by CI Fixer

microsasa · 2026-04-23T06:07:57Z

CI fix pushed (a2d7ca5)

The check job failed at the diff-coverage step (new/changed lines ≥ 90%) — make ci itself passed. The except ValueError handler in _first_pass is explicitly documented as "currently unreachable" defensive code, so no existing test exercises it, leaving those lines uncovered.

Fix: Added test_first_pass_tool_model_value_error which patches SessionEvent.as_tool_execution to raise ValueError, verifying the handler keeps tool_model as None.

Warning

⚠️ Firewall blocked 4 domains

The following domains were blocked by the firewall during workflow execution:

astral.sh
index.crates.io
pypi.org
static.rustlang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "astral.sh"
    - "index.crates.io"
    - "pypi.org"
    - "static.rustlang.org"

See Network Configuration for more information.

Generated by CI Fixer · ● 22.4M · ◷

microsasa · 2026-04-23T06:11:31Z

CI fix already attempted once — stopping to prevent loops. Manual intervention needed.

Generated by CI Fixer · ● 372.8K · ◷

microsasa · 2026-04-23T06:24:55Z

❌ Pipeline orchestrator: review-response loop reached 5 rounds. Marking as stuck for human review.

Copilot AI review requested due to automatic review settings April 22, 2026 14:12

microsasa added the aw Created by agentic workflow label Apr 22, 2026

microsasa enabled auto-merge April 22, 2026 14:12

Copilot started reviewing on behalf of microsasa April 22, 2026 14:13 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread src/copilot_usage/parser.py Outdated

Comment on lines +804 to +805

except (ValidationError, ValueError):

m = None

Comment thread src/copilot_usage/parser.py Outdated

if isinstance(m, str) and m:

try:

m = ev.as_tool_execution().model

except (ValidationError, ValueError):

microsasa added aw-review-response-1 Review response round 1 aw-review-response-2 Review response round 2 labels Apr 22, 2026

microsasa added the aw-ci-fix-attempted CI fixer has attempted to fix CI label Apr 22, 2026

Copilot AI review requested due to automatic review settings April 22, 2026 15:13

Copilot started reviewing on behalf of microsasa April 22, 2026 15:14 View session

microsasa added the aw-pr-stuck:ci label Apr 22, 2026

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread src/copilot_usage/parser.py

Comment on lines +802 to +805

try:

m = ev.as_tool_execution().model

except ValidationError as exc:

logger.debug(

fix: ruff format on test_parser.py

6aee7e1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

microsasa removed aw-pr-stuck:ci aw-ci-fix-attempted CI fixer has attempted to fix CI labels Apr 23, 2026

microsasa added the aw-review-response-3 Review response round 3 label Apr 23, 2026

Copilot AI review requested due to automatic review settings April 23, 2026 05:43

Copilot started reviewing on behalf of microsasa April 23, 2026 05:43 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

microsasa added the aw-ci-fix-attempted CI fixer has attempted to fix CI label Apr 23, 2026

microsasa added the aw-review-response-4 Responder dispatch round 4 (orchestrator-managed) label Apr 23, 2026

microsasa added aw-review-response-5 Responder dispatch round 5 (orchestrator-managed) aw-pr-stuck:review labels Apr 23, 2026

Conversation

microsasa commented Apr 22, 2026

Summary

Changes

Why

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

microsasa commented Apr 22, 2026

Uh oh!

microsasa commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

microsasa commented Apr 23, 2026

Uh oh!

github-actions Bot commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Apr 23, 2026

Uh oh!

microsasa commented Apr 23, 2026

Uh oh!

microsasa commented Apr 23, 2026

Uh oh!

microsasa commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants