feat(report): annotate static findings with structured source roles

[optimization2026]

Summary

Add bounded, report-only source-role annotations for static findings in supported local AISOP/AISP JSON files.

This is a conservative first increment of Phase 2 in #130, building on the phase-1 discovery and summaries in #211. It does not implement Phase 3 or close the broader issue.

The practical improvement is occurrence-level context: when identical text appears in an executable step and a constraint, each reported occurrence receives its own structural role and JSON Pointer rather than borrowing the representative occurrence's context.

Implementation

• Index admitted, cached UTF-8 source bytes with bounded JSON value spans. Reject decoded duplicate keys, invalid input and unsupported layouts rather than assigning an exact role.
• Attach evidence.structured_source_role to occurrence-specific display copies, after baseline suppression, risk calculation and finding compaction.
• Include the original character span, cached-content digest, structural role, mapping status and a bounded reason. risk_polarity remains unknown; role_confidence is null, not an uncalibrated probability.
• Reuse existing terminal, Markdown, JSON and SARIF sanitization/rendering. Keep phase-1 summaries separate and expose auxiliary mapping coverage.
• Bound documents, cached bytes, annotation records, pointer length, mapping time and ASCII-escaped annotation JSON size. Exhausting an annotation budget omits auxiliary context, not the original finding.

No new production dependency, workflow execution, resource traversal or filesystem reopening is introduced.

Safety and scope boundaries

Canonical findings, fingerprints, original severities, detection confidence, suppression decisions and risk results are not enriched or downgraded. The MCP response's embedded report carries annotations; its canonical findings array remains unchanged.

Precise mapping requires an admitted local .aisop.json source, matching raw/text caches, scanner-owned static origin and exact original columns. Transitive provenance, normalized or reconstructed views, missing columns and ambiguous ownership remain unknown. Suppressed findings are not annotated.

A structural constraint label is not proof that a policy was enforced or that the text is benign. Non-reserved function fields remain executable steps even when named hard_deny or example.

This is source-location context, not protocol certification, runtime conformance or a demonstrated reduction in false positives.

Validation

Submitted change on the updated base

Windows, Python 3.12.9, with an independent installed environment and verified imports from the submission checkout.

Base: a0d489e67f847aeedb5e6d471adfbbafc9e15670.

• The three added feature-test files passed.
• Seven selected upstream regression files: 500 passed, 1 skipped. The skip requires POSIX FIFO support.
• Ruff lint and format checks passed for src/ and tests/.
• Staged whitespace checks passed.
• The eight feature-file hashes matched the reviewed candidate after moving to this base.
• No local Windows test-compatibility changes were applied to the submission checkout or included in this PR.

The selected regression files cover reporting, deduplication, suppression, finding models, phase-1 structured summaries, shared source-location helpers and static-runner filtering.

Commands for the selected test groups

python -m pytest -q -ra -m "not integration and not provider" \
  tests/unit/test_structured_source.py \
  tests/unit/test_structured_role_report.py \
  tests/nodes/test_structured_role_reporting.py

python -m pytest -q -ra -m "not integration and not provider" \
  tests/nodes/test_report.py \
  tests/nodes/test_deduplicate.py \
  tests/unit/test_suppression.py \
  tests/test_models.py \
  tests/nodes/analyzers/test_structured_skill_roles.py \
  tests/nodes/analyzers/test_common.py \
  tests/nodes/analyzers/test_static_runner_filtering.py

ruff check --no-fix src/ tests/
ruff format --check src/ tests/

These are targeted checks, not the complete upstream CI suite. The existing test_static_runner_prepared_context.py was not included in this clean-checkout selection.

Earlier evidence, kept separate by revision

On the original base, 3c8e4b958fda9602e0044885f766be0e49f03585, the same final eight feature files passed 6/6 separate static-only native-directory acceptance cases: graph invocation, the in-process CLI application and MCP run_scan, each with JSON and SARIF output.

The synthetic directory had no SKILL.md sidecar. The checks verified both P1 occurrence roles and original source ranges, source digests, report structure, and canonical-finding isolation where exposed. This did not test the console launcher or MCP transport. These six cases were not rerun after the base update.

An earlier, pre-budget-fix candidate completed a broader regular Windows run with 10,693 passed and 16 failed. Eleven non-release failures were reproduced on the clean original baseline with matching classified outcomes. Five release-test failures remain unvalidated under safe Windows fake-CLI isolation. These failures are not treated as passes.

