Skip to content

[Change] Implement truthful Code Review scope and differential enforcement #416

Closed

Description

@djm81

Why

specfact code review run --scope changed currently describes local worktree changes rather than a committed pull-request delta. Git-resolution failure can collapse into an empty selection and produce false-green evidence. C14 must make scope and uncertainty explicit before the project makes PR-range assurance claims.

Parent and coordination

Immutable compatibility contract

The first signed C14 release may advertise only core_compatibility: '===0.55.1', proven against lightweight core tag v0.55.1, full commit b1e517e60e669eaba15a18ecfa83ef5a9df65276, and full tree 47984be5434d7ae65ed6908bf525a32053290337. Ordinary equality, local aliases, ranges, wildcards, branch fallbacks, or another identity are not accepted.

What changes

  • Resolve explicit worktree, index, range, full, and positional-file scopes with immutable evidence and fail-closed UNKNOWN handling.
  • Compare merge-base/head analyzer findings under one pinned target-tip policy and classify introduced, fixed, unchanged, or unknown results deterministically.
  • Preserve changed tests and governed policy/config impact; reject incomplete or candidate-self-authorized assurance.
  • Produce module-side range_candidate/range_preview evidence and a signed static consumer contract. The protected workflow-native verifier and pr_range envelope remain core-owned downstream work.
  • Separate lifecycle, differential state, autofix availability, blocking policy, and authoritative schema 1.6 status.
  • Publish through the canonical signed modules workflow only after all prescribed gates pass.

Implementation gates

  • Revalidate the amended OpenSpec change against synchronized origin/dev and record readiness evidence.
  • Follow the exact named-tests → collected-selectors → accepted mapping → IMPLEMENTATION_CHECKPOINT.json → actual failing evidence → production implementation → passing evidence order.
  • Stay inside the closed allowlist in tasks.md; do not simplify the accepted contract without approval.
  • Run supported-Python exact-core compatibility smoke, contracts, full tests, type/lint, range/full Code Review, signature, and registry-integrity gates.
  • Stop after the verified signed module handoff; no core source implementation belongs to this issue.

Acceptance criteria

  • Scope resolution never converts Git, snapshot, policy, runtime, analyzer, or coverage uncertainty into no-impact or passing evidence.
  • Differential classifications and aggregate schema 1.6 status obey the accepted signed contracts.
  • The exact core v0.55.1 tag/commit/tree matrix loads the candidate module and validates the complete consumer matrix on CPython 3.11–3.13.
  • The reviewed implementation PR merges to dev, the canonical publish workflow produces the signed package/registry artifacts, and every identity required for core adoption is recorded.

Non-goals

  • Implement the protected core PR/merge-queue consumer or verification-envelope emitter.
  • Change Requirements R07/R08 semantics.
  • Add new analyzer rules, LLM review, or unbounded dependency inference.
  • Archive C14 before signed publication and downstream core-adoption gates permit it.

