test: add direct unit tests for _infer_model_from_metrics (#238)#246
test: add direct unit tests for _infer_model_from_metrics (#238)#246
_infer_model_from_metrics (#238)#246Conversation
There was a problem hiding this comment.
Pull request overview
Adds explicit unit and integration coverage around model inference in copilot_usage.parser, ensuring _infer_model_from_metrics() and build_session_summary() behavior is exercised when currentModel is missing.
Changes:
- Add direct unit tests covering empty/single/multi-model branches of
_infer_model_from_metrics. - Add an integration test verifying
build_session_summaryinfers a model frommodelMetricsfor completed sessions whencurrentModelis absent.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
microsasa
added
the
aw-review-response-attempted
|
Commit pushed:
|
Add tests covering all four branches of _infer_model_from_metrics: - Empty dict returns None - Single key returns that key - Multi-key returns highest requests.count - Tied counts return deterministically Add integration test for build_session_summary when a completed session has multi-model metrics but no currentModel field. Closes #238 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
microsasa
force-pushed
the
fix/238-infer-model-tests-680e15e11c3c49e9
branch
from
2c6cccf to
5ff7528
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Quality Gate: AUTO-APPROVED ✅
Low-impact test-only addition with good coverage. All four branches of _infer_model_from_metrics are covered (empty → None, single key, highest count, tie-breaking by insertion order), plus an integration test through build_session_summary. Tests follow existing patterns, use shared helpers, and include meaningful assertions. No production code changes.
Impact: LOW (test-only)
Code quality: Good
microsasa
added
aw-quality-gate-approved
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Low-impact test-only change with good coverage. Auto-approving for merge.
Evaluated:
- 4 unit tests covering all branches of
_infer_model_from_metrics(empty, single-key, multi-key, tie-breaking) - 1 integration test for
build_session_summarymodel inference whencurrentModelis absent - Uses proper model types, follows existing test conventions
- No production code changes — blast radius is LOW
microsasa
added
aw-quality-gate-approved
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Impact: LOW — Test-only changes (98 lines added to tests/copilot_usage/test_parser.py, no production code modified).
What was evaluated:
- 4 unit tests covering all branches of
_infer_model_from_metrics(empty → None, single key, multi-key max, tie-breaking by insertion order) - 1 integration test verifying
build_session_summaryinvokes inference whencurrentModelis absent - All tests use meaningful assertions and follow existing test patterns (class structure, helpers, type hints)
- No bugs, security issues, or regressions identified
Low-impact test addition with complete branch coverage. Auto-approving for merge.
microsasa
added
aw-quality-gate-approved
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Low-impact test-only addition with good coverage. No production code changes.
What was evaluated:
- 4 unit tests covering every branch of
_infer_model_from_metrics(empty, single key, highest count, tie-breaking) - 1 integration test for
build_session_summarymodel inference whencurrentModelis absent - Tests follow existing patterns, use established fixtures, and assertions are meaningful
- All 8 CI checks passing
Auto-approving for merge.
microsasa
added
aw-quality-gate-approved
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Changes reviewed: 98 lines added to tests/copilot_usage/test_parser.py (test-only, no production code changes).
What was evaluated:
TestInferModelFromMetrics— 4 unit tests covering all branches of_infer_model_from_metrics: empty dict →None, single key → returns it, multi-key → highestrequests.count, tied counts → stable insertion-order tie-breaking. All assertions correctly match the source implementation.TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent— integration test verifyingbuild_session_summaryinfers model via_infer_model_from_metricswhen shutdown event has nocurrentModel. Validates model,is_active, andtotal_premium_requests.
Impact: LOW — Pure test additions with no changes to production code, APIs, or dependencies. Auto-approving for merge.
microsasa
added
aw-quality-gate-approved
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition with thorough coverage. Auto-approving for merge.
What was evaluated:
- 1 file changed:
tests/copilot_usage/test_parser.py(+98 lines, no deletions to existing code) TestInferModelFromMetrics: 4 unit tests covering all branches of_infer_model_from_metrics(empty → None, single key, highest count, deterministic tie-breaking)TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent: integration test verifyingbuild_session_summaryinfers model frommodelMetricswhencurrentModelis absent- Tests follow existing patterns (class-based, reuse
_write_events/_START_EVENT/_USER_MSGfixtures) - No production code, API, or dependency changes
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Changes reviewed: 1 file, +98 lines (test-only)
Adds 5 well-structured tests for _infer_model_from_metrics:
- 4 unit tests covering all branches (empty, single-key, multi-key, tie-breaking)
- 1 integration test verifying
build_session_summaryinfers model whencurrentModelis absent
Code quality: Good — follows existing test patterns, uses proper type annotations, documents tie-breaking behavior intentionally, and uses existing test helpers.
Impact: LOW — test-only addition, zero production code changes.
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition with thorough branch coverage for _infer_model_from_metrics. All four branches (empty, single-key, multi-key highest count, tie-breaking) are directly tested, plus an integration test for build_session_summary when currentModel is absent. Follows existing test patterns. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact, test-only PR adding direct unit tests for _infer_model_from_metrics. All four branches covered (empty → None, single key, highest count wins, deterministic tie-breaking) plus an integration test through build_session_summary. No production code changes. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Low-impact test-only addition with meaningful branch coverage. Auto-approving for merge.
Evaluated:
- 1 file changed:
tests/copilot_usage/test_parser.py(+98 lines, test-only) - 4 unit tests covering all branches of
_infer_model_from_metrics(empty, single key, highest count, tie-breaking) - 1 integration test verifying
build_session_summaryinfers model from metrics whencurrentModelis absent - Tests follow existing patterns and use proper helpers/fixtures
- No source code changes, no security concerns
Impact: LOW — Test-only changes, no blast radius.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition with good coverage. Auto-approving for merge.
Evaluated:
- 4 unit tests for
_infer_model_from_metricscovering all branches (empty, single, multi-key highest count, tie-breaking insertion order) - 1 integration test verifying
build_session_summaryinfers model whencurrentModelis absent - Tests follow existing patterns, use existing helpers, and exercise meaningful behavior
- No source code changes — only
tests/copilot_usage/test_parser.pymodified (+98 lines)
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Impact: LOW — test-only addition, no production code changes.
What was evaluated:
- 4 direct unit tests for
_infer_model_from_metricscovering all branches (empty → None, single key, highest count, tie-breaking by insertion order) - 1 integration test verifying
build_session_summaryinfers model from metrics whencurrentModelis absent - Tests follow existing file conventions (reuse of
_write_events/_START_EVENT/_USER_MSGhelpers, consistent with# pyright: reportPrivateUsage=falsepattern already in use) - No bugs, no security concerns
Auto-approving for merge.
There was a problem hiding this comment.
Low-impact test-only addition with thorough branch coverage. All four branches of _infer_model_from_metrics are directly tested (empty, single key, highest count, tie-breaking), plus a realistic integration test through build_session_summary. No production code changes. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Changes reviewed: Test-only addition — 98 lines added to tests/copilot_usage/test_parser.py, zero production code changes.
Code quality: Good. Four unit tests cover every branch of _infer_model_from_metrics (empty → None, single key, highest count wins, deterministic tie-breaking). Integration test verifies build_session_summary correctly infers model from modelMetrics when currentModel is absent. Tests use correct model types (ModelMetrics, RequestMetrics) and assertions match the actual implementation.
Impact: LOW — test-only change with no production code modifications.
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Changes reviewed: Test-only addition in tests/copilot_usage/test_parser.py (+98 lines). Adds direct unit tests for _infer_model_from_metrics covering all three branches (empty → None, single key → returns it, multi-key → highest requests.count wins) plus a tie-breaking determinism test. Also includes an integration test verifying build_session_summary correctly infers the model when currentModel is absent.
Code quality: Good — follows existing test patterns (class naming, tmp_path fixture, _write_events/parse_events helpers, simple assert style). Tests are meaningful and cover real logic branches.
Impact: LOW — test-only changes, no production code modified.
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact, test-only PR adding direct unit tests for _infer_model_from_metrics and an integration test for build_session_summary.
Evaluated:
- All 4 branches of
_infer_model_from_metricscovered (empty, single, multi, tie) - Tie-breaking test correctly documents Python
max()insertion-order semantics - Integration test validates model inference when
currentModelis absent - Only
tests/copilot_usage/test_parser.pymodified — zero production code changes - Tests follow existing patterns (
_write_events,_START_EVENT, class naming, type annotations)
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition covering all 4 branches of _infer_model_from_metrics plus an integration test for model inference in build_session_summary. Tests are well-structured, meaningful, and follow existing patterns. No production code changes. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Low-impact test-only addition with good coverage. Auto-approving for merge.
Evaluated:
- 1 file changed:
tests/copilot_usage/test_parser.py(+98 lines, 0 source changes) TestInferModelFromMetrics: 4 unit tests covering all branches of_infer_model_from_metrics(empty → None, single key, highest count, tie-breaking)TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent: integration test verifyingbuild_session_summarycorrectly infers model whencurrentModelis absent- Tests use existing helpers (
_write_events,_START_EVENT,_USER_MSG) and follow established patterns - No security concerns (test code only)
There was a problem hiding this comment.
Low-impact test-only addition — auto-approving for merge.
98 lines added to test_parser.py, zero production code changes. Adds 5 well-structured tests:
- 4 unit tests for
_infer_model_from_metricscovering all branches: empty dict →None, single key → returns it, multiple keys → highestrequests.countwins, equal counts → documents insertion-order tie-breaking. - 1 integration test verifying
build_session_summarycorrectly infers model frommodelMetricswhencurrentModelis absent on a completed session.
Tests follow existing patterns, use proper type annotations, and the tie-breaking test is particularly valuable for documenting and locking in Python max semantics.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition — 98 lines of new tests in test_parser.py, zero production code changes.
What was evaluated:
TestInferModelFromMetrics: 4 unit tests covering all branches of_infer_model_from_metrics(empty → None, single key, highest count wins, deterministic tie-breaking by insertion order)TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent: integration test verifying model inference throughbuild_session_summarywith realistic multi-model shutdown event data
Assessment: Tests are meaningful, well-structured, and follow repo conventions. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Code Quality: Good — 5 well-structured tests covering all branches of _infer_model_from_metrics plus an integration test through build_session_summary. Tests are meaningful, use existing helpers correctly, and include good documentation (tie-breaking behavior).
Impact: LOW — test-only changes to tests/copilot_usage/test_parser.py, no production code modified.
Low-impact test addition with complete branch coverage. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact, test-only addition with good coverage. Auto-approving for merge.
What was evaluated:
- 98 lines added to
tests/copilot_usage/test_parser.py, zero production code changes - 4 unit tests covering every branch of
_infer_model_from_metrics(empty → None, single key, highest count, tie-breaking by insertion order) - 1 integration test verifying
build_session_summarycorrectly infers model whencurrentModelis absent - Tests follow existing patterns, use existing helpers (
_write_events,_START_EVENT,_USER_MSG) - Tie-breaking test is well-documented and tests both insertion orders
Impact: LOW — test-only changes, no production code modified.
There was a problem hiding this comment.
Quality Gate — Auto-approved ✅
What was evaluated:
- 98 lines added to
tests/copilot_usage/test_parser.py(no production code changes) - 4 direct unit tests covering every branch of
_infer_model_from_metrics: empty dict →None, single key, highest-count selection, and deterministic tie-breaking - 1 integration test verifying
build_session_summaryinfers the model frommodelMetricswhencurrentModelis absent
Code quality: Good — tests are well-structured, follow existing class-based patterns, reuse existing helpers (_write_events, _START_EVENT, _USER_MSG), and provide meaningful coverage.
Impact: LOW — test-only addition with no changes to production code or APIs.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Changes reviewed: Test-only addition — 98 lines added to tests/copilot_usage/test_parser.py, no production code modified.
What was evaluated:
- 4 unit tests for
_infer_model_from_metricscovering all branches (empty dict → None, single key, highest count wins, deterministic tie-breaking by insertion order) - 1 integration test verifying
build_session_summarycorrectly infers model frommodelMetricswhencurrentModelis absent - Tests correctly match the source function's
max()semantics (first-encountered max for ties) - Follows existing test patterns and conventions in the file
Impact: LOW — purely additive test coverage, single test file, no changes to business logic or APIs.
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Changes reviewed: Test-only addition — 98 new lines in tests/copilot_usage/test_parser.py, no production code changes.
What was evaluated:
TestInferModelFromMetrics— 4 unit tests covering all branches of_infer_model_from_metrics(empty → None, single key → returns it, multi key → highest count wins, tie → insertion-order deterministic)TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent— integration test verifyingbuild_session_summaryinfers model from metrics whencurrentModelis absent- Imports, test class naming, helper usage (
_write_events,_START_EVENT,_USER_MSG,parse_events) all follow existing file conventions ModelMetrics/RequestMetricsusage matches model definitions correctly
Impact: LOW — Test-only changes with no production code modifications. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition with good coverage. Changes limited to tests/copilot_usage/test_parser.py (98 new lines).
What was evaluated:
- 4 unit tests covering all branches of
_infer_model_from_metrics(empty → None, single key, highest count, tie-breaking) - 1 integration test validating
build_session_summaryinfers model from metrics whencurrentModelis absent - Tests follow existing patterns, use established helpers (
_write_events,_START_EVENT,_USER_MSG) - No source code changes, no dependency changes
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only addition with thorough coverage. Auto-approving for merge.
What was evaluated:
- Scope: Single test file (
tests/copilot_usage/test_parser.py) — no production code changes - Unit tests: 4 direct tests covering all branches of
_infer_model_from_metrics(empty → None, single key, highest count, tie-breaking) - Integration test: Verifies
build_session_summaryinfers model frommodelMetricswhencurrentModelis absent on a completed session - Code quality: Tests are meaningful, well-structured, and follow existing patterns (reuses
_write_events,_START_EVENT,_USER_MSGfixtures) - Impact: LOW — test-only, no behavior changes
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Changes reviewed: Test-only addition — 98 new lines in tests/copilot_usage/test_parser.py covering _infer_model_from_metrics.
Tests evaluated:
- 4 direct unit tests covering all branches (empty → None, single key, highest count wins, deterministic tie-breaking)
- 1 integration test verifying
build_session_summaryinfers model from metrics whencurrentModelis absent
Impact: LOW — No source code changes; purely additive test coverage for an existing private function.
CI: All 8 checks passed.
Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Impact: LOW — Test-only changes; single file modified (tests/copilot_usage/test_parser.py, +98 lines). No source code changes.
What was evaluated:
TestInferModelFromMetrics: 4 direct unit tests covering all branches of_infer_model_from_metrics— empty dict →None, single key, multi-key highest count, and tie-breaking by insertion order (tested in both orderings).TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent: Integration test verifyingbuild_session_summarycorrectly infers model via_infer_model_from_metricswhen a completed session has multi-modelmodelMetricsbut nocurrentModel.
Tests are meaningful, well-structured, follow existing class-based patterns with type annotations, and directly address the gaps identified in issue #238. Auto-approving for merge.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Low-impact test-only addition with good coverage. Auto-approving for merge.
What was evaluated:
- Adds 4 direct unit tests for
_infer_model_from_metricscovering all branches (empty → None, single key, highest count wins, tie-breaking) - Adds 1 integration test verifying
build_session_summarycorrectly infers the model whencurrentModelis absent - Tests follow existing patterns (
_write_events, fixture constants,parse_events) - Assertions are meaningful and verify real behavior
- No production code changes — test file only
There was a problem hiding this comment.
Quality Gate: AUTO-APPROVED ✅
Low-impact test-only addition with good coverage. All four branches of _infer_model_from_metrics are covered (empty, single key, multi-key highest count, tie-breaking), plus an integration test verifying build_session_summary correctly infers the model when currentModel is absent. Tests follow existing patterns, use proper fixtures, and are well-documented.
- Impact: LOW (test-only, no production code changes)
- Code quality: Good — meaningful tests, correct assertions, existing helpers reused
There was a problem hiding this comment.
Quality Gate: Auto-approved ✅
Impact: LOW — test-only changes, no production code modified.
What was evaluated:
- 98 lines added to
tests/copilot_usage/test_parser.py— 4 unit tests for_infer_model_from_metricscovering all branches (empty dict, single key, multi-key highest count, tie-breaking by insertion order) plus 1 integration test verifyingbuild_session_summaryinfers the model whencurrentModelis absent. - Tests follow existing conventions (class-based, docstrings, section separators).
- All imports (
ModelMetrics,RequestMetrics,_write_events,_START_EVENT,_USER_MSG,parse_events) are already present in the file. - No bugs, no security concerns, no production code changes.
There was a problem hiding this comment.
Quality Gate: Auto-approved ✅
Low-impact test-only change with good coverage. Adds 5 tests covering all branches of _infer_model_from_metrics (empty, single-key, multi-key, tie-breaking) plus an integration test through build_session_summary. No production code modified. Follows existing test patterns.
There was a problem hiding this comment.
Quality Gate: AUTO-APPROVED ✅
Changes evaluated: Test-only addition — 98 lines added to tests/copilot_usage/test_parser.py, zero production code changes.
What was added:
TestInferModelFromMetrics: 4 direct unit tests covering all branches of_infer_model_from_metrics(empty → None, single key, highest count wins, deterministic tie-breaking)TestBuildSessionSummaryInfersModelWhenCurrentModelAbsent: Integration test verifyingbuild_session_summarycorrectly infers the model whencurrentModelis absent butmodelMetricsis present
Code quality: Good — tests are well-structured, use proper model types (ModelMetrics, RequestMetrics), follow existing patterns, include meaningful docstrings, and reuse existing test fixtures (_START_EVENT, _USER_MSG, _write_events).
Impact: LOW — test-only changes appended to end of existing test file with no modifications to existing tests or production code.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Impact: LOW — test-only changes, no source code modified.
What was evaluated:
- 4 direct unit tests for
_infer_model_from_metricscovering all branches: empty dict →None, single key → returns it, multiple keys → highestrequests.countwins, tie → stable insertion-order behavior documented. - 1 integration test for
build_session_summaryverifying model inference whencurrentModelis absent on a completed session with multi-modelmodelMetrics. - Tests use proper types (
ModelMetrics,RequestMetrics), clear naming, and meaningful assertions.
Auto-approving for merge.
Closes #238
Changes
Adds direct unit tests for
_infer_model_from_metricscovering all four branches:test_empty_returns_noneNonetest_single_key_returns_ittest_multi_key_returns_highest_countrequests.countwinstest_tie_returns_a_model_deterministicallyAdds an integration test (
TestBuildSummaryInfersModelWhenCurrentModelAbsent) verifying thatbuild_session_summarycorrectly infers the model via_infer_model_from_metricswhen a completed session has multi-modelmodelMetricsbut nocurrentModelat either the top level or indata.Validation
All checks pass locally:
ruff check✅ruff format✅pyright— 0 errors ✅pytest --cov --cov-fail-under=80— 506 passed, 98.91% coverage ✅Warning
The following domain was blocked by the firewall during workflow execution:
astral.shTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.