The complete upstream CI suite has not passed locally, and no current-candidate full-suite or complete cross-platform result is claimed.

Change boundary

Eight files only: two existing production files, two new production modules, three test files and one documentation file.

No changes to dependency manifests, lockfiles, release tooling, CI workflows or existing upstream tests. Local diagnostic scripts and test-compatibility patches are not part of this PR.

Refs #130. Follow-up to #211.

[rng1995]

[SkillSpector Review]

@optimization2026 thank you for the careful scoping here. The annotations are report-only and bounded, they fail closed to unknown, and the PR body is explicit about what was and was not validated.

Value and readiness

I found no blocking defect in the production code. CI is green, and the branch merges cleanly with current main. The open question is product value, and that needs the PIC.

This increment adds about 780 lines of production code: structured_source.py, a second hand-written JSON structural walker, plus structured_role_report.py. It also adds new public output fields. It changes no detection, score or completeness result, and it applies only to local *.aisop.json files. Its main purpose is to enable Phase 3 of #130, which your 2026-10-10 comment there describes as role-based counterfactual severity for P1 findings. That direction should be decided before Phase 2 lands, so I am marking this for manual review rather than approving it.

Material findings

None blocking. Optional:

1. src/skillspector/nodes/report.py:1861-1868: SARIF is now built from the expanded, possibly annotated display_findings instead of reported_findings. From reading _build_sarif, _expand_occurrences, _occurrence_columns and _occurrence_provenance, the output looks equivalent for ordinary findings. But the parity tests in tests/nodes/test_structured_role_reporting.py compare two runs of the new code with the annotator stubbed out. They would not catch a SARIF regression against main for compacted, multi-occurrence or transitive findings. Consider adding one golden comparison.
2. docs/structured-source-roles.md: no other page links to this one. Its "Verification before submission" section, which mentions a "development bundle's isolated-import runner", is contributor process rather than user documentation.
3. report.py:1947-1957: the terminal and Markdown summary line is appended after the formatted body, outside the Markdown report's sections.

PIC tradeoffs

• Format fit. AISOP/AISP is one third-party format (AIXP-Labs). The commit is authored and signed off as AISOP.dev <noreply@AISOP.dev>, which suggests the contributor is affiliated with the format. Is its adoption enough to justify a format-specific parser and report fields in core? Phase 1 (#211) is already merged.

• Roadmap risk. Phase 3 would use structural roles, taken from attacker-controlled files, to project lower severities. Even now, humans or LLM triage may read text_role: "constraint" as a mitigation, despite risk_polarity: "unknown".

• Output surface. The PR adds:

  • evidence.structured_source_role on findings in JSON, SARIF and the embedded MCP report
  • a top-level structured_role_coverage field
  • a SARIF structuredRoleCoverage property

  The canonical MCP findings do not carry the annotation, while the embedded report does. This is documented.

• DCO identity. The only sign-off is an organization alias with a no-reply address. CONTRIBUTING.md asks for Your Name <your.email>. Maintainers should decide whether that is acceptable.

Verification and gaps

• Read in full: structured_source.py, structured_role_report.py, the report.py and state.py diffs, and the docs page.
• Traced: report() from partition_findings through _expand_occurrences, the annotator, _sanitize_finding and _build_sarif.
• Context: I read #130 with its comments, and checked the status of PR #211.
• CI and merge: all 6 checks pass on fce629cf, and git merge-tree against main (789f5175) is clean.
• Not done: I reviewed the 1,381 test lines only by test name and key assertions, not line by line, and I ran no PR code.

[optimization2026]

Thank you for the thorough review and for clarifying that the remaining question is product fit rather than a blocking production-code defect.

I agree that Phase 2 should have value independently of Phase 3. Its immediate purpose is to provide occurrence-level source provenance for existing static findings, particularly when identical text appears in executable steps and constraint declarations. This is diagnostic context only, not evidence of safety, mitigation, or runtime enforcement.

I also understand the concern about maintaining a format-specific parser and public report fields in core. I’m open to narrowing the scope or considering a different integration approach if that better fits SkillSpector’s roadmap.

Phase 3 remains a separate, default-off experiment. Any future change to official risk scoring would require its own policy and safety review.

I can add the suggested baseline SARIF golden comparison to strengthen regression coverage.

For the product decision, would you prefer to bring this to the PIC, or is there a specific review process I should follow?

Please also let me know if the DCO sign-off format needs adjustment.

Thanks again for the detailed review.