From fbd48996a1f3414b88fc0a1f5fd0b2cd6d380cb6 Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Mon, 22 Jun 2026 17:54:44 +0200 Subject: [PATCH 1/7] docs(openspec): propose consolidating requirement verdict computation Adds the OpenSpec proposal/design/tasks for unifying the per-requirement "complete" verdict behind a single computation, replacing the duplicate predicate in common/queries/details.py that has already drifted from StatisticsService once (#411). Signed-off-by: Jimisola Laursen --- CLAUDE.md | 1 + .../.openspec.yaml | 2 + .../consolidate-requirement-verdict/design.md | 82 +++++++++++++++++++ .../proposal.md | 48 +++++++++++ .../specs/mcp/spec.md | 7 ++ .../consolidate-requirement-verdict/tasks.md | 30 +++++++ openspec/config.yaml | 20 +++++ 7 files changed, 190 insertions(+) create mode 100644 openspec/changes/consolidate-requirement-verdict/.openspec.yaml create mode 100644 openspec/changes/consolidate-requirement-verdict/design.md create mode 100644 openspec/changes/consolidate-requirement-verdict/proposal.md create mode 100644 openspec/changes/consolidate-requirement-verdict/specs/mcp/spec.md create mode 100644 openspec/changes/consolidate-requirement-verdict/tasks.md create mode 100644 openspec/config.yaml diff --git a/CLAUDE.md b/CLAUDE.md index 03ad44f5..8146b6a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -161,6 +161,7 @@ Full rationale in `docs/DESIGN.md`. - **Cycle detection covers both chains**: `CircularImportError` for the import chain, `CircularImplementationError` for the implementation chain. - **FK constraints scope evidence from implementation children**: SVCs/MVRs/annotations referencing out-of-scope requirements are rejected by SQLite FK checks on insert — no explicit filtering needed. - **Test results need explicit scoping**: no FK (keyed by FQN), so a scope check is required when inserting test results from implementation children. +- **The requirement "complete" verdict has ONE source of truth**: `StatisticsService`'s per-requirement computation (producing `RequirementStatus`). Every consumer — `status`/`report`/`export`, the MCP tools (`get_status`, `get_requirement_status`, `get_requirements_status`), and LSP — MUST derive completeness from that same computation. Do NOT re-implement "is this requirement met / are its automated tests satisfied" anywhere else (e.g. a parallel helper in `common/queries/details.py`). Two parallel verdict paths silently drifted and caused bugs (#411, #412); a private re-derivation is the regression to guard against. When the verdict logic must be reused, extract/call the shared predicate — never copy its traversal. ## Key Conventions diff --git a/openspec/changes/consolidate-requirement-verdict/.openspec.yaml b/openspec/changes/consolidate-requirement-verdict/.openspec.yaml new file mode 100644 index 00000000..6c351aca --- /dev/null +++ b/openspec/changes/consolidate-requirement-verdict/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-21 diff --git a/openspec/changes/consolidate-requirement-verdict/design.md b/openspec/changes/consolidate-requirement-verdict/design.md new file mode 100644 index 00000000..fd53b8d7 --- /dev/null +++ b/openspec/changes/consolidate-requirement-verdict/design.md @@ -0,0 +1,82 @@ +## Context + +Two code paths compute whether a requirement is "complete": + +- `StatisticsService._calculate_requirement_stats` (`src/reqstool/services/statistics_service.py`) + produces a `RequirementStatus` (with `TestStats` for automated and manual evidence) and is the + authoritative path behind `status`, `report`, `export`, and the MCP `get_status` tool. +- `common/queries/details.py` (`_compute_meets` + `_build_automated_test_summary`) re-derives the + same verdict via a different traversal and powers the MCP `get_requirement_status` and + `get_requirements_status` tools. + +The two are not equivalent, even after #411's patch: + +| Aspect | `StatisticsService` | `details.py` | +| --- | --- | --- | +| SVC phase scoping | build-phase only unless `include_post_build` | all SVCs, no phase filter | +| "applies at all" gate | `not_applicable` when no SVC expects automated/MVR evidence | always requires all-passing | +| automated test source | walks test annotations, matches by FQN, marks MISSING per annotation | reads `get_test_results_for_svc` directly | +| no-qualifying-SVC | `completed=False` unless some SVC expects evidence | only requires a non-empty SVC list | + +`get_status` already uses `StatisticsService` directly, so the inconsistency is isolated to the +two per-requirement/scoped MCP tools. The constraint that matters: there is no backwards-compat +requirement on MCP tool output, so the fix can change both the schema and the verdict values. + +## Goals / Non-Goals + +**Goals:** +- One per-requirement verdict computation, called by both `StatisticsService` and `details.py`. +- MCP `get_requirement_status` / `get_requirements_status` report the same `completed` verdict and + the same output structure as the `status` command for the same input. +- Delete `_compute_meets` and `_build_automated_test_summary`. + +**Non-Goals:** +- Changing the `status`, `report`, or `export` command behavior or output. +- Changing `get_status` (already unified). +- Adding new MCP tools or changing transports / dataset resolution. + +## Decisions + +### Extract a freestanding per-requirement predicate (not a wrapper) + +Introduce a single function that computes a `RequirementStatus` for one requirement given the +repository and a `include_post_build` flag. `StatisticsService._calculate_requirement_stats` +calls it inside its loop and keeps owning totals accumulation; `details.py` calls it per id. + +- **Why over a thin wrapper** (have `details.py` build a `StatisticsService` and read + `.requirement_statistics[urn_id]`): a wrapper makes the two agree but keeps two algorithms; + it also couples the MCP query layer to `StatisticsService` internals. Extracting the predicate + yields one literal source of truth and cleanly separates "verdict for one requirement" from + "aggregate totals across all requirements." +- **Trade-off**: requires untangling `_calculate_requirement_stats` from + `_update_requirement_totals`, which today run in the same pass. This is the bulk of the work. + +### Emit the unified shape directly from the MCP tools + +The two `details.py` status functions serialize the `RequirementStatus` the same way +`StatisticsService.to_status_dict()` does per requirement (`completed`, `implementation_type`, +`automated_tests`/`manual_tests` with `total` and `not_applicable`). No mapping back to the old +`meets_requirements` / flat `test_summary` dict. + +- **Why**: backwards compatibility is not required (project decision), so preserving the legacy + dict would only perpetuate a second shape. A shared per-requirement serializer keeps all status + tools on one schema. + +### Default MCP tools to build-phase-only scoping + +To match the `status` command default, the predicate is invoked with `include_post_build=False` +from the MCP tools. Optionally expose the flag as a tool parameter for parity with +`status --with-post-tests` (can be deferred). + +## Risks / Trade-offs + +- **Untangling totals from per-req stats introduces regressions in `status`/`report`/`export`** → + Mitigation: keep totals accumulation in `StatisticsService`; the extracted predicate returns the + same `RequirementStatus` the loop already builds, so totals read identical inputs. Guard with the + existing statistics unit tests and the CLAUDE.md regression smoke diffs (must be byte-identical for + `status`/`report`). +- **MCP verdict/shape change surprises consumers** → Mitigation: documented as intentional BREAKING + in the proposal; the new values are the correct, `status`-consistent ones. +- **Hidden behavioral difference between the two old paths becomes visible** → Mitigation: add MCP + tests asserting `get_requirement_status` / `get_requirements_status` agree with + `StatisticsService` on the same fixtures (including the `REQ_ext002_300` divergence case). diff --git a/openspec/changes/consolidate-requirement-verdict/proposal.md b/openspec/changes/consolidate-requirement-verdict/proposal.md new file mode 100644 index 00000000..d20d73ff --- /dev/null +++ b/openspec/changes/consolidate-requirement-verdict/proposal.md @@ -0,0 +1,48 @@ +## Why + +The "is this requirement complete?" verdict is computed by two independent code paths that +have already drifted once: `StatisticsService` (powering `status`/`report`/`export` and the +MCP `get_status` tool) and a separate set of helpers in `common/queries/details.py` +(`_compute_meets`, `_build_automated_test_summary`) powering the MCP `get_requirement_status` +and `get_requirements_status` tools. Issue #411 patched a symptom of that drift; this change +removes the second implementation so the two can never disagree again. + +## What Changes + +- Extract a single per-requirement verdict computation that produces one `RequirementStatus` + value, used by both `StatisticsService` (in its per-requirement loop) and the `details.py` + MCP status functions. +- Delete the parallel predicate from `details.py` (`_compute_meets`, + `_build_automated_test_summary`). +- **BREAKING (MCP output):** the MCP `get_requirement_status` and `get_requirements_status` + tools emit the unified status shape directly (`completed`, `implementation_type`, + `automated_tests`/`manual_tests` objects with `total` and `not_applicable`), replacing the + old `meets_requirements` / flat `test_summary` shape. Backwards compatibility for MCP client + output is explicitly not required. +- **BREAKING (verdict values):** routing the MCP tools through the real predicate changes + reported verdicts for requirements with post-build-phase-only SVCs, "not applicable" cases, + and test annotations without recorded executions — these now match the `status` command + exactly. The MCP tools default to build-phase-only scoping to match the `status` default. + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `mcp`: adds a requirement that the per-requirement MCP status tools report a completion + verdict and output structure identical to the `status` command, derived from a single shared + verdict computation. + +## Impact + +- `src/reqstool/common/queries/details.py` — removes the duplicate predicate; status functions + delegate to the shared computation. +- `src/reqstool/services/statistics_service.py` — per-requirement verdict logic extracted so it + can be shared (totals accumulation stays here). +- `src/reqstool/mcp/server.py` — `get_requirement_status` / `get_requirements_status` tool + output shape changes. +- reqstool SSOT (`docs/reqstool/requirements.yml`, `software_verification_cases.yml`) — new + `MCP_0005` requirement and `SVC_MCP_0005`. +- MCP clients consuming `get_requirement_status` / `get_requirements_status` — output schema and + some verdict values change (intentional). diff --git a/openspec/changes/consolidate-requirement-verdict/specs/mcp/spec.md b/openspec/changes/consolidate-requirement-verdict/specs/mcp/spec.md new file mode 100644 index 00000000..00fa7992 --- /dev/null +++ b/openspec/changes/consolidate-requirement-verdict/specs/mcp/spec.md @@ -0,0 +1,7 @@ +## ADDED Requirements + +### Requirement: MCP_0005 +The system SHALL implement MCP_0005. + +#### Scenario: SVC_MCP_0005 +The system SHALL pass SVC_MCP_0005. diff --git a/openspec/changes/consolidate-requirement-verdict/tasks.md b/openspec/changes/consolidate-requirement-verdict/tasks.md new file mode 100644 index 00000000..6a56a7c5 --- /dev/null +++ b/openspec/changes/consolidate-requirement-verdict/tasks.md @@ -0,0 +1,30 @@ +## 1. reqstool SSOT + +- [ ] 1.1 Add `MCP_0005` (per-requirement status tool verdict/shape consistency) to `docs/reqstool/requirements.yml` under the mcp capability block +- [ ] 1.2 Add `SVC_MCP_0005` (verifies `get_requirement_status`/`get_requirements_status` agree with `status`) to `docs/reqstool/software_verification_cases.yml` +- [ ] 1.3 Run `openspec validate consolidate-requirement-verdict --type change --strict` and confirm it passes + +## 2. Extract the shared verdict predicate + +- [ ] 2.1 Add a freestanding `compute_requirement_status(req, repo, *, include_post_build) -> RequirementStatus` that encapsulates the per-requirement implementation/automated/manual verdict logic currently in `StatisticsService._calculate_requirement_stats` +- [ ] 2.2 Refactor `StatisticsService._calculate_requirement_stats` to call the extracted predicate, keeping `_update_requirement_totals` (totals accumulation) in `StatisticsService` +- [ ] 2.3 Verify `status`/`report`/`export` output is unchanged (statistics unit tests pass; CLAUDE.md regression smoke diffs are byte-identical) + +## 3. Route MCP status tools through the predicate + +- [ ] 3.1 Rewrite `get_requirement_status` / `get_requirements_status_all` in `details.py` to call `compute_requirement_status` with `include_post_build=False` +- [ ] 3.2 Serialize the resulting `RequirementStatus` to the unified shape (`completed`, `implementation_type`, `automated_tests`/`manual_tests` with `total` and `not_applicable`), matching `to_status_dict()`'s per-requirement shape +- [ ] 3.3 Delete `_compute_meets` and `_build_automated_test_summary` from `details.py` +- [ ] 3.4 Add `@Requirements("MCP_0005")` to the implementing function(s) for the consolidated MCP status path + +## 4. Tests + +- [ ] 4.1 Add a test asserting `get_requirement_status` / `get_requirements_status` agree with `StatisticsService` per requirement on the `test_standard/baseline/ms-001` fixture, covering the previously divergent `REQ_ext002_300` +- [ ] 4.2 Add `@SVCs("SVC_MCP_0005")` to the test method from 4.1 +- [ ] 4.3 Update any existing tests asserting the old `meets_requirements` / flat `test_summary` MCP shape to the new shape + +## 5. Verification + +- [ ] 5.1 Run `hatch run dev:pytest --cov=reqstool` and `hatch run dev:flake8` +- [ ] 5.2 Run `reqstool status local -p docs/reqstool` (via `hatch run python src/reqstool/command.py`) and confirm 72/72 complete with `SVC_MCP_0005` covered +- [ ] 5.3 Run `openspec validate --all --strict` diff --git a/openspec/config.yaml b/openspec/config.yaml new file mode 100644 index 00000000..392946c6 --- /dev/null +++ b/openspec/config.yaml @@ -0,0 +1,20 @@ +schema: spec-driven + +# Project context (optional) +# This is shown to AI when creating artifacts. +# Add your tech stack, conventions, style guides, domain knowledge, etc. +# Example: +# context: | +# Tech stack: TypeScript, React, Node.js +# We use conventional commits +# Domain: e-commerce platform + +# Per-artifact rules (optional) +# Add custom rules for specific artifacts. +# Example: +# rules: +# proposal: +# - Keep proposals under 500 words +# - Always include a "Non-goals" section +# tasks: +# - Break tasks into chunks of max 2 hours From 6eeee17903ea1887e854f6b48e8dba977396f817 Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Mon, 22 Jun 2026 18:04:10 +0200 Subject: [PATCH 2/7] docs(openspec): fix issue ref and drop brittle count in verdict proposal Signed-off-by: Jimisola Laursen --- CLAUDE.md | 2 +- openspec/changes/consolidate-requirement-verdict/tasks.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8146b6a7..d1d4e93a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -161,7 +161,7 @@ Full rationale in `docs/DESIGN.md`. - **Cycle detection covers both chains**: `CircularImportError` for the import chain, `CircularImplementationError` for the implementation chain. - **FK constraints scope evidence from implementation children**: SVCs/MVRs/annotations referencing out-of-scope requirements are rejected by SQLite FK checks on insert — no explicit filtering needed. - **Test results need explicit scoping**: no FK (keyed by FQN), so a scope check is required when inserting test results from implementation children. -- **The requirement "complete" verdict has ONE source of truth**: `StatisticsService`'s per-requirement computation (producing `RequirementStatus`). Every consumer — `status`/`report`/`export`, the MCP tools (`get_status`, `get_requirement_status`, `get_requirements_status`), and LSP — MUST derive completeness from that same computation. Do NOT re-implement "is this requirement met / are its automated tests satisfied" anywhere else (e.g. a parallel helper in `common/queries/details.py`). Two parallel verdict paths silently drifted and caused bugs (#411, #412); a private re-derivation is the regression to guard against. When the verdict logic must be reused, extract/call the shared predicate — never copy its traversal. +- **The requirement "complete" verdict has ONE source of truth**: `StatisticsService`'s per-requirement computation (producing `RequirementStatus`). Every consumer — `status`/`report`/`export`, the MCP tools (`get_status`, `get_requirement_status`, `get_requirements_status`), and LSP — MUST derive completeness from that same computation. Do NOT re-implement "is this requirement met / are its automated tests satisfied" anywhere else (e.g. a parallel helper in `common/queries/details.py`). Two parallel verdict paths silently drifted and caused bug #411 (consolidation tracked in #412); a private re-derivation is the regression to guard against. When the verdict logic must be reused, extract/call the shared predicate — never copy its traversal. ## Key Conventions diff --git a/openspec/changes/consolidate-requirement-verdict/tasks.md b/openspec/changes/consolidate-requirement-verdict/tasks.md index 6a56a7c5..348e0444 100644 --- a/openspec/changes/consolidate-requirement-verdict/tasks.md +++ b/openspec/changes/consolidate-requirement-verdict/tasks.md @@ -26,5 +26,5 @@ ## 5. Verification - [ ] 5.1 Run `hatch run dev:pytest --cov=reqstool` and `hatch run dev:flake8` -- [ ] 5.2 Run `reqstool status local -p docs/reqstool` (via `hatch run python src/reqstool/command.py`) and confirm 72/72 complete with `SVC_MCP_0005` covered +- [ ] 5.2 Run `reqstool status local -p docs/reqstool` (via `hatch run python src/reqstool/command.py`) and confirm all requirements complete with `SVC_MCP_0005` covered - [ ] 5.3 Run `openspec validate --all --strict` From f1207702f4eddb7fb864c2369d2527b129f73996 Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Mon, 22 Jun 2026 18:39:00 +0200 Subject: [PATCH 3/7] docs(openspec): refine verdict consolidation design for repo-querying, shared serializer, flag parity, LSP constraint Signed-off-by: Jimisola Laursen --- .../consolidate-requirement-verdict/design.md | 80 +++++++++++++------ .../proposal.md | 25 ++++-- .../consolidate-requirement-verdict/tasks.md | 20 ++--- 3 files changed, 85 insertions(+), 40 deletions(-) diff --git a/openspec/changes/consolidate-requirement-verdict/design.md b/openspec/changes/consolidate-requirement-verdict/design.md index fd53b8d7..8907048e 100644 --- a/openspec/changes/consolidate-requirement-verdict/design.md +++ b/openspec/changes/consolidate-requirement-verdict/design.md @@ -22,51 +22,81 @@ The two are not equivalent, even after #411's patch: two per-requirement/scoped MCP tools. The constraint that matters: there is no backwards-compat requirement on MCP tool output, so the fix can change both the schema and the verdict values. +The goal is stronger than "make the two agree": there must be **one** place that computes the +"complete" verdict and **one** place that serializes it, so that the CLI (`status`), the MCP +tools, and any future LSP completion display all return identical results for identical input. + ## Goals / Non-Goals **Goals:** - One per-requirement verdict computation, called by both `StatisticsService` and `details.py`. -- MCP `get_requirement_status` / `get_requirements_status` report the same `completed` verdict and - the same output structure as the `status` command for the same input. +- One per-requirement serializer, called by both `StatisticsService.to_status_dict()` and the MCP + status tools — same verdict *and* same output shape, not two shapes that happen to match. +- `status` (CLI), the MCP status tools, and any future LSP completion display report the same + `completed` verdict and output structure for the same input, in **both** build-only and + `--with-post-tests` (post-build) modes. - Delete `_compute_meets` and `_build_automated_test_summary`. **Non-Goals:** - Changing the `status`, `report`, or `export` command behavior or output. - Changing `get_status` (already unified). - Adding new MCP tools or changing transports / dataset resolution. +- Implementing an LSP completion display now (none exists yet) — see the forward-constraint below. ## Decisions -### Extract a freestanding per-requirement predicate (not a wrapper) +### Extract a per-requirement predicate that queries the repository directly + +Introduce a single function `compute_requirement_status(req, repo, *, include_post_build) -> +RequirementStatus` that computes the verdict for one requirement by **querying the repository** +through its scoped, index-backed per-requirement getters (`get_svcs_for_req`, +`get_annotations_impls_for_req`, `get_annotations_tests_for_svc`, `get_test_results_for_svc`, +`get_effective_mvr_for_svc`). `StatisticsService._calculate_requirement_stats` calls it inside its +loop; `details.py` calls it per id. `StatisticsService` keeps owning global aggregation +(`_calculate_global_totals`) and totals accumulation (`_update_requirement_totals`), fed by the +`RequirementStatus` the predicate returns. + +- **Why query the repo rather than thread a pre-fetched data bundle**: the repository layer exists + precisely so business logic asks the database for what it needs. Passing the four bulk tables + (`get_all_svcs`, `get_annotations_impls`, `get_annotations_tests`, `get_automated_test_results`) + into the predicate would leak the repo's job onto every caller and couple the signature to + `StatisticsService`'s fetch strategy. +- **Why this is not a perf regression**: the per-req getters are backed by primary keys and FK + indexes (`schema.py`) on an in-memory SQLite database; total work across a `status` run is + comparable to today's four bulk `SELECT *` calls. If query volume ever matters at scale, the fix + is repository-level caching — a separate concern, not a reason to complicate this signature. +- **Trade-off**: requires untangling `_calculate_requirement_stats` from + `_update_requirement_totals`, which today run in the same pass. -Introduce a single function that computes a `RequirementStatus` for one requirement given the -repository and a `include_post_build` flag. `StatisticsService._calculate_requirement_stats` -calls it inside its loop and keeps owning totals accumulation; `details.py` calls it per id. +### One shared per-requirement serializer -- **Why over a thin wrapper** (have `details.py` build a `StatisticsService` and read - `.requirement_statistics[urn_id]`): a wrapper makes the two agree but keeps two algorithms; - it also couples the MCP query layer to `StatisticsService` internals. Extracting the predicate - yields one literal source of truth and cleanly separates "verdict for one requirement" from - "aggregate totals across all requirements." -- **Trade-off**: requires untangling `_calculate_requirement_stats` from - `_update_requirement_totals`, which today run in the same pass. This is the bulk of the work. +Extract `_requirement_to_dict(status: RequirementStatus) -> dict` (the per-requirement body of +`StatisticsService.to_status_dict()`, producing `completed`, `implementation_type`, +`automated_tests`/`manual_tests` with `total` and `not_applicable`). `to_status_dict()` calls it, +and the `details.py` MCP status functions call it. No mapping back to the old `meets_requirements` +/ flat `test_summary` dict. -### Emit the unified shape directly from the MCP tools +- **Why**: a unified verdict that is serialized two different ways still diverges from the + consumer's point of view — same value, different JSON — which is how the original drift began. + Sharing the serializer keeps all status surfaces on one schema by construction, not by + coincidence. Backwards compatibility is not required (project decision), so preserving the legacy + dict would only perpetuate a second shape. -The two `details.py` status functions serialize the `RequirementStatus` the same way -`StatisticsService.to_status_dict()` does per requirement (`completed`, `implementation_type`, -`automated_tests`/`manual_tests` with `total` and `not_applicable`). No mapping back to the old -`meets_requirements` / flat `test_summary` dict. +### Expose the post-build scoping flag on every status surface -- **Why**: backwards compatibility is not required (project decision), so preserving the legacy - dict would only perpetuate a second shape. A shared per-requirement serializer keeps all status - tools on one schema. +The predicate's `include_post_build` flag is plumbed through to the MCP tools as an optional +parameter (default `False`, matching the `status` default) so the MCP tools have parity with +`status --with-post-tests`. The same parameter is the contract for any future LSP completion +display. This closes the last verdict-divergence gap: CLI = MCP = LSP in **both** modes, not just +the default. -### Default MCP tools to build-phase-only scoping +### LSP is a forward-constraint, not implemented here -To match the `status` command default, the predicate is invoked with `include_post_build=False` -from the MCP tools. Optionally expose the flag as a tool parameter for parity with -`status --with-post-tests` (can be deferred). +There is no LSP completion display today (no verdict code in `src/reqstool/lsp/`). This change does +not add one. It does bind the future: when LSP gains a completion display it MUST call +`compute_requirement_status` + `_requirement_to_dict` (with the same `include_post_build` +contract) and MUST NOT re-derive the verdict. `MCP_0005` is worded to cover all status surfaces so +the third consumer cannot silently fork later. This mirrors the convention note in `CLAUDE.md`. ## Risks / Trade-offs diff --git a/openspec/changes/consolidate-requirement-verdict/proposal.md b/openspec/changes/consolidate-requirement-verdict/proposal.md index d20d73ff..0abd55f7 100644 --- a/openspec/changes/consolidate-requirement-verdict/proposal.md +++ b/openspec/changes/consolidate-requirement-verdict/proposal.md @@ -9,11 +9,23 @@ removes the second implementation so the two can never disagree again. ## What Changes -- Extract a single per-requirement verdict computation that produces one `RequirementStatus` - value, used by both `StatisticsService` (in its per-requirement loop) and the `details.py` - MCP status functions. +- Extract a single per-requirement verdict computation + (`compute_requirement_status(req, repo, *, include_post_build)`) that produces one + `RequirementStatus` value, used by both `StatisticsService` (in its per-requirement loop) and + the `details.py` MCP status functions. The predicate queries the repository through its scoped + per-req getters rather than reusing bulk fetches, keeping the repository layer as the data + boundary. +- Extract a single per-requirement serializer (`_requirement_to_dict`) called by both + `StatisticsService.to_status_dict()` and the MCP status functions, so all status surfaces share + one output shape by construction (not two shapes that happen to match). - Delete the parallel predicate from `details.py` (`_compute_meets`, `_build_automated_test_summary`). +- Expose `include_post_build` (default `False`) on the MCP `get_requirement_status` / + `get_requirements_status` tools, for parity with `status --with-post-tests`, so CLI and MCP + agree in both build-only and post-build modes. +- **Forward-constraint (LSP):** no LSP completion display exists today and none is added here, but + any future one MUST consume `compute_requirement_status` + `_requirement_to_dict` and MUST NOT + re-derive the verdict. `MCP_0005` is worded to cover all status surfaces (CLI, MCP, LSP). - **BREAKING (MCP output):** the MCP `get_requirement_status` and `get_requirements_status` tools emit the unified status shape directly (`completed`, `implementation_type`, `automated_tests`/`manual_tests` objects with `total` and `not_applicable`), replacing the @@ -30,9 +42,10 @@ removes the second implementation so the two can never disagree again. ### Modified Capabilities -- `mcp`: adds a requirement that the per-requirement MCP status tools report a completion - verdict and output structure identical to the `status` command, derived from a single shared - verdict computation. +- `mcp`: adds a requirement that all per-requirement status surfaces (the `status` CLI, the MCP + status tools, and any future LSP completion display) report an identical completion verdict and + output structure for the same input, derived from a single shared verdict computation and a + single shared serializer, in both build-only and post-build scoping modes. ## Impact diff --git a/openspec/changes/consolidate-requirement-verdict/tasks.md b/openspec/changes/consolidate-requirement-verdict/tasks.md index 348e0444..3bce327d 100644 --- a/openspec/changes/consolidate-requirement-verdict/tasks.md +++ b/openspec/changes/consolidate-requirement-verdict/tasks.md @@ -6,22 +6,24 @@ ## 2. Extract the shared verdict predicate -- [ ] 2.1 Add a freestanding `compute_requirement_status(req, repo, *, include_post_build) -> RequirementStatus` that encapsulates the per-requirement implementation/automated/manual verdict logic currently in `StatisticsService._calculate_requirement_stats` -- [ ] 2.2 Refactor `StatisticsService._calculate_requirement_stats` to call the extracted predicate, keeping `_update_requirement_totals` (totals accumulation) in `StatisticsService` +- [ ] 2.1 Add `compute_requirement_status(req, repo, *, include_post_build) -> RequirementStatus` that encapsulates the per-requirement implementation/automated/manual verdict logic currently in `StatisticsService._calculate_requirement_stats`. It MUST obtain its data by querying the repository through the scoped per-req getters (`get_svcs_for_req`, `get_annotations_impls_for_req`, `get_annotations_tests_for_svc`, `get_test_results_for_svc`, `get_effective_mvr_for_svc`) — do NOT thread pre-fetched bulk tables through the signature +- [ ] 2.2 Refactor `StatisticsService._calculate_requirement_stats` to call the extracted predicate, keeping `_calculate_global_totals` and `_update_requirement_totals` (global aggregation + totals accumulation) in `StatisticsService` - [ ] 2.3 Verify `status`/`report`/`export` output is unchanged (statistics unit tests pass; CLAUDE.md regression smoke diffs are byte-identical) -## 3. Route MCP status tools through the predicate +## 3. Share the serializer and route MCP status tools through the predicate -- [ ] 3.1 Rewrite `get_requirement_status` / `get_requirements_status_all` in `details.py` to call `compute_requirement_status` with `include_post_build=False` -- [ ] 3.2 Serialize the resulting `RequirementStatus` to the unified shape (`completed`, `implementation_type`, `automated_tests`/`manual_tests` with `total` and `not_applicable`), matching `to_status_dict()`'s per-requirement shape -- [ ] 3.3 Delete `_compute_meets` and `_build_automated_test_summary` from `details.py` -- [ ] 3.4 Add `@Requirements("MCP_0005")` to the implementing function(s) for the consolidated MCP status path +- [ ] 3.1 Extract `_requirement_to_dict(status: RequirementStatus) -> dict` (the per-requirement body of `to_status_dict()`: `completed`, `implementation_type`, `automated_tests`/`manual_tests` with `total` and `not_applicable`) and make `to_status_dict()` call it +- [ ] 3.2 Rewrite `get_requirement_status` / `get_requirements_status_all` in `details.py` to call `compute_requirement_status` and serialize via the shared `_requirement_to_dict` — same code, not a re-implemented matching shape +- [ ] 3.3 Expose `include_post_build` (default `False`) as an optional parameter on the MCP `get_requirement_status` / `get_requirements_status` tools, for parity with `status --with-post-tests` +- [ ] 3.4 Delete `_compute_meets` and `_build_automated_test_summary` from `details.py` +- [ ] 3.5 Add `@Requirements("MCP_0005")` to the implementing function(s) for the consolidated MCP status path ## 4. Tests - [ ] 4.1 Add a test asserting `get_requirement_status` / `get_requirements_status` agree with `StatisticsService` per requirement on the `test_standard/baseline/ms-001` fixture, covering the previously divergent `REQ_ext002_300` -- [ ] 4.2 Add `@SVCs("SVC_MCP_0005")` to the test method from 4.1 -- [ ] 4.3 Update any existing tests asserting the old `meets_requirements` / flat `test_summary` MCP shape to the new shape +- [ ] 4.2 Assert agreement in **both** modes: `include_post_build=False` (default) and `True` (parity with `status --with-post-tests`) +- [ ] 4.3 Add `@SVCs("SVC_MCP_0005")` to the test method from 4.1 +- [ ] 4.4 Update any existing tests asserting the old `meets_requirements` / flat `test_summary` MCP shape to the new shape ## 5. Verification From b6afaa71d85e735e82e83bb911b88c1de89a750a Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Mon, 22 Jun 2026 19:00:13 +0200 Subject: [PATCH 4/7] feat: consolidate requirement verdict computation across status and MCP Extracts compute_requirement_status() and the shared _requirement_to_dict() serializer so StatisticsService (status/report/export) and the MCP get_requirement_status/get_requirements_status tools derive the "complete" verdict from one predicate instead of two that had already drifted (#411). Deletes the duplicate _compute_meets/_build_automated_test_summary path in details.py, exposes include_post_build on the MCP tools for parity with `status --with-post-tests`, and adds MCP_0005/SVC_MCP_0005 plus an agreement test covering the previously divergent REQ_ext002_300. Closes #412 Signed-off-by: Jimisola Laursen --- docs/reqstool/requirements.yml | 6 + docs/reqstool/software_verification_cases.yml | 6 + .../consolidate-requirement-verdict/tasks.md | 36 +- src/reqstool/common/queries/details.py | 93 ++--- src/reqstool/mcp/server.py | 19 +- src/reqstool/services/statistics_service.py | 319 +++++++++--------- .../storage/requirements_repository.py | 7 + .../reqstool/mcp/test_mcp_integration.py | 13 +- .../reqstool/common/queries/test_details.py | 103 ++++-- 9 files changed, 298 insertions(+), 304 deletions(-) diff --git a/docs/reqstool/requirements.yml b/docs/reqstool/requirements.yml index 5f244c74..3c93f05d 100644 --- a/docs/reqstool/requirements.yml +++ b/docs/reqstool/requirements.yml @@ -242,6 +242,12 @@ requirements: description: The system shall report a clear, actionable error when the optional dependencies required for the MCP server are not installed. categories: ["reliability"] revision: "0.11.0" + - id: MCP_0005 + title: MCP status tool verdict/shape consistency + significance: shall + description: The system shall report an identical completion verdict and output structure for a given requirement across the status CLI command and the MCP get_requirement_status and get_requirements_status tools, in both build-only and post-build scoping modes, derived from a single shared verdict computation and a single shared serializer. + categories: ["functional-suitability"] + revision: "0.11.0" # --- data-sources capability (derived from openspec/specs/data-sources) --- - id: SOURCE_0001 diff --git a/docs/reqstool/software_verification_cases.yml b/docs/reqstool/software_verification_cases.yml index 5bb740ac..126ca68c 100644 --- a/docs/reqstool/software_verification_cases.yml +++ b/docs/reqstool/software_verification_cases.yml @@ -236,6 +236,12 @@ cases: description: "GIVEN the MCP dependencies are not installed WHEN the command runs THEN it reports how to install them and does not start the server" verification: automated-test revision: "0.11.0" + - id: SVC_MCP_0005 + requirement_ids: ["MCP_0005"] + title: "MCP status tools agree with status command" + description: "GIVEN the same dataset WHEN get_requirement_status and get_requirements_status are called THEN they report the same completed verdict and output structure as the status command for the same requirement, in both build-only and post-build modes" + verification: automated-test + revision: "0.11.0" # --- data-sources --- - id: SVC_SOURCE_0001 diff --git a/openspec/changes/consolidate-requirement-verdict/tasks.md b/openspec/changes/consolidate-requirement-verdict/tasks.md index 3bce327d..6d8b7908 100644 --- a/openspec/changes/consolidate-requirement-verdict/tasks.md +++ b/openspec/changes/consolidate-requirement-verdict/tasks.md @@ -1,32 +1,32 @@ ## 1. reqstool SSOT -- [ ] 1.1 Add `MCP_0005` (per-requirement status tool verdict/shape consistency) to `docs/reqstool/requirements.yml` under the mcp capability block -- [ ] 1.2 Add `SVC_MCP_0005` (verifies `get_requirement_status`/`get_requirements_status` agree with `status`) to `docs/reqstool/software_verification_cases.yml` -- [ ] 1.3 Run `openspec validate consolidate-requirement-verdict --type change --strict` and confirm it passes +- [x] 1.1 Add `MCP_0005` (per-requirement status tool verdict/shape consistency) to `docs/reqstool/requirements.yml` under the mcp capability block +- [x] 1.2 Add `SVC_MCP_0005` (verifies `get_requirement_status`/`get_requirements_status` agree with `status`) to `docs/reqstool/software_verification_cases.yml` +- [x] 1.3 Run `openspec validate consolidate-requirement-verdict --type change --strict` and confirm it passes ## 2. Extract the shared verdict predicate -- [ ] 2.1 Add `compute_requirement_status(req, repo, *, include_post_build) -> RequirementStatus` that encapsulates the per-requirement implementation/automated/manual verdict logic currently in `StatisticsService._calculate_requirement_stats`. It MUST obtain its data by querying the repository through the scoped per-req getters (`get_svcs_for_req`, `get_annotations_impls_for_req`, `get_annotations_tests_for_svc`, `get_test_results_for_svc`, `get_effective_mvr_for_svc`) — do NOT thread pre-fetched bulk tables through the signature -- [ ] 2.2 Refactor `StatisticsService._calculate_requirement_stats` to call the extracted predicate, keeping `_calculate_global_totals` and `_update_requirement_totals` (global aggregation + totals accumulation) in `StatisticsService` -- [ ] 2.3 Verify `status`/`report`/`export` output is unchanged (statistics unit tests pass; CLAUDE.md regression smoke diffs are byte-identical) +- [x] 2.1 Add `compute_requirement_status(req, repo, *, include_post_build) -> RequirementStatus` that encapsulates the per-requirement implementation/automated/manual verdict logic currently in `StatisticsService._calculate_requirement_stats`. It MUST obtain its data by querying the repository through the scoped per-req getters (`get_svcs_for_req`, `get_annotations_impls_for_req`, `get_annotations_tests_for_svc`, `get_test_results_for_svc`, `get_effective_mvr_for_svc`) — do NOT thread pre-fetched bulk tables through the signature +- [x] 2.2 Refactor `StatisticsService._calculate_requirement_stats` to call the extracted predicate, keeping `_calculate_global_totals` and `_update_requirement_totals` (global aggregation + totals accumulation) in `StatisticsService` +- [x] 2.3 Verify `status`/`report`/`export` output is unchanged (statistics unit tests pass; CLAUDE.md regression smoke diffs are byte-identical) ## 3. Share the serializer and route MCP status tools through the predicate -- [ ] 3.1 Extract `_requirement_to_dict(status: RequirementStatus) -> dict` (the per-requirement body of `to_status_dict()`: `completed`, `implementation_type`, `automated_tests`/`manual_tests` with `total` and `not_applicable`) and make `to_status_dict()` call it -- [ ] 3.2 Rewrite `get_requirement_status` / `get_requirements_status_all` in `details.py` to call `compute_requirement_status` and serialize via the shared `_requirement_to_dict` — same code, not a re-implemented matching shape -- [ ] 3.3 Expose `include_post_build` (default `False`) as an optional parameter on the MCP `get_requirement_status` / `get_requirements_status` tools, for parity with `status --with-post-tests` -- [ ] 3.4 Delete `_compute_meets` and `_build_automated_test_summary` from `details.py` -- [ ] 3.5 Add `@Requirements("MCP_0005")` to the implementing function(s) for the consolidated MCP status path +- [x] 3.1 Extract `_requirement_to_dict(status: RequirementStatus) -> dict` (the per-requirement body of `to_status_dict()`: `completed`, `implementation_type`, `automated_tests`/`manual_tests` with `total` and `not_applicable`) and make `to_status_dict()` call it +- [x] 3.2 Rewrite `get_requirement_status` / `get_requirements_status_all` in `details.py` to call `compute_requirement_status` and serialize via the shared `_requirement_to_dict` — same code, not a re-implemented matching shape +- [x] 3.3 Expose `include_post_build` (default `False`) as an optional parameter on the MCP `get_requirement_status` / `get_requirements_status` tools, for parity with `status --with-post-tests` +- [x] 3.4 Delete `_compute_meets` and `_build_automated_test_summary` from `details.py` +- [x] 3.5 Add `@Requirements("MCP_0005")` to the implementing function(s) for the consolidated MCP status path ## 4. Tests -- [ ] 4.1 Add a test asserting `get_requirement_status` / `get_requirements_status` agree with `StatisticsService` per requirement on the `test_standard/baseline/ms-001` fixture, covering the previously divergent `REQ_ext002_300` -- [ ] 4.2 Assert agreement in **both** modes: `include_post_build=False` (default) and `True` (parity with `status --with-post-tests`) -- [ ] 4.3 Add `@SVCs("SVC_MCP_0005")` to the test method from 4.1 -- [ ] 4.4 Update any existing tests asserting the old `meets_requirements` / flat `test_summary` MCP shape to the new shape +- [x] 4.1 Add a test asserting `get_requirement_status` / `get_requirements_status` agree with `StatisticsService` per requirement on the `test_standard/baseline/ms-001` fixture, covering the previously divergent `REQ_ext002_300` +- [x] 4.2 Assert agreement in **both** modes: `include_post_build=False` (default) and `True` (parity with `status --with-post-tests`) +- [x] 4.3 Add `@SVCs("SVC_MCP_0005")` to the test method from 4.1 +- [x] 4.4 Update any existing tests asserting the old `meets_requirements` / flat `test_summary` MCP shape to the new shape ## 5. Verification -- [ ] 5.1 Run `hatch run dev:pytest --cov=reqstool` and `hatch run dev:flake8` -- [ ] 5.2 Run `reqstool status local -p docs/reqstool` (via `hatch run python src/reqstool/command.py`) and confirm all requirements complete with `SVC_MCP_0005` covered -- [ ] 5.3 Run `openspec validate --all --strict` +- [x] 5.1 Run `hatch run dev:pytest --cov=reqstool` and `hatch run dev:flake8` +- [x] 5.2 Run `reqstool status local -p docs/reqstool` (via `hatch run python src/reqstool/command.py`) and confirm all requirements complete with `SVC_MCP_0005` covered +- [x] 5.3 Run `openspec validate --all --strict` diff --git a/src/reqstool/common/queries/details.py b/src/reqstool/common/queries/details.py index 97586074..b4e4d90c 100644 --- a/src/reqstool/common/queries/details.py +++ b/src/reqstool/common/queries/details.py @@ -1,68 +1,13 @@ # Copyright © LFV +from reqstool_python_decorators.decorators.decorators import Requirements + from reqstool.common.models.urn_id import UrnId -from reqstool.models.requirements import IMPLEMENTATION, NON_CODE_IMPLEMENTATIONS -from reqstool.models.svcs import EXPECTS_AUTOMATED_TESTS, SVCData +from reqstool.services.statistics_service import compute_requirement_status, _requirement_to_dict from reqstool.storage.requirements_repository import RequirementsRepository -def _compute_meets(req, repo: RequirementsRepository, svc_urn_ids: list, all_passing: bool) -> bool: - """Return whether a requirement is considered met by this lightweight check. - - For IN_CODE: requires at least one @Requirements annotation, all automated tests - passing, and no failing MVRs. - For non-code types: requires at least one SVC, all automated tests passing, and - no failing MVRs. - """ - if not svc_urn_ids: - return False - if req.implementation == IMPLEMENTATION.IN_CODE: - if not (len(repo.get_annotations_impls_for_req(req.id)) > 0 and all_passing): - return False - elif req.implementation in NON_CODE_IMPLEMENTATIONS: - if not all_passing: - return False - else: - raise ValueError(f"Unhandled IMPLEMENTATION value: {req.implementation}") - # Check MVR results using only the effective (latest) verdict per SVC - for svc_uid in svc_urn_ids: - effective = repo.get_effective_mvr_for_svc(svc_uid) - if effective is not None and not effective.passed: - return False - return True - - -def _build_automated_test_summary( - svc_urn_ids: list, all_svcs: dict[UrnId, SVCData], repo: RequirementsRepository -) -> tuple[dict, bool]: - """Build an automated-test summary across the given SVCs and whether they all pass. - - An SVC whose verification type expects automated tests but has zero recorded test - executions counts as missing. SVCs verified by other means (e.g. manual test) are only - counted if they happen to have automated test results attached; their absence is not a - gap here since they are verified via MVRs instead (checked separately by the caller). - A skipped test also means the requirement is not (yet) met. - - ``all_svcs`` is passed in (rather than fetched here) so callers iterating over many - requirements can fetch it once instead of re-querying per requirement. - """ - test_summary = {"passed": 0, "failed": 0, "skipped": 0, "missing": 0} - for svc_uid in svc_urn_ids: - svc = all_svcs.get(svc_uid) - results = repo.get_test_results_for_svc(svc_uid) - if not results: - if svc is not None and svc.verification in EXPECTS_AUTOMATED_TESTS: - test_summary["missing"] += 1 - continue - for t in results: - key = t.status.value - if key in test_summary: - test_summary[key] += 1 - all_passing = test_summary["failed"] == 0 and test_summary["missing"] == 0 and test_summary["skipped"] == 0 - return test_summary, all_passing - - def _svc_test_summary(svc_urn_id: UrnId, repo: RequirementsRepository) -> dict: test_results = repo.get_test_results_for_svc(svc_urn_id) return { @@ -248,42 +193,42 @@ def get_urn_details( } -def get_requirement_status(raw_id: str, repo: RequirementsRepository) -> dict | None: - """Lightweight status check — avoids the full detail lookup.""" +@Requirements("MCP_0005") +def get_requirement_status( + raw_id: str, repo: RequirementsRepository, *, include_post_build: bool = False +) -> dict | None: + """Status check for one requirement — delegates to the shared verdict computation + so this surface can never drift from `status`/`report`/`export`.""" initial_urn = repo.get_initial_urn() urn_id = UrnId.assure_urn_id(initial_urn, raw_id) req = repo.get_all_requirements().get(urn_id) if req is None: return None - svc_urn_ids = repo.get_svcs_for_req(req.id) - all_svcs = repo.get_all_svcs() - test_summary, all_passing = _build_automated_test_summary(svc_urn_ids, all_svcs, repo) + status = compute_requirement_status(req, repo, include_post_build=include_post_build) return { "id": req.id.id, "lifecycle_state": req.lifecycle.state.value, - "implementation": req.implementation.value, - "test_summary": test_summary, - "meets_requirements": _compute_meets(req, repo, svc_urn_ids, all_passing), + **_requirement_to_dict(status), } -def get_requirements_status_all(repo: RequirementsRepository, urn: str | None = None) -> list[dict]: - """Batch status for all requirements. Optionally scoped to a URN.""" +@Requirements("MCP_0005") +def get_requirements_status_all( + repo: RequirementsRepository, urn: str | None = None, *, include_post_build: bool = False +) -> list[dict]: + """Batch status for all requirements. Optionally scoped to a URN. Delegates to the + shared verdict computation so this surface can never drift from `status`/`report`/`export`.""" reqs = repo.get_all_requirements(urn=urn) - all_svcs = repo.get_all_svcs() result = [] for req in reqs.values(): - svc_urn_ids = repo.get_svcs_for_req(req.id) - test_summary, all_passing = _build_automated_test_summary(svc_urn_ids, all_svcs, repo) + status = compute_requirement_status(req, repo, include_post_build=include_post_build) result.append( { "id": req.id.id, "urn": req.id.urn, "lifecycle_state": req.lifecycle.state.value, - "implementation": req.implementation.value, - "test_summary": test_summary, - "meets_requirements": _compute_meets(req, repo, svc_urn_ids, all_passing), + **_requirement_to_dict(status), } ) return result diff --git a/src/reqstool/mcp/server.py b/src/reqstool/mcp/server.py index 6447c244..88ef18ba 100644 --- a/src/reqstool/mcp/server.py +++ b/src/reqstool/mcp/server.py @@ -66,11 +66,12 @@ def get_requirement(id: str) -> dict: return result @mcp.tool() - def get_requirements_status(urn: str | None = None) -> list[dict]: - """Batch status for all requirements: id, urn, lifecycle_state, implementation, test_summary, - meets_requirements. Use this to find requirements that are incomplete, partially tested, - or not yet implemented. Optionally filter by URN.""" - return _get_requirements_status_all(repo, urn=urn) + def get_requirements_status(urn: str | None = None, include_post_build: bool = False) -> list[dict]: + """Batch status for all requirements: id, urn, lifecycle_state, completed, implementation_type, + automated_tests, manual_tests. Use this to find requirements that are incomplete, partially + tested, or not yet implemented. Optionally filter by URN. Set include_post_build=True for + parity with `status --with-post-tests` (scopes to post-build-phase SVCs too).""" + return _get_requirements_status_all(repo, urn=urn, include_post_build=include_post_build) @mcp.tool() def list_svcs(urn: str | None = None, lifecycle_state: str | None = None) -> list[dict]: @@ -105,9 +106,11 @@ def get_status() -> dict: return StatisticsService(repo).to_status_dict() @mcp.tool() - def get_requirement_status(id: str) -> dict: - """Quick status check for one requirement: lifecycle state, implementation status, test summary.""" - result = _get_requirement_status(id, repo) + def get_requirement_status(id: str, include_post_build: bool = False) -> dict: + """Status check for one requirement: lifecycle_state, completed, implementation_type, + automated_tests, manual_tests. Set include_post_build=True for parity with + `status --with-post-tests` (scopes to post-build-phase SVCs too).""" + result = _get_requirement_status(id, repo, include_post_build=include_post_build) if result is None: raise ValueError(f"Requirement {id!r} not found") return result diff --git a/src/reqstool/services/statistics_service.py b/src/reqstool/services/statistics_service.py index 30b0e32a..80c7bacd 100644 --- a/src/reqstool/services/statistics_service.py +++ b/src/reqstool/services/statistics_service.py @@ -18,6 +18,8 @@ "TestStats", "RequirementStatus", "TotalStats", + "compute_requirement_status", + "_requirement_to_dict", ] @@ -96,6 +98,152 @@ def code_completed(self) -> int: return self.completed_requirements - self.non_code_completed +def compute_requirement_status(req, repo: RequirementsRepository, *, include_post_build: bool = False) -> RequirementStatus: + """Compute the single "is this requirement complete?" verdict for one requirement. + + Queries the repository through its scoped per-requirement getters so callers never + thread pre-fetched bulk tables through this signature. This is the one place the + completion verdict is computed; every status surface (CLI, MCP, future LSP) must call + this rather than re-deriving it. + """ + svcs_urn_ids = repo.get_svcs_for_req(req.id) + svcs = [s for s in (repo.get_svc(sid) for sid in svcs_urn_ids) if s is not None] + verdict_svcs = svcs if include_post_build else [s for s in svcs if s.phase == VERIFICATIONPHASE.BUILD] + verdict_svc_urn_ids = [s.id for s in verdict_svcs] + + should_have_mvrs = any(svc.verification in EXPECTS_MVRS for svc in verdict_svcs) + should_have_automated_tests = any(svc.verification in EXPECTS_AUTOMATED_TESTS for svc in verdict_svcs) + + nr_of_implementations = len(repo.get_annotations_impls_for_req(req.id)) + + mvr_stats = _compute_requirement_mvr_stats(repo, verdict_svc_urn_ids, verdict_svcs, should_have_mvrs) + automated_test_stats = _compute_requirement_automated_stats(repo, verdict_svcs, should_have_automated_tests) + + implementation_ok = _check_implementation( + urn_id=req.id, nr_of_implementations=nr_of_implementations, implementation=req.implementation + ) + + completed = ( + implementation_ok + and mvr_stats.is_completed() + and automated_test_stats.is_completed() + and (should_have_mvrs or should_have_automated_tests) + ) + + return RequirementStatus( + completed=completed, + implementations=nr_of_implementations, + implementation_type=req.implementation, + automated_tests=automated_test_stats, + manual_tests=mvr_stats, + ) + + +def _compute_requirement_mvr_stats(repo: RequirementsRepository, svcs_urn_ids, svcs, should_have_mvrs) -> TestStats: + if not should_have_mvrs: + return TestStats(not_applicable=True) + + total = 0 + passed = 0 + failed = 0 + missing = 0 + svc_map = {s.id: s for s in svcs} + for svc_uid in svcs_urn_ids: + svc = svc_map.get(svc_uid) + if svc is None or svc.verification not in EXPECTS_MVRS: + continue + effective = repo.get_effective_mvr_for_svc(svc_uid) + if effective is None: + missing += 1 + elif effective.passed: + total += 1 + passed += 1 + else: + total += 1 + failed += 1 + + return TestStats(total=total, passed=passed, failed=failed, missing=missing) + + +def _compute_requirement_automated_stats( + repo: RequirementsRepository, verdict_svcs, should_have_automated_tests +) -> TestStats: + if not should_have_automated_tests: + return TestStats(not_applicable=True) + + tests: list[TestData] = [] + for svc in verdict_svcs: + annotations = repo.get_annotations_tests_for_svc(svc.id) + if annotations: + tests.extend(repo.get_test_results_for_svc(svc.id)) + elif svc.verification in EXPECTS_AUTOMATED_TESTS: + tests.append(TestData(fully_qualified_name="", status=TEST_RUN_STATUS.MISSING)) + + return _compute_test_stats(tests=tests, svcs=verdict_svcs) + + +def _compute_test_stats(tests: list[TestData], svcs) -> TestStats: + if not tests: + no_of_missing = sum(1 for svc in svcs if svc.verification in EXPECTS_AUTOMATED_TESTS) + return TestStats(missing=no_of_missing) + + total = len(tests) + passed = 0 + failed = 0 + skipped = 0 + missing = 0 + + for test in tests: + if test.fully_qualified_name == "": + total -= 1 + match test.status: + case TEST_RUN_STATUS.PASSED: + passed += 1 + case TEST_RUN_STATUS.FAILED: + failed += 1 + case TEST_RUN_STATUS.SKIPPED: + skipped += 1 + case TEST_RUN_STATUS.MISSING: + missing += 1 + + return TestStats(total=total, passed=passed, failed=failed, skipped=skipped, missing=missing) + + +def _check_implementation(urn_id: UrnId, nr_of_implementations: int, implementation: IMPLEMENTATION) -> bool: + if implementation == IMPLEMENTATION.IN_CODE: + return nr_of_implementations > 0 + if implementation in NON_CODE_IMPLEMENTATIONS: + if nr_of_implementations > 0: + raise TypeError(f"Requirement {urn_id} should not have an implementation") + return True + raise ValueError(f"Unhandled IMPLEMENTATION value: {implementation}") + + +def _requirement_to_dict(status: RequirementStatus) -> dict: + """Serialize one requirement's verdict. The single shape shared by `status`/`report`/`export` and MCP.""" + return { + "completed": status.completed, + "implementations": status.implementations, + "implementation_type": status.implementation_type.value, + "automated_tests": { + "total": status.automated_tests.total, + "passed": status.automated_tests.passed, + "failed": status.automated_tests.failed, + "skipped": status.automated_tests.skipped, + "missing": status.automated_tests.missing, + "not_applicable": status.automated_tests.not_applicable, + }, + "manual_tests": { + "total": status.manual_tests.total, + "passed": status.manual_tests.passed, + "failed": status.manual_tests.failed, + "skipped": status.manual_tests.skipped, + "missing": status.manual_tests.missing, + "not_applicable": status.manual_tests.not_applicable, + }, + } + + @Requirements("STATUS_0001") class StatisticsService: def __init__(self, repository: RequirementsRepository, include_post_build: bool = False): @@ -120,16 +268,13 @@ def initial_urn(self) -> str: def _calculate(self): requirements = self._repo.get_all_requirements() all_svcs = self._repo.get_all_svcs() - annotations_impls = self._repo.get_annotations_impls() annotations_tests = self._repo.get_annotations_tests() automated_test_results = self._repo.get_automated_test_results() self._calculate_global_totals(all_svcs, annotations_tests, automated_test_results) for urn_id, req_data in requirements.items(): - self._calculate_requirement_stats( - urn_id, req_data, all_svcs, annotations_impls, annotations_tests, automated_test_results - ) + self._calculate_requirement_stats(urn_id, req_data) def _calculate_global_totals(self, all_svcs, annotations_tests, automated_test_results): self._totals.total_svcs = len(all_svcs) @@ -167,88 +312,13 @@ def _count_automated_test_totals(self, annotations_tests, automated_test_results case TEST_RUN_STATUS.MISSING: self._totals.total_tests -= 1 - def _calculate_requirement_stats( - self, urn_id, req_data, all_svcs, annotations_impls, annotations_tests, automated_test_results - ): - svcs_urn_ids = self._repo.get_svcs_for_req(urn_id) - svcs = [all_svcs[sid] for sid in svcs_urn_ids if sid in all_svcs] - verdict_svcs = svcs if self._include_post_build else [s for s in svcs if s.phase == VERIFICATIONPHASE.BUILD] - verdict_svc_urn_ids = [s.id for s in verdict_svcs] - - should_have_mvrs = any(svc.verification in EXPECTS_MVRS for svc in verdict_svcs) - should_have_automated_tests = any(svc.verification in EXPECTS_AUTOMATED_TESTS for svc in verdict_svcs) - - nr_of_implementations = len(annotations_impls.get(urn_id, [])) - - mvr_stats = self._get_requirement_mvr_stats(verdict_svc_urn_ids, verdict_svcs, should_have_mvrs) - automated_test_stats = self._get_requirement_automated_stats( - verdict_svc_urn_ids, - all_svcs, - annotations_tests, - automated_test_results, - verdict_svcs, - should_have_automated_tests, - ) - - implementation_ok = self._check_implementation( - urn_id=urn_id, nr_of_implementations=nr_of_implementations, implementation=req_data.implementation - ) - - completed = ( - implementation_ok - and mvr_stats.is_completed() - and automated_test_stats.is_completed() - and (should_have_mvrs or should_have_automated_tests) + def _calculate_requirement_stats(self, urn_id, req_data): + status = compute_requirement_status(req_data, self._repo, include_post_build=self._include_post_build) + self._requirement_stats[urn_id] = status + self._update_requirement_totals( + req_data, status.implementations, status.completed, status.automated_tests, status.manual_tests ) - self._requirement_stats[urn_id] = RequirementStatus( - completed=completed, - implementations=nr_of_implementations, - implementation_type=req_data.implementation, - automated_tests=automated_test_stats, - manual_tests=mvr_stats, - ) - - self._update_requirement_totals(req_data, nr_of_implementations, completed, automated_test_stats, mvr_stats) - - def _get_requirement_mvr_stats(self, svcs_urn_ids, svcs, should_have_mvrs) -> TestStats: - if not should_have_mvrs: - return TestStats(not_applicable=True) - - total = 0 - passed = 0 - failed = 0 - missing = 0 - svc_map = {s.id: s for s in svcs} - for svc_uid in svcs_urn_ids: - svc = svc_map.get(svc_uid) - if svc is None or svc.verification not in EXPECTS_MVRS: - continue - effective = self._repo.get_effective_mvr_for_svc(svc_uid) - if effective is None: - missing += 1 - elif effective.passed: - total += 1 - passed += 1 - else: - total += 1 - failed += 1 - - return TestStats(total=total, passed=passed, failed=failed, missing=missing) - - def _get_requirement_automated_stats( - self, svcs_urn_ids, all_svcs, annotations_tests, automated_test_results, svcs, should_have_automated_tests - ): - if not should_have_automated_tests: - return TestStats(not_applicable=True) - test_results_for_req = self._get_annotated_automated_test_results_for_req( - svcs_urn_ids=svcs_urn_ids, - all_svcs=all_svcs, - annotations_tests=annotations_tests, - automated_test_results=automated_test_results, - ) - return self._get_test_stats(tests=test_results_for_req, svcs=svcs) - def _update_requirement_totals(self, req_data, nr_of_implementations, completed, automated_test_stats, mvr_stats): self._totals.total_requirements += 1 @@ -285,90 +355,11 @@ def _update_requirement_totals(self, req_data, nr_of_implementations, completed, if not mvr_stats.not_applicable: self._totals.missing_manual_tests += mvr_stats.missing - def _check_implementation(self, urn_id: UrnId, nr_of_implementations: int, implementation: IMPLEMENTATION) -> bool: - if implementation == IMPLEMENTATION.IN_CODE: - return nr_of_implementations > 0 - if implementation in NON_CODE_IMPLEMENTATIONS: - if nr_of_implementations > 0: - raise TypeError(f"Requirement {urn_id} should not have an implementation") - return True - raise ValueError(f"Unhandled IMPLEMENTATION value: {implementation}") - - def _get_test_stats(self, tests: list[TestData], svcs) -> TestStats: - if not tests: - no_of_missing = sum(1 for svc in svcs if svc.verification in EXPECTS_AUTOMATED_TESTS) - return TestStats(missing=no_of_missing) - - total = len(tests) - passed = 0 - failed = 0 - skipped = 0 - missing = 0 - - for test in tests: - if test.fully_qualified_name == "": - total -= 1 - match test.status: - case TEST_RUN_STATUS.PASSED: - passed += 1 - case TEST_RUN_STATUS.FAILED: - failed += 1 - case TEST_RUN_STATUS.SKIPPED: - skipped += 1 - case TEST_RUN_STATUS.MISSING: - missing += 1 - - return TestStats(total=total, passed=passed, failed=failed, skipped=skipped, missing=missing) - - def _get_annotated_automated_test_results_for_req( - self, - svcs_urn_ids: list[UrnId], - all_svcs: dict, - annotations_tests: dict[UrnId, list], - automated_test_results: dict[UrnId, list[TestData]], - ) -> list[TestData]: - results: list[TestData] = [] - for svc_uid in svcs_urn_ids: - if svc_uid in annotations_tests: - for ann in annotations_tests[svc_uid]: - test_urn_id = UrnId(urn=svc_uid.urn, id=ann.fully_qualified_name) - if test_urn_id in automated_test_results: - results.extend(automated_test_results[test_urn_id]) - else: - results.append( - TestData(fully_qualified_name=ann.fully_qualified_name, status=TEST_RUN_STATUS.MISSING) - ) - elif svc_uid in all_svcs and all_svcs[svc_uid].verification in EXPECTS_AUTOMATED_TESTS: - results.append(TestData(fully_qualified_name="", status=TEST_RUN_STATUS.MISSING)) - return results - def to_status_dict(self) -> dict: initial_urn = self._repo.get_initial_urn() filtered = self._repo.is_filtered() - requirements = {} - for urn_id, status in self._requirement_stats.items(): - requirements[str(urn_id)] = { - "completed": status.completed, - "implementations": status.implementations, - "implementation_type": status.implementation_type.value, - "automated_tests": { - "total": status.automated_tests.total, - "passed": status.automated_tests.passed, - "failed": status.automated_tests.failed, - "skipped": status.automated_tests.skipped, - "missing": status.automated_tests.missing, - "not_applicable": status.automated_tests.not_applicable, - }, - "manual_tests": { - "total": status.manual_tests.total, - "passed": status.manual_tests.passed, - "failed": status.manual_tests.failed, - "skipped": status.manual_tests.skipped, - "missing": status.manual_tests.missing, - "not_applicable": status.manual_tests.not_applicable, - }, - } + requirements = {str(urn_id): _requirement_to_dict(status) for urn_id, status in self._requirement_stats.items()} ts = self._totals totals = { diff --git a/src/reqstool/storage/requirements_repository.py b/src/reqstool/storage/requirements_repository.py index 632482ce..aab908ad 100644 --- a/src/reqstool/storage/requirements_repository.py +++ b/src/reqstool/storage/requirements_repository.py @@ -128,6 +128,13 @@ def get_all_mvrs(self, urn: str | None = None, passed: bool | None = None) -> di # -- Index/lookup queries -- + def get_svc(self, svc_urn_id: UrnId) -> SVCData | None: + row = self._db.connection.execute( + "SELECT * FROM svcs WHERE urn = ? AND id = ?", + (svc_urn_id.urn, svc_urn_id.id), + ).fetchone() + return self._row_to_svc_data(row) if row else None + def get_svcs_for_req(self, req_urn_id: UrnId) -> list[UrnId]: rows = self._db.connection.execute( "SELECT svc_urn, svc_id FROM svc_requirement_links WHERE req_urn = ? AND req_id = ?", diff --git a/tests/integration/reqstool/mcp/test_mcp_integration.py b/tests/integration/reqstool/mcp/test_mcp_integration.py index 1263fe8d..d2b5dcbd 100644 --- a/tests/integration/reqstool/mcp/test_mcp_integration.py +++ b/tests/integration/reqstool/mcp/test_mcp_integration.py @@ -156,10 +156,11 @@ async def test_get_requirement_status(mcp_session): status = _parse_result(result) assert status["id"] == KNOWN_REQ_ID assert "lifecycle_state" in status - assert "implementation" in status - assert "test_summary" in status - assert "meets_requirements" in status - assert isinstance(status["meets_requirements"], bool) + assert "implementation_type" in status + assert "automated_tests" in status + assert "manual_tests" in status + assert "completed" in status + assert isinstance(status["completed"], bool) async def test_get_requirement_status_not_found(mcp_session): @@ -177,8 +178,8 @@ async def test_get_requirement_status_missing_automated_test_not_met(mcp_session """ result = await mcp_session.call_tool("get_requirement_status", {"id": "REQ_MISSING_TEST"}) status = _parse_result(result) - assert status["test_summary"]["missing"] >= 1 - assert status["meets_requirements"] is False + assert status["automated_tests"]["missing"] >= 1 + assert status["completed"] is False # --------------------------------------------------------------------------- diff --git a/tests/unit/reqstool/common/queries/test_details.py b/tests/unit/reqstool/common/queries/test_details.py index 1e09c52c..f9045a76 100644 --- a/tests/unit/reqstool/common/queries/test_details.py +++ b/tests/unit/reqstool/common/queries/test_details.py @@ -1,6 +1,7 @@ # Copyright © LFV import pytest +from reqstool_python_decorators.decorators.decorators import SVCs from reqstool.common.project_session import ProjectSession from reqstool.common.models.urn_id import UrnId @@ -20,6 +21,7 @@ ) from reqstool.models.svcs import SVCData, VERIFICATIONTYPES from reqstool.models.test_data import TEST_RUN_STATUS +from reqstool.services.statistics_service import StatisticsService from reqstool.storage.database import RequirementsDatabase from reqstool.storage.requirements_repository import RequirementsRepository from reqstool.locations.local_location import LocalLocation @@ -111,31 +113,60 @@ def test_get_requirement_status_known(session): assert result is not None assert result["id"] == "REQ_010" assert "lifecycle_state" in result - assert "implementation" in result - assert "test_summary" in result - assert set(result["test_summary"].keys()) == {"passed", "failed", "skipped", "missing"} - assert "meets_requirements" in result - assert isinstance(result["meets_requirements"], bool) + assert "implementation_type" in result + assert "automated_tests" in result + assert set(result["automated_tests"].keys()) == {"total", "passed", "failed", "skipped", "missing", "not_applicable"} + assert "manual_tests" in result + assert set(result["manual_tests"].keys()) == {"total", "passed", "failed", "skipped", "missing", "not_applicable"} + assert "completed" in result + assert isinstance(result["completed"], bool) def test_get_requirement_status_unknown(session): assert get_requirement_status("REQ_NONEXISTENT", session.repo) is None +@pytest.mark.parametrize("include_post_build", [False, True]) +@SVCs("SVC_MCP_0005") +def test_mcp_status_tools_agree_with_statistics_service(session, include_post_build): + """get_requirement_status / get_requirements_status must report the same verdict and + shape as StatisticsService for every requirement, in both build-only and post-build + scoping modes — the consolidation this change exists to guarantee (issue #412).""" + repo = session.repo + stats = StatisticsService(repo, include_post_build=include_post_build) + expected = stats.to_status_dict()["requirements"] + + all_results = {r["id"]: r for r in get_requirements_status_all(repo, include_post_build=include_post_build)} + + assert "REQ_ext002_300" in all_results, "fixture must cover the previously divergent requirement" + + for urn_id_str, expected_status in expected.items(): + # urn_id_str is the full "urn:id" form; pass it through unsplit so get_requirement_status + # resolves requirements that don't live in the project's initial urn (e.g. ext-002:*). + single_result = get_requirement_status(urn_id_str, repo, include_post_build=include_post_build) + bare_id = urn_id_str.rsplit(":", 1)[1] + + for key in ("completed", "implementation_type", "automated_tests", "manual_tests"): + assert single_result[key] == expected_status[key], f"{urn_id_str}: get_requirement_status[{key}] mismatch" + assert all_results[bare_id][key] == expected_status[key], ( + f"{urn_id_str}: get_requirements_status[{key}] mismatch" + ) + + def _make_db_with_req( impl_type, passed: bool | None = None, with_annotation: bool = False, - verification: VERIFICATIONTYPES = VERIFICATIONTYPES.MANUAL_TEST, + verification: VERIFICATIONTYPES = VERIFICATIONTYPES.AUTOMATED_TEST, with_test_annotation: bool = True, status: TEST_RUN_STATUS | None = None, ): """Build a minimal in-memory DB with one requirement + SVC (+ optional test annotation/result). - By default builds a manual-test SVC with a passing/failing automated test result (driven by - `passed`), matching the original fixture shape. Pass `verification`/`status` to instead build - an automated-test SVC with a specific test outcome, or `with_test_annotation=False` to build - an SVC with no test annotation/result at all (the "entirely missing automated test" case). + By default builds an automated-test SVC with a passing/failing automated test result (driven by + `passed`). Pass `verification`/`status` to build a SVC with a different verification type and/or + a specific test outcome, or `with_test_annotation=False` to build an SVC with no test + annotation/result at all (the "entirely missing automated test" case). """ db = RequirementsDatabase() db.set_metadata("initial_urn", "ms-001") @@ -172,8 +203,8 @@ def test_meets_requirements_in_code_with_annotation_and_passing_tests(): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["implementation"] == "in-code" - assert result["meets_requirements"] is True + assert result["implementation_type"] == "in-code" + assert result["completed"] is True db.close() @@ -182,7 +213,7 @@ def test_meets_requirements_in_code_without_annotation_is_false(): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is False + assert result["completed"] is False db.close() @@ -200,8 +231,8 @@ def test_meets_requirements_non_code_type_passing_tests_true(impl_type): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["implementation"] == impl_type.value - assert result["meets_requirements"] is True + assert result["implementation_type"] == impl_type.value + assert result["completed"] is True db.close() @@ -219,12 +250,12 @@ def test_meets_requirements_non_code_type_failing_tests_false(impl_type): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is False + assert result["completed"] is False db.close() def test_get_requirements_status_all_mixed_types(): - """get_requirements_status_all returns correct meets_requirements for each impl type.""" + """get_requirements_status_all returns correct completed verdict for each impl type.""" db = RequirementsDatabase() db.set_metadata("initial_urn", "ms-001") URN = "ms-001" @@ -248,7 +279,11 @@ def test_get_requirements_status_all_mixed_types(): for svc_id, req_id in [(svc_code, in_code_id), (svc_cfg, cfg_id)]: svc = SVCData( - id=svc_id, title="S", verification=VERIFICATIONTYPES.MANUAL_TEST, revision="1.0.0", requirement_ids=[req_id] + id=svc_id, + title="S", + verification=VERIFICATIONTYPES.AUTOMATED_TEST, + revision="1.0.0", + requirement_ids=[req_id], ) db.insert_svc(svc_id.urn, svc) ann = AnnotationData(element_kind="METHOD", fully_qualified_name=f"test_{svc_id.id}") @@ -261,8 +296,8 @@ def test_get_requirements_status_all_mixed_types(): repo = RequirementsRepository(db) results = {r["id"]: r for r in get_requirements_status_all(repo)} - assert results["REQ_CODE"]["meets_requirements"] is True - assert results["REQ_CFG"]["meets_requirements"] is True + assert results["REQ_CODE"]["completed"] is True + assert results["REQ_CFG"]["completed"] is True db.close() @@ -281,7 +316,7 @@ def test_get_requirements_status_all_in_code_without_annotation_false(): ) svc_id = UrnId(urn="ms-001", id="SVC_T") svc = SVCData( - id=svc_id, title="S", verification=VERIFICATIONTYPES.MANUAL_TEST, revision="1.0.0", requirement_ids=[req_id] + id=svc_id, title="S", verification=VERIFICATIONTYPES.AUTOMATED_TEST, revision="1.0.0", requirement_ids=[req_id] ) ann = AnnotationData(element_kind="METHOD", fully_qualified_name="test_m") db.insert_requirement(req_id.urn, req) @@ -292,7 +327,7 @@ def test_get_requirements_status_all_in_code_without_annotation_false(): repo = RequirementsRepository(db) results = {r["id"]: r for r in get_requirements_status_all(repo)} - assert results["REQ_NO_ANN"]["meets_requirements"] is False + assert results["REQ_NO_ANN"]["completed"] is False db.close() @@ -370,7 +405,7 @@ def test_meets_requirements_non_code_failing_mvr_is_false(impl_type): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is False, f"{impl_type}: failing MVR should make meets_requirements False" + assert result["completed"] is False, f"{impl_type}: failing MVR should make completed False" db.close() @@ -389,7 +424,7 @@ def test_meets_requirements_non_code_passing_mvr_is_true(impl_type): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is True + assert result["completed"] is True db.close() @@ -424,7 +459,7 @@ def test_meets_requirements_in_code_failing_mvr_is_false(): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is False, "failing MVR should make IN_CODE meets_requirements False" + assert result["completed"] is False, "failing MVR should make IN_CODE completed False" db.close() @@ -442,8 +477,8 @@ def test_meets_requirements_automated_skipped_test_is_false(): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["test_summary"]["skipped"] == 1 - assert result["meets_requirements"] is False, "a skipped automated test should make meets_requirements False" + assert result["automated_tests"]["skipped"] == 1 + assert result["completed"] is False, "a skipped automated test should make completed False" db.close() @@ -458,8 +493,8 @@ def test_meets_requirements_automated_zero_test_results_is_false(): repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["test_summary"]["missing"] == 1 - assert result["meets_requirements"] is False, "zero automated test executions should make meets_requirements False" + assert result["automated_tests"]["missing"] == 1 + assert result["completed"] is False, "zero automated test executions should make completed False" db.close() @@ -473,7 +508,7 @@ def test_get_requirements_status_all_automated_skipped_and_missing(): ) repo = RequirementsRepository(db) results = {r["id"]: r for r in get_requirements_status_all(repo)} - assert results[req_id.id]["meets_requirements"] is False + assert results[req_id.id]["completed"] is False db.close() @@ -528,22 +563,22 @@ def _make_db_with_superseded_mvrs(mvr_pass_sequence: list[tuple[str, bool]]) -> def test_compute_meets_superseded_fail_latest_pass_is_true(): - """fail→pass: latest (passing) MVR makes _compute_meets True.""" + """fail→pass: latest (passing) MVR makes the verdict completed.""" db, req_id, _ = _make_db_with_superseded_mvrs([("2026-01-01T00:00:00Z", False), ("2026-01-02T00:00:00Z", True)]) repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is True + assert result["completed"] is True db.close() def test_compute_meets_superseded_pass_latest_fail_is_false(): - """pass→fail: latest (failing) MVR makes _compute_meets False.""" + """pass→fail: latest (failing) MVR makes the verdict not completed.""" db, req_id, _ = _make_db_with_superseded_mvrs([("2026-01-01T00:00:00Z", True), ("2026-01-02T00:00:00Z", False)]) repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) assert result is not None - assert result["meets_requirements"] is False + assert result["completed"] is False db.close() From 7474dc0280724f5f5e41fe2db3b1a6b499139ace Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Tue, 23 Jun 2026 22:38:07 +0200 Subject: [PATCH 5/7] style: apply black formatting Signed-off-by: Jimisola Laursen --- src/reqstool/services/statistics_service.py | 4 +++- .../unit/reqstool/common/queries/test_details.py | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/reqstool/services/statistics_service.py b/src/reqstool/services/statistics_service.py index 80c7bacd..c16928c2 100644 --- a/src/reqstool/services/statistics_service.py +++ b/src/reqstool/services/statistics_service.py @@ -98,7 +98,9 @@ def code_completed(self) -> int: return self.completed_requirements - self.non_code_completed -def compute_requirement_status(req, repo: RequirementsRepository, *, include_post_build: bool = False) -> RequirementStatus: +def compute_requirement_status( + req, repo: RequirementsRepository, *, include_post_build: bool = False +) -> RequirementStatus: """Compute the single "is this requirement complete?" verdict for one requirement. Queries the repository through its scoped per-requirement getters so callers never diff --git a/tests/unit/reqstool/common/queries/test_details.py b/tests/unit/reqstool/common/queries/test_details.py index f9045a76..8a4c6636 100644 --- a/tests/unit/reqstool/common/queries/test_details.py +++ b/tests/unit/reqstool/common/queries/test_details.py @@ -115,7 +115,14 @@ def test_get_requirement_status_known(session): assert "lifecycle_state" in result assert "implementation_type" in result assert "automated_tests" in result - assert set(result["automated_tests"].keys()) == {"total", "passed", "failed", "skipped", "missing", "not_applicable"} + assert set(result["automated_tests"].keys()) == { + "total", + "passed", + "failed", + "skipped", + "missing", + "not_applicable", + } assert "manual_tests" in result assert set(result["manual_tests"].keys()) == {"total", "passed", "failed", "skipped", "missing", "not_applicable"} assert "completed" in result @@ -148,9 +155,9 @@ def test_mcp_status_tools_agree_with_statistics_service(session, include_post_bu for key in ("completed", "implementation_type", "automated_tests", "manual_tests"): assert single_result[key] == expected_status[key], f"{urn_id_str}: get_requirement_status[{key}] mismatch" - assert all_results[bare_id][key] == expected_status[key], ( - f"{urn_id_str}: get_requirements_status[{key}] mismatch" - ) + assert ( + all_results[bare_id][key] == expected_status[key] + ), f"{urn_id_str}: get_requirements_status[{key}] mismatch" def _make_db_with_req( From e1a0145283162b26c284dde1b6d9573e12756fa8 Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Tue, 23 Jun 2026 22:53:46 +0200 Subject: [PATCH 6/7] fix: address full-PR-review findings on requirement-verdict consolidation - docs/modules/ROOT/pages/mcp.adoc: update stale MCP status tool docs to the new shape (completed/implementation_type/automated_tests/manual_tests) and document include_post_build, replacing the deleted meets_requirements/ test_summary/implementation shape. - Rename _requirement_to_dict -> requirement_to_dict: it's a deliberate shared serializer imported across module boundaries, not module-private. - requirements_repository.py: extract get_test_results_for_annotations so _compute_requirement_automated_stats reuses the annotation list it already fetched instead of re-querying it inside get_test_results_for_svc. - Type the req parameter on compute_requirement_status. - Add direct unit tests for compute_requirement_status (not_applicable, missing MVR/test, completed, and the TypeError guard), for the new RequirementsRepository.get_svc() getter (found/not-found), and a dedicated test proving include_post_build actually changes the verdict (the existing fixture never varies SVC phase, so the prior agreement test alone couldn't prove this). - Strengthen the cross-path agreement test with known-correct expected values for REQ_ext002_300, not just cross-path equality. - Rename test_meets_requirements_*/test_compute_meets_* to test_completed_* to match the new vocabulary, and document which gate (automated_tests vs manual_tests) each _make_db_with_req-based test exercises. - Add a negative-path test for get_requirement_status with an invalid id combined with include_post_build. Addresses findings from the automated full-PR-review on #414. Signed-off-by: Jimisola Laursen --- docs/modules/ROOT/pages/mcp.adoc | 18 ++-- src/reqstool/common/queries/details.py | 6 +- src/reqstool/services/statistics_service.py | 12 +-- .../storage/requirements_repository.py | 10 +- .../reqstool/mcp/test_mcp_integration.py | 9 ++ .../reqstool/common/queries/test_details.py | 97 ++++++++++++++++--- .../services/test_statistics_service.py | 81 +++++++++++++++- .../storage/test_requirements_repository.py | 20 ++++ 8 files changed, 219 insertions(+), 34 deletions(-) diff --git a/docs/modules/ROOT/pages/mcp.adoc b/docs/modules/ROOT/pages/mcp.adoc index 57cc682f..e401cf1e 100644 --- a/docs/modules/ROOT/pages/mcp.adoc +++ b/docs/modules/ROOT/pages/mcp.adoc @@ -168,13 +168,16 @@ Overall traceability status across all requirements — completion counts, test ==== `get_requirement_status` -Quick status for a single requirement. +Status for a single requirement, derived from the same verdict computation as the `status` CLI +command — `get_status`, `get_requirement_status`, and `get_requirements_status` always agree. *Parameters:* * `id` _(string, required)_ +* `include_post_build` _(boolean, optional, default `false`)_ — also scope to post-build-phase + SVCs, for parity with `status --with-post-tests` -*Returns:* `{ id, lifecycle_state, implementation, test_summary: {passed, failed, skipped, missing}, meets_requirements }` +*Returns:* `{ id, lifecycle_state, completed, implementations, implementation_type, automated_tests: {total, passed, failed, skipped, missing, not_applicable}, manual_tests: {total, passed, failed, skipped, missing, not_applicable} }` ==== `get_requirements_status` @@ -185,13 +188,14 @@ requirements without N+1 individual calls. *Parameters:* * `urn` _(string, optional)_ — scope to a single project node +* `include_post_build` _(boolean, optional, default `false`)_ — same as `get_requirement_status` -*Returns:* array of `{ id, urn, lifecycle_state, implementation, test_summary, meets_requirements }` +*Returns:* array of `{ id, urn, lifecycle_state, completed, implementations, implementation_type, automated_tests, manual_tests }` == Example: Finding Incomplete Requirements The following client-side filter finds requirements that have started but are not yet done — they -have an implementation and at least one passing test, but `meets_requirements` is still false +have an implementation and at least one passing automated test, but are still not `completed` (e.g. due to missing or failing tests for other SVCs): [source,python] @@ -200,9 +204,9 @@ statuses = get_requirements_status() in_progress = [ r for r in statuses - if not r["meets_requirements"] - and r["implementation"] != "not_implemented" - and r["test_summary"]["passed"] > 0 + if not r["completed"] + and r["implementation_type"] != "N/A" + and r["automated_tests"]["passed"] > 0 ] ---- diff --git a/src/reqstool/common/queries/details.py b/src/reqstool/common/queries/details.py index b4e4d90c..6c2e7778 100644 --- a/src/reqstool/common/queries/details.py +++ b/src/reqstool/common/queries/details.py @@ -4,7 +4,7 @@ from reqstool_python_decorators.decorators.decorators import Requirements from reqstool.common.models.urn_id import UrnId -from reqstool.services.statistics_service import compute_requirement_status, _requirement_to_dict +from reqstool.services.statistics_service import compute_requirement_status, requirement_to_dict from reqstool.storage.requirements_repository import RequirementsRepository @@ -209,7 +209,7 @@ def get_requirement_status( return { "id": req.id.id, "lifecycle_state": req.lifecycle.state.value, - **_requirement_to_dict(status), + **requirement_to_dict(status), } @@ -228,7 +228,7 @@ def get_requirements_status_all( "id": req.id.id, "urn": req.id.urn, "lifecycle_state": req.lifecycle.state.value, - **_requirement_to_dict(status), + **requirement_to_dict(status), } ) return result diff --git a/src/reqstool/services/statistics_service.py b/src/reqstool/services/statistics_service.py index c16928c2..ee61718d 100644 --- a/src/reqstool/services/statistics_service.py +++ b/src/reqstool/services/statistics_service.py @@ -6,7 +6,7 @@ from reqstool_python_decorators.decorators.decorators import Requirements from reqstool.common.models.urn_id import UrnId -from reqstool.models.requirements import IMPLEMENTATION, NON_CODE_IMPLEMENTATIONS +from reqstool.models.requirements import IMPLEMENTATION, NON_CODE_IMPLEMENTATIONS, RequirementData from reqstool.models.svcs import EXPECTS_AUTOMATED_TESTS, EXPECTS_MVRS, VERIFICATIONPHASE from reqstool.models.test_data import TEST_RUN_STATUS, TestData from reqstool.storage.requirements_repository import RequirementsRepository @@ -19,7 +19,7 @@ "RequirementStatus", "TotalStats", "compute_requirement_status", - "_requirement_to_dict", + "requirement_to_dict", ] @@ -99,7 +99,7 @@ def code_completed(self) -> int: def compute_requirement_status( - req, repo: RequirementsRepository, *, include_post_build: bool = False + req: RequirementData, repo: RequirementsRepository, *, include_post_build: bool = False ) -> RequirementStatus: """Compute the single "is this requirement complete?" verdict for one requirement. @@ -177,7 +177,7 @@ def _compute_requirement_automated_stats( for svc in verdict_svcs: annotations = repo.get_annotations_tests_for_svc(svc.id) if annotations: - tests.extend(repo.get_test_results_for_svc(svc.id)) + tests.extend(repo.get_test_results_for_annotations(svc.id.urn, annotations)) elif svc.verification in EXPECTS_AUTOMATED_TESTS: tests.append(TestData(fully_qualified_name="", status=TEST_RUN_STATUS.MISSING)) @@ -221,7 +221,7 @@ def _check_implementation(urn_id: UrnId, nr_of_implementations: int, implementat raise ValueError(f"Unhandled IMPLEMENTATION value: {implementation}") -def _requirement_to_dict(status: RequirementStatus) -> dict: +def requirement_to_dict(status: RequirementStatus) -> dict: """Serialize one requirement's verdict. The single shape shared by `status`/`report`/`export` and MCP.""" return { "completed": status.completed, @@ -361,7 +361,7 @@ def to_status_dict(self) -> dict: initial_urn = self._repo.get_initial_urn() filtered = self._repo.is_filtered() - requirements = {str(urn_id): _requirement_to_dict(status) for urn_id, status in self._requirement_stats.items()} + requirements = {str(urn_id): requirement_to_dict(status) for urn_id, status in self._requirement_stats.items()} ts = self._totals totals = { diff --git a/src/reqstool/storage/requirements_repository.py b/src/reqstool/storage/requirements_repository.py index aab908ad..74ac5bc8 100644 --- a/src/reqstool/storage/requirements_repository.py +++ b/src/reqstool/storage/requirements_repository.py @@ -265,10 +265,18 @@ def get_annotations_tests_for_svc(self, svc_urn_id: UrnId) -> list[AnnotationDat def get_test_results_for_svc(self, svc_urn_id: UrnId) -> list[TestData]: """Return test results for each annotation attached to the given SVC.""" annotations = self.get_annotations_tests_for_svc(svc_urn_id) + return self.get_test_results_for_annotations(svc_urn_id.urn, annotations) + + def get_test_results_for_annotations(self, urn: str, annotations: list[AnnotationData]) -> list[TestData]: + """Resolve test results for an already-fetched list of test annotations. + + Lets callers that already hold the annotation list (e.g. to check for emptiness) + avoid re-querying `annotations_tests` via `get_test_results_for_svc`. + """ results = [] for ann in annotations: if ann.element_kind == "CLASS": - results.append(self._process_class_annotated_test_results(svc_urn_id.urn, ann.fully_qualified_name)) + results.append(self._process_class_annotated_test_results(urn, ann.fully_qualified_name)) else: row = self._db.connection.execute( "SELECT fqn, status FROM test_results WHERE fqn = ?", diff --git a/tests/integration/reqstool/mcp/test_mcp_integration.py b/tests/integration/reqstool/mcp/test_mcp_integration.py index d2b5dcbd..0841e349 100644 --- a/tests/integration/reqstool/mcp/test_mcp_integration.py +++ b/tests/integration/reqstool/mcp/test_mcp_integration.py @@ -168,6 +168,15 @@ async def test_get_requirement_status_not_found(mcp_session): assert result.isError +@pytest.mark.parametrize("include_post_build", [False, True]) +async def test_get_requirement_status_not_found_with_include_post_build(mcp_session, include_post_build): + """The not-found path must be unaffected by the include_post_build parameter combination.""" + result = await mcp_session.call_tool( + "get_requirement_status", {"id": "REQ_NONEXISTENT", "include_post_build": include_post_build} + ) + assert result.isError + + async def test_get_requirement_status_missing_automated_test_not_met(mcp_session): """An entirely missing automated test must not be reported as meeting requirements (regression for #410). diff --git a/tests/unit/reqstool/common/queries/test_details.py b/tests/unit/reqstool/common/queries/test_details.py index 8a4c6636..101009d0 100644 --- a/tests/unit/reqstool/common/queries/test_details.py +++ b/tests/unit/reqstool/common/queries/test_details.py @@ -19,7 +19,7 @@ SIGNIFICANCETYPES, RequirementData, ) -from reqstool.models.svcs import SVCData, VERIFICATIONTYPES +from reqstool.models.svcs import SVCData, VERIFICATIONPHASE, VERIFICATIONTYPES from reqstool.models.test_data import TEST_RUN_STATUS from reqstool.services.statistics_service import StatisticsService from reqstool.storage.database import RequirementsDatabase @@ -146,6 +146,13 @@ def test_mcp_status_tools_agree_with_statistics_service(session, include_post_bu all_results = {r["id"]: r for r in get_requirements_status_all(repo, include_post_build=include_post_build)} assert "REQ_ext002_300" in all_results, "fixture must cover the previously divergent requirement" + # Known-correct values, not just cross-path agreement: REQ_ext002_300 is verified only by + # SVC_025 ("SVC with missing automated-test", verification automated-test, no test result), + # so it must be reported as incomplete with exactly one missing automated test — pinning this + # guards against both call paths silently sharing the same wrong answer. + assert all_results["REQ_ext002_300"]["completed"] is False + assert all_results["REQ_ext002_300"]["automated_tests"]["missing"] == 1 + assert all_results["REQ_ext002_300"]["automated_tests"]["not_applicable"] is False for urn_id_str, expected_status in expected.items(): # urn_id_str is the full "urn:id" form; pass it through unsplit so get_requirement_status @@ -160,6 +167,57 @@ def test_mcp_status_tools_agree_with_statistics_service(session, include_post_bu ), f"{urn_id_str}: get_requirements_status[{key}] mismatch" +def test_get_requirement_status_include_post_build_changes_verdict(): + """include_post_build must actually change scoping, not just agree-with-itself. + + A requirement verified only by a post-build SVC with a passing test is non-applicable + (incomplete) by default, and completed once include_post_build=True pulls that SVC into + scope — proving the parameter has a real effect, not just consistent ignoring of it. + """ + db = RequirementsDatabase() + db.set_metadata("initial_urn", "ms-001") + req_id = UrnId(urn="ms-001", id="REQ_PB") + svc_id = UrnId(urn="ms-001", id="SVC_PB") + req = RequirementData( + id=req_id, + title="T", + significance=SIGNIFICANCETYPES.SHALL, + description="D", + implementation=IMPLEMENTATION.IN_CODE, + categories=[CATEGORIES.FUNCTIONAL_SUITABILITY], + revision="1.0.0", + ) + svc = SVCData( + id=svc_id, + title="S", + verification=VERIFICATIONTYPES.AUTOMATED_TEST, + phase=VERIFICATIONPHASE.POST_BUILD, + revision="1.0.0", + requirement_ids=[req_id], + ) + db.insert_requirement(req_id.urn, req) + db.insert_svc(svc_id.urn, svc) + db.insert_annotation_impl(req_id, AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.Foo.bar")) + ann = AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.E2ETest.testFlow") + db.insert_annotation_test(svc_id, ann) + db.insert_test_result("ms-001", "com.example.E2ETest.testFlow", TEST_RUN_STATUS.PASSED) + db.commit() + + repo = RequirementsRepository(db) + + default_result = get_requirement_status(req_id.id, repo) + assert default_result["completed"] is False + assert default_result["automated_tests"]["not_applicable"] is True + + post_build_result = get_requirement_status(req_id.id, repo, include_post_build=True) + assert post_build_result["completed"] is True + assert post_build_result["automated_tests"]["passed"] == 1 + + all_results = {r["id"]: r for r in get_requirements_status_all(repo, include_post_build=True)} + assert all_results["REQ_PB"]["completed"] is True + db.close() + + def _make_db_with_req( impl_type, passed: bool | None = None, @@ -170,10 +228,12 @@ def _make_db_with_req( ): """Build a minimal in-memory DB with one requirement + SVC (+ optional test annotation/result). - By default builds an automated-test SVC with a passing/failing automated test result (driven by - `passed`). Pass `verification`/`status` to build a SVC with a different verification type and/or - a specific test outcome, or `with_test_annotation=False` to build an SVC with no test - annotation/result at all (the "entirely missing automated test" case). + By default builds an automated-test SVC, so callers exercise the `automated_tests` + (`should_have_automated_tests`) gate driven by `passed`. Pass `verification=MANUAL_TEST` + to exercise the `manual_tests` (`should_have_mvrs`) gate instead — see `_make_db_with_mvr` + below for the dedicated MVR-gate fixture. Pass `status` for a specific test outcome, or + `with_test_annotation=False` to build an SVC with no test annotation/result at all (the + "entirely missing automated test" case). """ db = RequirementsDatabase() db.set_metadata("initial_urn", "ms-001") @@ -205,7 +265,8 @@ def _make_db_with_req( return db, req_id -def test_meets_requirements_in_code_with_annotation_and_passing_tests(): +def test_completed_in_code_with_annotation_and_passing_tests(): + """Exercises the automated_tests gate (default AUTOMATED_TEST verification).""" db, req_id = _make_db_with_req(IMPLEMENTATION.IN_CODE, passed=True, with_annotation=True) repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) @@ -215,7 +276,7 @@ def test_meets_requirements_in_code_with_annotation_and_passing_tests(): db.close() -def test_meets_requirements_in_code_without_annotation_is_false(): +def test_completed_in_code_without_annotation_is_false(): db, req_id = _make_db_with_req(IMPLEMENTATION.IN_CODE, passed=True, with_annotation=False) repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) @@ -233,7 +294,9 @@ def test_meets_requirements_in_code_without_annotation_is_false(): IMPLEMENTATION.FRAMEWORK, ], ) -def test_meets_requirements_non_code_type_passing_tests_true(impl_type): +def test_completed_non_code_type_passing_tests_true(impl_type): + """Exercises the automated_tests gate (default AUTOMATED_TEST verification); see + test_completed_non_code_passing_mvr_is_true for the equivalent manual_tests-gate case.""" db, req_id = _make_db_with_req(impl_type, passed=True, with_annotation=False) repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) @@ -252,7 +315,9 @@ def test_meets_requirements_non_code_type_passing_tests_true(impl_type): IMPLEMENTATION.FRAMEWORK, ], ) -def test_meets_requirements_non_code_type_failing_tests_false(impl_type): +def test_completed_non_code_type_failing_tests_false(impl_type): + """Exercises the automated_tests gate (default AUTOMATED_TEST verification); see + test_completed_non_code_failing_mvr_is_false for the equivalent manual_tests-gate case.""" db, req_id = _make_db_with_req(impl_type, passed=False, with_annotation=False) repo = RequirementsRepository(db) result = get_requirement_status(req_id.id, repo) @@ -406,7 +471,7 @@ def _make_db_with_mvr(impl_type, mvr_passed: bool): IMPLEMENTATION.FRAMEWORK, ], ) -def test_meets_requirements_non_code_failing_mvr_is_false(impl_type): +def test_completed_non_code_failing_mvr_is_false(impl_type): """Non-code req with a failing MVR must not be considered met.""" db, req_id = _make_db_with_mvr(impl_type, mvr_passed=False) repo = RequirementsRepository(db) @@ -425,7 +490,7 @@ def test_meets_requirements_non_code_failing_mvr_is_false(impl_type): IMPLEMENTATION.FRAMEWORK, ], ) -def test_meets_requirements_non_code_passing_mvr_is_true(impl_type): +def test_completed_non_code_passing_mvr_is_true(impl_type): """Non-code req with a passing MVR and no failing automated tests is met.""" db, req_id = _make_db_with_mvr(impl_type, mvr_passed=True) repo = RequirementsRepository(db) @@ -435,7 +500,7 @@ def test_meets_requirements_non_code_passing_mvr_is_true(impl_type): db.close() -def test_meets_requirements_in_code_failing_mvr_is_false(): +def test_completed_in_code_failing_mvr_is_false(): """IN_CODE req with annotation + passing auto-test but failing MVR must not be met.""" from reqstool.models.mvrs import MVRData @@ -473,7 +538,7 @@ def test_meets_requirements_in_code_failing_mvr_is_false(): # -- F2: skipped/missing automated tests must not be silently treated as passing -- -def test_meets_requirements_automated_skipped_test_is_false(): +def test_completed_automated_skipped_test_is_false(): """An automated-test SVC with a skipped test result must not count as met.""" db, req_id = _make_db_with_req( IMPLEMENTATION.IN_CODE, @@ -489,7 +554,7 @@ def test_meets_requirements_automated_skipped_test_is_false(): db.close() -def test_meets_requirements_automated_zero_test_results_is_false(): +def test_completed_automated_zero_test_results_is_false(): """An automated-test SVC with zero recorded test executions must count as missing, not passing.""" db, req_id = _make_db_with_req( IMPLEMENTATION.IN_CODE, @@ -569,7 +634,7 @@ def _make_db_with_superseded_mvrs(mvr_pass_sequence: list[tuple[str, bool]]) -> return db, req_id, svc_id -def test_compute_meets_superseded_fail_latest_pass_is_true(): +def test_completed_superseded_fail_latest_pass_is_true(): """fail→pass: latest (passing) MVR makes the verdict completed.""" db, req_id, _ = _make_db_with_superseded_mvrs([("2026-01-01T00:00:00Z", False), ("2026-01-02T00:00:00Z", True)]) repo = RequirementsRepository(db) @@ -579,7 +644,7 @@ def test_compute_meets_superseded_fail_latest_pass_is_true(): db.close() -def test_compute_meets_superseded_pass_latest_fail_is_false(): +def test_completed_superseded_pass_latest_fail_is_false(): """pass→fail: latest (failing) MVR makes the verdict not completed.""" db, req_id, _ = _make_db_with_superseded_mvrs([("2026-01-01T00:00:00Z", True), ("2026-01-02T00:00:00Z", False)]) repo = RequirementsRepository(db) diff --git a/tests/unit/reqstool/services/test_statistics_service.py b/tests/unit/reqstool/services/test_statistics_service.py index 8650cabe..c118c1be 100644 --- a/tests/unit/reqstool/services/test_statistics_service.py +++ b/tests/unit/reqstool/services/test_statistics_service.py @@ -14,7 +14,7 @@ ) from reqstool.models.svcs import SVCData, VERIFICATIONPHASE, VERIFICATIONTYPES from reqstool.models.test_data import TEST_RUN_STATUS -from reqstool.services.statistics_service import StatisticsService, TestStats +from reqstool.services.statistics_service import StatisticsService, TestStats, compute_requirement_status from reqstool.storage.database import RequirementsDatabase from reqstool.storage.requirements_repository import RequirementsRepository @@ -600,6 +600,85 @@ def test_post_build_manual_test_svc_is_non_gating_by_default(db): assert req_status.manual_tests.not_applicable is True +# -- compute_requirement_status (direct, not via StatisticsService) -- + + +def test_compute_requirement_status_not_applicable_with_no_svcs(db): + """A requirement with no linked SVCs at all is incomplete (nothing to verify against).""" + _insert_req(db) + db.insert_annotation_impl(REQ_ID, AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.Foo.bar")) + db.commit() + + repo = RequirementsRepository(db) + req = repo.get_all_requirements()[REQ_ID] + status = compute_requirement_status(req, repo) + + assert status.automated_tests.not_applicable is True + assert status.manual_tests.not_applicable is True + assert status.completed is False + + +def test_compute_requirement_status_missing_mvr(db): + _insert_req(db) + _insert_svc(db, verification=VERIFICATIONTYPES.MANUAL_TEST) + db.insert_annotation_impl(REQ_ID, AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.Foo.bar")) + db.commit() + + repo = RequirementsRepository(db) + req = repo.get_all_requirements()[REQ_ID] + status = compute_requirement_status(req, repo) + + assert status.manual_tests.missing == 1 + assert status.completed is False + + +def test_compute_requirement_status_missing_automated_test(db): + _insert_req(db) + _insert_svc(db, verification=VERIFICATIONTYPES.AUTOMATED_TEST) + ann = AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.FooTest.testBar") + db.insert_annotation_test(SVC_ID, ann) + db.insert_annotation_impl(REQ_ID, AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.Foo.bar")) + db.commit() + + repo = RequirementsRepository(db) + req = repo.get_all_requirements()[REQ_ID] + status = compute_requirement_status(req, repo) + + assert status.automated_tests.missing == 1 + assert status.completed is False + + +def test_compute_requirement_status_completed(db): + _insert_req(db) + _insert_svc(db, verification=VERIFICATIONTYPES.AUTOMATED_TEST) + ann = AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.FooTest.testBar") + db.insert_annotation_test(SVC_ID, ann) + db.insert_annotation_impl(REQ_ID, AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.Foo.bar")) + db.insert_test_result(URN, "com.example.FooTest.testBar", TEST_RUN_STATUS.PASSED) + db.commit() + + repo = RequirementsRepository(db) + req = repo.get_all_requirements()[REQ_ID] + status = compute_requirement_status(req, repo) + + assert status.completed is True + + +@pytest.mark.parametrize("impl_type", _ALL_NON_CODE) +def test_compute_requirement_status_non_code_with_annotation_raises(db, impl_type): + """The TypeError guard in _check_implementation must be reachable directly, not just via + StatisticsService's constructor (which also exercises this branch).""" + req_id = UrnId(urn=URN, id="REQ_ERR") + _insert_req(db, req_id=req_id, implementation=impl_type) + db.insert_annotation_impl(req_id, AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.Foo.bar")) + db.commit() + + repo = RequirementsRepository(db) + req = repo.get_all_requirements()[req_id] + with pytest.raises(TypeError, match="should not have an implementation"): + compute_requirement_status(req, repo) + + def test_post_build_manual_test_svc_gates_when_include_post_build_true(db): _insert_req(db, implementation=IMPLEMENTATION.NOT_APPLICABLE) _insert_svc(db, verification=VERIFICATIONTYPES.MANUAL_TEST, phase=VERIFICATIONPHASE.POST_BUILD) diff --git a/tests/unit/reqstool/storage/test_requirements_repository.py b/tests/unit/reqstool/storage/test_requirements_repository.py index 8a381488..c9eb187b 100644 --- a/tests/unit/reqstool/storage/test_requirements_repository.py +++ b/tests/unit/reqstool/storage/test_requirements_repository.py @@ -204,6 +204,26 @@ def test_get_all_mvrs(db): # -- Index/lookup queries -- +def test_get_svc(db): + _insert_requirement(db) + _insert_svc(db) + db.commit() + + repo = RequirementsRepository(db) + svc = repo.get_svc(SVC_ID) + assert svc is not None + assert svc.title == "SVC" + assert svc.verification == VERIFICATIONTYPES.AUTOMATED_TEST + assert REQ_ID in svc.requirement_ids + + +def test_get_svc_not_found(db): + db.commit() + + repo = RequirementsRepository(db) + assert repo.get_svc(SVC_ID) is None + + def test_get_svcs_for_req(db): _insert_requirement(db) _insert_svc(db, SVC_ID, req_ids=[REQ_ID]) From 5ff3728602d7759f56bf000005f857c6afae95c4 Mon Sep 17 00:00:00 2001 From: Jimisola Laursen Date: Tue, 23 Jun 2026 23:01:13 +0200 Subject: [PATCH 7/7] fix: address round-2 full-PR-review follow-ups - get_urn_details (details.py) still had the duplicate-annotations-query pattern that get_test_results_for_annotations was extracted to fix; route it through the shared method like compute_requirement_status does. - Add direct unit tests for get_test_results_for_annotations (METHOD found, METHOD missing, CLASS aggregation) and a test pinning that get_test_results_for_svc delegates to it without behavior drift. Signed-off-by: Jimisola Laursen --- src/reqstool/common/queries/details.py | 2 +- .../storage/test_requirements_repository.py | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/reqstool/common/queries/details.py b/src/reqstool/common/queries/details.py index 6c2e7778..50341f72 100644 --- a/src/reqstool/common/queries/details.py +++ b/src/reqstool/common/queries/details.py @@ -89,7 +89,7 @@ def get_svc_details( superseded_ids = {m.id for m in repo.get_superseded_mvrs_for_svc(svc.id)} test_annotations = repo.get_annotations_tests_for_svc(svc.id) - test_results = repo.get_test_results_for_svc(svc.id) + test_results = repo.get_test_results_for_annotations(svc.id.urn, test_annotations) all_reqs = repo.get_all_requirements() diff --git a/tests/unit/reqstool/storage/test_requirements_repository.py b/tests/unit/reqstool/storage/test_requirements_repository.py index c9eb187b..d2479ce4 100644 --- a/tests/unit/reqstool/storage/test_requirements_repository.py +++ b/tests/unit/reqstool/storage/test_requirements_repository.py @@ -311,6 +311,58 @@ def test_get_annotations_tests_for_svc(db): assert tests[0].fully_qualified_name == "com.example.FooTest.testBar" +def test_get_test_results_for_annotations_method(db): + _insert_requirement(db) + _insert_svc(db) + ann = AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.FooTest.testBar") + db.insert_test_result(URN, "com.example.FooTest.testBar", TEST_RUN_STATUS.PASSED) + db.commit() + + repo = RequirementsRepository(db) + results = repo.get_test_results_for_annotations(URN, [ann]) + assert len(results) == 1 + assert results[0].fully_qualified_name == "com.example.FooTest.testBar" + assert results[0].status == TEST_RUN_STATUS.PASSED + + +def test_get_test_results_for_annotations_method_missing(db): + ann = AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.FooTest.testBar") + db.commit() + + repo = RequirementsRepository(db) + results = repo.get_test_results_for_annotations(URN, [ann]) + assert len(results) == 1 + assert results[0].status == TEST_RUN_STATUS.MISSING + + +def test_get_test_results_for_annotations_class(db): + ann = AnnotationData(element_kind="CLASS", fully_qualified_name="com.example.FooTest") + db.insert_test_result(URN, "com.example.FooTest.testA", TEST_RUN_STATUS.PASSED) + db.insert_test_result(URN, "com.example.FooTest.testB", TEST_RUN_STATUS.FAILED) + db.commit() + + repo = RequirementsRepository(db) + results = repo.get_test_results_for_annotations(URN, [ann]) + assert len(results) == 1 + assert results[0].status == TEST_RUN_STATUS.FAILED + + +def test_get_test_results_for_svc_delegates_to_annotations(db): + """get_test_results_for_svc must produce the same results as resolving its own + annotations through get_test_results_for_annotations (it's a thin wrapper).""" + _insert_requirement(db) + _insert_svc(db) + ann = AnnotationData(element_kind="METHOD", fully_qualified_name="com.example.FooTest.testBar") + db.insert_annotation_test(SVC_ID, ann) + db.insert_test_result(URN, "com.example.FooTest.testBar", TEST_RUN_STATUS.PASSED) + db.commit() + + repo = RequirementsRepository(db) + via_svc = repo.get_test_results_for_svc(SVC_ID) + via_annotations = repo.get_test_results_for_annotations(URN, repo.get_annotations_tests_for_svc(SVC_ID)) + assert via_svc == via_annotations + + # -- Test result resolution --