Activity

  1. self-assigned this
    on Aug 19, 2026
  2. added theissue type on Aug 19, 2026
  3. moved this from Todo to In Progress in SpecFact CLIon Aug 21, 2026
  4. djm81 commented on Aug 21, 2026

    @djm81
    ContributorAuthor

    Source Tracking update (2026-08-21): implementation PR #418 is open against dev from feature/code-review-14-delivery. Final candidate head is 158e33cbd699b3413a2cfddfd9ff83d20cfa28a6 / tree 41e51726b320f74bb11a5444bd3c4b64b81d779f; module metadata is specfact-code-review 0.49.0, checksum sha256:bc205355b32388d968ef30919bd57f30d147f46472c9099e8c19fc8f8b5d5470, core compatibility ===0.55.1. Local gates and the CPython 3.11-3.13 immutable-core matrix pass. Protected Linux capsule/cache/Bubblewrap and independent range-candidate verification remain the only delivery evidence pending CI; no core implementation is included.

  5. djm81 commented on Aug 23, 2026

    @djm81
    ContributorAuthor

    Final delivery update — 2026-08-23

    Implementation PR #418 has reached its frozen signed delivery head.

    • final substantive human code checkpoint: bbb5a705fe65e9036d1cf07229a6faddae5b7d51 / tree 37dda86458d3ed5b5797c81f14558722c220746c
    • canonical signed PR head: bdb16b69857fa4d4947d58a757f807c47627dc3e / tree de20668b7ec3ecb5ee832fdbee701da283c5b829
    • module: specfact-code-review 0.49.46
    • checksum: sha256:62175eace29561d22ae6c46751c4fe39b3f6e54890f4387ee571b05801c512ca
    • core compatibility: exact ===0.55.1, proven against tag v0.55.1, commit b1e517e60e669eaba15a18ecfa83ef5a9df65276, and tree 47984be5434d7ae65ed6908bf525a32053290337

    The review/remediation loop is complete at this head: all 156 GitHub review threads are resolved, no Critical/High/P1/P2 finding remains open, and no production or test code changed after the final substantive checkpoint. The two later commits contain only TDD evidence and the canonical manifest signature.

    Final verification passes: strict OpenSpec; format/type/Ruff/Pylint/YAML/registry/import/documentation gates; 28 contracts; independent smart and complete suites at 1563 passed each; exact-core schema/handoff/candidate-identity smoke on CPython 3.11-3.13; module signatures; protected signed-registry capsule cache-miss/cache-hit and empty Bubblewrap proof; CodeQL and Socket security; Requirements Evidence and Docs Review. Final changed-line Code Review has 0 blockers.

    C14's delivery boundary remains the accepted one: it freezes and hardens truthful scope, immutable snapshot/differential evidence, sealed analyzer/runtime identities, schema 1.6 status truth, and the signed module-side range candidate. It does not implement C15 severity/blocking calibration (#417), paired core adoption (nold-ai/specfact-cli#679), the protected pr_range consumer/envelope, or Requirements R07/R08 replay semantics.

    Two adjacent delivery corrections are retained with bounded scope: the legacy development-source Linux compatibility opt-in never applies to immutable/index/range assurance, and the signature hook now compares versions against the fetched target branch rather than a bot signature-only predecessor.

    No further code changes are planned before approval and squash merge to dev. After merge, the canonical publish workflow must produce the signed release/handoff; C14 must not be archived until the accepted downstream publication and adoption conditions are satisfied.

  6. 8 remaining items

  7. djm81 commented on Aug 25, 2026

    @djm81
    ContributorAuthor

    Promotion status update — 2026-08-25

    The bounded remediation and publication sequence reached dev through #429 and #430:

    • current dev: 3ef93ac4e4eda8762192faf95b83f67e074cbf9f
    • published candidate: specfact-code-review 0.49.49
    • archive SHA-256: 78706d115bd0fb0b5156df44d4f9d0dedb96a8d3d434627f4f30d5aaf191865d
    • signed payload checksum: sha256:4d707d9938b787d96d61ce88e0498da566ff263cf47fba869e3014a5744f1da1
    • exact core compatibility remains ===0.55.1
    • PR release: publish C14 Code Review module to main #420 final-head CI is green across exact-core, signature, and full quality matrices on Python 3.11–3.13

    Promotion is nevertheless blocked by one newly surfaced, validated P2 thread on #420. Suppression identity currently uses path/family/token/ordinal without source-location correspondence, so relocating identical directive text within a file is classified as unchanged/UNKNOWN instead of the C14-required introduced blocker. A direct reproducer confirms that relocation and a legitimate correspondence-preserving line shift are currently indistinguishable.

    This finding was not among #429s four authorized fixes. To preserve scope boundaries, it remains unresolved and requires a new narrow dev-target remediation limited to relocation correspondence, with failing-before relocation coverage and legitimate line-shift/pure-rename controls, followed by a new immutable patch publication. Do not merge #420, close #416, or archive C14 before that sequence is complete.

  8. linked a pull request that will close this issuefix(code-review): detect relocated suppressions #435on Aug 25, 2026
  9. djm81 commented on Aug 26, 2026

    @djm81
    ContributorAuthor

    Final bounded analyzer-completion remediation: #440. Fresh PR #420 review found two in-scope C14 defects (projected basedpyright positional override and Semgrep fatal/structured-error acceptance). The requested local review-agent pass confirmed both and found one additional instance of the same completion invariant: basedpyright fatal/configuration/argument exits could be accepted when JSON parsed. PR #440 fixes only those states; its own valid non-project extra_args regression was reproduced and corrected without admitting another finding family. Candidate specfact-code-review 0.49.58 is replacement-signed at head 3d1cda53; exact-core and quality matrices pass on Python 3.11/3.12/3.13, CodeRabbit reports no corrected-head actionable comments, and all #440 threads are resolved. Remaining sequence: human approval/merge #440, publish 0.49.58 to dev, then resolve the two intentionally open #420 threads and refresh the promotion candidate.

  10. linked a pull request that will close this issuefix(code-review): reject Ruff operational exits #442on Aug 27, 2026
  11. moved this from In Progress to Done in SpecFact CLIon Aug 27, 2026
  12. closed this as completedby moving to Done in SpecFact CLIon Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

change-proposalProposal for a new changecodebaseSpecfact codebase related topicenhancementNew feature or requestopenspecopenspec change

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions