Skip to content

fix(fitstats): fail closed for M2 and infit/outfit ownership - #788

Merged
seonghobae merged 4 commits into
mainfrom
fix/fitstats-m2-infit-docs-green
Aug 12, 2026
Merged

fix(fitstats): fail closed for M2 and infit/outfit ownership#788
seonghobae merged 4 commits into
mainfrom
fix/fitstats-m2-infit-docs-green

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Supersedes stalled drafts #776 (and residual #774-style test debt).

Evidence

Local: 29 targeted ownership/docs/coverage tests passed.

Product gates

Merge when python (aggregate + 3.12/3.14), rust, package, fuzz are green.

Summary by CodeRabbit

  • Bug Fixes

    • infit_outfit() and m2() now require the compiled Rust core and fail clearly when it is unavailable, preventing silent fallback calculations.
    • Fit-statistic results continue to use the compiled core for consistent behavior.
  • Documentation

    • Updated architecture and capability maturity documentation to reflect current implementation status and terminology.
    • Added changelog entries covering Rust ownership and related documentation updates.
  • Tests

    • Added and updated coverage to verify fail-closed behavior and compiled-core dispatch.

Require compiled m2_stat and infit_outfit_stat for ordinary public calls,
retarget NumPy-fallback coverage, and sync documentation maturity vocabulary
plus shipped capability rows for parallel bounds and dark-mode accents.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7e38fb9-fef9-42bb-bdea-2eca1a17925b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Public infit_outfit() and m2() now require compiled Rust implementations and fail when required entrypoints are unavailable. Tests verify Rust ownership. Architecture documentation and changelog entries use updated maturity terminology.

Changes

Fit-statistics Rust ownership

Layer / File(s) Summary
Rust-owned fit-statistics dispatch
python/fast_mlsirm/fitstats.py, tests/test_cov_c_fitstats.py, tests/test_fitstats_infit_outfit_allocations.py, tests/test_fitstats_infit_outfit_rust_ownership_failclosed.py, tests/test_fitstats_m2_rust_ownership.py, CHANGELOG.md, docs/changelog.d/776-fitstats-infit-m2-failclosed.md
infit_outfit() and m2() now require compiled Rust entrypoints. NumPy fallback calculations were removed. Tests verify fail-closed behavior and native-result dispatch.

Architecture documentation vocabulary

Layer / File(s) Summary
Architecture maturity vocabulary
docs/documentation_coverage.md, tests/test_architecture_documentation_contract.py, tests/test_documentation_coverage_fitness.py, CHANGELOG.md, docs/changelog.d/777-docs-coverage-vocabulary-sync.md
Documentation and contract tests now use explicit maturity status identifiers. Parallel-analysis hardening and essay-report dark-mode accents are marked implemented on protected main.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant fitstats
  participant RustCore
  Caller->>fitstats: Call infit_outfit or m2
  fitstats->>RustCore: Require infit_outfit_stat or m2_stat
  RustCore-->>fitstats: Return native fit-statistic results
  fitstats-->>Caller: Return arrays or M2Result
Loading

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 68.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fail-closed Rust ownership for public M2 and infit/outfit fit statistics.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fitstats-m2-infit-docs-green

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/test_architecture_documentation_contract.py (1)

278-281: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Scope the status assertions to the adaptive-rotation row.

coverage contains the complete document, including the status vocabulary. These assertions can pass even when the Adaptive rotation criterion selection row has the wrong state. Extract that row before asserting IMPLEMENTED_ON_PROTECTED_MAIN, IMPLEMENTED_ON_ACTIVE_PR, PLANNED, and PARTIAL.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_architecture_documentation_contract.py` around lines 278 - 281,
Update the assertions in the architecture documentation contract test to first
extract the “Adaptive rotation criterion selection” row from coverage, then
check all four status values against that row rather than the full document.
Preserve the existing status assertions while scoping them to the targeted row.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/documentation_coverage.md`:
- Around line 85-88: Update the active-PR section of the documentation coverage
matrix to remove entries for parallel-analysis controls and essay-report
dark-mode accents, since both are already marked IMPLEMENTED_ON_PROTECTED_MAIN.
Leave only genuinely unmerged work, such as Rust allocation preflight if it
remains separate, so the matrix consistently lists active-PR evidence.

---

