Skip to content

Consolidate duplicated automated-test-completeness logic between details.py and statistics_service.py #412

Description

@jimisola

Context

#411 fixed a bug where `get_requirement_status`/`get_requirements_status_all` (`src/reqstool/common/queries/details.py`) computed automated-test completeness differently than `StatisticsService` (`src/reqstool/services/statistics_service.py`), causing the MCP server's reported `meets_requirements` to disagree with the CLI's `status` command.

The fix in #411 added a new helper, `_build_automated_test_summary`, in `details.py` that independently re-derives "is this SVC's automated-test obligation satisfied" via a different traversal strategy than `StatisticsService` (`TestStats.is_completed()`, `_get_test_stats`, `_get_annotated_automated_test_results_for_req`). Both now encode the same concept but via separate code paths — which is exactly the kind of duplication that caused the original bug (the two paths drifted apart silently).

Proposal

Consolidate the two "automated test completeness" computations into one shared implementation that both `StatisticsService` (used by `status`/`report`/`export`) and the MCP query functions in `details.py` can call, so they can't drift again. Possible approaches:

  • Have `details.py`'s lightweight check delegate to (a thin wrapper around) `StatisticsService`'s per-requirement stats computation.
  • Or extract the shared predicate into a small standalone function/module that both consumers call.

Flagged by `/x:full-pr-review` on #411 (finding #4) and intentionally deferred to avoid expanding that bugfix's scope.

Scope: emit the unified shape directly (no legacy dict)

Folded in from #413 (now closed). Backwards compatibility for MCP client output is not required, so this change should go further than a pure de-duplication:

  • Extract a single per-requirement predicate (producing a RequirementStatus) that both StatisticsService._calculate_requirement_stats and the details.py status functions call — this is the actual drift fix.
  • The two details.py-backed MCP tools (get_requirement_status, get_requirements_status) should serialize that RequirementStatus directlycompleted, implementation_type, automated_tests/manual_tests with total + not_applicable — matching StatisticsService.to_status_dict()'s per-req shape. No mapping back to the old test_summary/meets_requirements dict.
  • Net result: one predicate, one output shape, three status tools (get_status, get_requirement_status, get_requirements_status) all consistent.

Behavior change (deliberate)

Routing details.py through the real predicate also fixes computation differences beyond what #411 patched. meets_requirements/completed output will change for requirements with:

  • post-build-phase-only SVCs (the predicate filters to phase == BUILD unless include_post_build=True; the old details.py path used all SVCs), and
  • "not applicable" cases (no SVC expects automated tests / MVRs), and
  • test annotations without recorded executions (the predicate walks annotations → matches FQN → marks MISSING; the old path read test results directly).

This is the intended outcome — MCP verdicts now match status exactly — not an accidental regression. The MCP tools should default include_post_build=False to match the status CLI default, and may optionally expose it as a parameter for parity with --with-post-tests.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions