Skip to content

fix(scoring): seal enterprise observation admission - #1043

Merged
seonghobae merged 4 commits into
mainfrom
fix/enterprise-observation-callback-safety-1042
Aug 24, 2026
Merged

fix(scoring): seal enterprise observation admission#1043
seonghobae merged 4 commits into
mainfrom
fix/enterprise-observation-callback-safety-1042

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Closes #1042.

Defect

The enterprise issue observation adapter admitted caller-defined ScoringRequest and EvidenceReference subclasses through isinstance(...) before provenance/fingerprint reads, and passed caller-defined string subclasses into ObservationStatus(...). Those paths could execute caller callbacks while semantic controls and provenance were still being validated.

RED → GREEN

  • RED d92b62e786daf245dcadd8dec5d414315ffdf48a: hostile request/evidence subclasses and a hostile status-string subclass require package-owned AssessmentSpecError rejection with zero callback execution. The protected-main implementation reaches metadata/fingerprint or enum hash/equality callbacks.
  • GREEN 5474ff4c58e3337b47828c4da87c82ef7ed21372: require exact ScoringRequest/EvidenceReference identities and accept status only as an exact ObservationStatus member or exact built-in serialized string before enum lookup.
  • Changelog 4c46667f48c4fff40c3a200718e09c61a3ab9ffe.

Ownership boundary

This changes only Python validation/provenance admission. Evidence counts remain simple validation/report metadata; no scoring, calibration, likelihood, estimator, ranking, utility, causal, or psychometric/statistical production arithmetic changes.

Verification boundary

Current branch is three commits ahead / zero behind protected main@04d0bc21a2a20693bcf16108cd76d394fe844d23, with a three-file effective diff. Keep Draft until exact-current-head CI, security/scanners, coverage/docstrings, package/fuzz evidence, and independent review are terminal and clean; predecessor-head evidence does not transfer.

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation for enterprise issue observations by rejecting unsupported custom object types before processing.
    • Prevented unintended callbacks or side effects during observation validation.
    • Preserved existing error codes and serialized status values.
  • Tests

    • Added regression coverage confirming unsafe custom objects are rejected without executing their callbacks.
  • Documentation

    • Added a changelog entry describing the observation validation safety improvements.

Open in Devin Review

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a71e2a9-72d1-407a-ab7e-f0599b14dd04

📥 Commits

Reviewing files that changed from the base of the PR and between 04d0bc2 and 0f25fc0.

📒 Files selected for processing (3)
  • docs/changelog.d/1042-enterprise-observation-callback-safety.md
  • python/fast_mlsirm/scoring/enterprise_issue/observation.py
  • tests/test_scoring_enterprise_issue_observation_callback_safety.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Enterprise observation validation now requires exact runtime types for requests, evidence references, and statuses. Regression tests verify that rejected subclasses cannot execute callbacks before validation completes. A changelog entry documents the behavior.

Changes

Enterprise observation callback safety

Layer / File(s) Summary
Exact-type admission and regression coverage
python/fast_mlsirm/scoring/enterprise_issue/observation.py, tests/test_scoring_enterprise_issue_observation_callback_safety.py, docs/changelog.d/1042-enterprise-observation-callback-safety.md
Observation validation now rejects ScoringRequest, EvidenceReference, and status subclasses before field access, hashing, equality, or enum lookup. Regression tests verify the existing assessment error codes and callback-free rejection. The changelog records the change.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 0f25f

The PR tightens enterprise observation input validation to reject caller-controlled subclasses before provenance or status handling, with focused regression coverage; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement issue #1042 by enforcing exact types, preserving valid values and error codes, and adding zero-callback regression tests.
Out of Scope Changes check ✅ Passed All changes remain within enterprise observation validation, marshalling, regression tests, and changelog documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: hardening enterprise observation admission.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/enterprise-observation-callback-safety-1042

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.

@seonghobae
seonghobae marked this pull request as ready for review August 20, 2026 02:33
@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 02:33
@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review
@cwl-noema-review review

Re-review exact current head 0f25fc0e28af166363e39c1ba255bee87854ee38 against protected main base 04d0bc21a2a20693bcf16108cd76d394fe844d23; use current source and Checks only. The current patch seals enterprise observation admission before callbacks.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review for 0f25fc0: required checks are terminal-success on this exact head. Please reacquire same-head approval and do not transfer stale predecessor or coverage-infrastructure review state. No source failure is shown. @opencode-agent review @cwl-noema-review review

@seonghobae

Copy link
Copy Markdown
Contributor Author

Reviewed exact current head 0f25fc0.

  • pytest -q tests/test_scoring_enterprise_issue_observation_callback_safety.py: 3 passed
  • Ruff and git diff --check: passed
  • Exact enum/string and record-admission boundaries reject caller subclasses before provenance or lookup callbacks; no additional source defect found.

Please regenerate protected review evidence for this exact head. @opencode-agent review @cwl-noema-review review

@seonghobae
seonghobae marked this pull request as draft August 20, 2026 16:13
auto-merge was automatically disabled August 20, 2026 16:13

Pull request was converted to draft

@seonghobae
seonghobae marked this pull request as ready for review August 21, 2026 02:47
@seonghobae

Copy link
Copy Markdown
Contributor Author

Reviewed exact HEAD 0f25fc0e28af166363e39c1ba255bee87854ee38. CodeGraph/source review confirms enterprise observation request/evidence/status boundaries reject caller-defined subclasses before provenance reads or enum lookup, preserving exact package records and serialized status behavior. Focused observation tests passed 15/15; Ruff, compileall, interrogate (100%), and diff checks passed. Hosted checks are green for this exact HEAD. No bypass or self-approval used.

@seonghobae
seonghobae enabled auto-merge (squash) August 21, 2026 02:47

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@opencode-agent opencode-agent Bot added priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: bug Defect or incorrect behavior labels Aug 22, 2026
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.


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.

@seonghobae
seonghobae merged commit 06e9892 into main Aug 24, 2026
44 of 45 checks passed
@seonghobae
seonghobae deleted the fix/enterprise-observation-callback-safety-1042 branch August 24, 2026 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(scoring): seal enterprise observation callback boundaries

1 participant