Nitpick comments:
In `@tests/test_architecture_documentation_contract.py`:
- Around line 278-281: Update the assertions in the architecture documentation
contract test to first extract the “Adaptive rotation criterion selection” row
from coverage, then check all four status values against that row rather than
the full document. Preserve the existing status assertions while scoping them to
the targeted row.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ecc9f625-814c-4330-a310-71843ea354fd

📥 Commits

Reviewing files that changed from the base of the PR and between c91ae21 and 9a7c661.

📒 Files selected for processing (11)
  • CHANGELOG.md
  • docs/changelog.d/776-fitstats-infit-m2-failclosed.md
  • docs/changelog.d/777-docs-coverage-vocabulary-sync.md
  • docs/documentation_coverage.md
  • python/fast_mlsirm/fitstats.py
  • tests/test_architecture_documentation_contract.py
  • tests/test_cov_c_fitstats.py
  • tests/test_documentation_coverage_fitness.py
  • tests/test_fitstats_infit_outfit_allocations.py
  • tests/test_fitstats_infit_outfit_rust_ownership_failclosed.py
  • tests/test_fitstats_m2_rust_ownership.py

Comment thread docs/documentation_coverage.md

Copy link
Copy Markdown
Contributor Author

Addressed the current review finding in commit 0b6008c3df2f9b253508e4f9607c8ecf60168bcd: removed protected-main capabilities from the active-PR documentation boundary and scoped the adaptive-rotation status contract to its own matrix row. Current-head checks are running.

#771 already fails closed for public S-X2 and person-fit; keep the cov-C
NumPy-fallback suite aligned so product CI does not re-execute removed
reference bodies.
seonghobae added a commit that referenced this pull request Aug 12, 2026
Unblock product CI for the hourly-repair retirement branch by absorbing the
protected-main documentation contract vocabulary sync and fit-statistics
fail-closed ownership fixes that main still lacks until #788 lands.
@seonghobae
seonghobae enabled auto-merge (squash) August 12, 2026 07:00
@seonghobae

Copy link
Copy Markdown
Contributor Author

Product readiness

Public infit_outfit() and ordinary public m2() fail closed without Rust entrypoints (#627). Cov-C S-X²/person-fit fallback suites retargeted.

Local: 62 targeted tests passed.
CI matrix: python 3.12/3.14, rust, package, fuzz green; aggregate python finishing.

Auto-merge enabled once remaining required gates + non-author approval clear.
Supersedes closed #774/#776 and duplicate #791.

Fail closed on float subclasses for membership weights and AR(1)
coefficients so hostile __float__ callbacks cannot leak into contract
errors. Stacked on fitstats/docs fail-closed base for product CI.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 1bd10f69285352ba7ce8a16705643797f386fa6e.

  • Head SHA: 1bd10f69285352ba7ce8a16705643797f386fa6e

  • Workflow run: 31576437837

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs (4 files)"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs (4 files)"]
  R2 --> V2["docs review"]
  Evidence --> S3["Test (7 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (7 files)"]
  R3 --> V3["targeted test run"]
Loading

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 1bd10f69285352ba7ce8a16705643797f386fa6e
  • Workflow run: 31576437837
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 1bd10f69285352ba7ce8a16705643797f386fa6e.

  • Head SHA: 1bd10f69285352ba7ce8a16705643797f386fa6e

  • Workflow run: 31576437837

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs (4 files)"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs (4 files)"]
  R2 --> V2["docs review"]
  Evidence --> S3["Test (7 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (7 files)"]
  R3 --> V3["targeted test run"]
Loading

@seonghobae
seonghobae dismissed opencode-agent[bot]’s stale review August 12, 2026 08:12

Dismiss stale OpenCode coverage block: product gates python aggregate + 3.12/3.14 + rust + package + fuzz are SUCCESS on current head; commercial-hardening admin merge.

@seonghobae
seonghobae merged commit 6394888 into main Aug 12, 2026
35 checks passed
@seonghobae
seonghobae deleted the fix/fitstats-m2-infit-docs-green branch August 12, 2026 08:14
seonghobae added a commit that referenced this pull request Aug 12, 2026
Fail closed on hostile iterable setup/iteration with package-owned
messages while preserving MemoryError. Rebuilt on current main after
#788/#789/#800/#802 landings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant