Repository navigation
Report injection_ml on ScanResult when the ML layer did not load - #203
HarshRajSinghania wants to merge 1 commit into
Conversation
 |
![greptile-apps[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return self._report_ml_degraded( | ||
| _limit_result( | ||
| LimitViolation( |
There was a problem hiding this comment.
The new _limit_result calls are indented outside their surrounding _report_ml_degraded calls. Python accepts this inside parentheses, but ruff format --check . requires changes. CI runs that check on every supported Python version, so this PR cannot pass as written. Format both wrappers and remove the extra blank line before _report_ml_degraded.
| if not isolated: | ||
| self._maybe_mark_session_tainted_from_scan(request.source) | ||
| return result | ||
| return self._report_ml_degraded(result) |
There was a problem hiding this comment.
Incremental streams still lose the new degradation fields after reusing a safe prefix. StreamScanner calls this newly annotated scan_request, then passes its result to merge_suffix_result. That function builds a fresh ScanResult without degraded or degraded_layers. After an allowed chunk exceeds the overlap length, later chunks report degraded=False even though the configured ML layer never loaded. Preserve both fields in merge_suffix_result and add a test covering two streamed chunks.
Knowledge Base Used:
| def test_scan_result_reports_injection_ml_degraded(self) -> None: | ||
| """Callers inspect ScanResult, not the private flag. #107.""" | ||
| with patch("unplug.guard.prepare_active_model_spec", return_value=None): | ||
| guard = Guard(config=GuardConfig(active_model="tiny")) | ||
|
|
||
| assert guard.ml_degraded is True | ||
| result = guard.scan("some text") | ||
| assert result.degraded is True | ||
| assert result.degraded_layers == ["injection_ml"] |
There was a problem hiding this comment.
The new tests check only ordinary guard.scan results. The PR also changes output scans, tool checks, limit returns, and exception returns, but none has an assertion for the new degradation fields. Add focused tests for those paths, plus preserving another layer and avoiding a duplicate injection_ml entry. Otherwise, removing one of the new wrappers or breaking the merge would still pass these tests.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This branch has not been deployed
1 participant
Fixes #107.
Summary
Local scans now copy
Guard._ml_degradedontoScanResultwhenactive_modelwas requested andinjection_mldid not load.degradedis true anddegraded_layersincludesinjection_ml.Motivation
#107:
Guard.ml_degradedwas true, butguard.scan()still returneddegraded=Falseanddegraded_layers=[]. Maintainer comment on the issue: keep degrade-by-default (require_ml=Truealready fails closed) and wire the existing flag through withdegraded_layers=["injection_ml"].Implementation
_report_ml_degradedannotates local results fromscan_request,scan_output_request, andcheck_tool_call, including limit and fail-closed results. Server-mode responses are left unchanged because that path reports degradation itself. An already-listedinjection_mllayer is not duplicated.Testing
cd sdk && uv venv --python 3.11 /tmp/unplug-venv && uv pip install --python /tmp/unplug-venv/bin/python -e . pytest/tmp/unplug-venv/bin/python -m pytest tests/unit/test_ml_degraded_warning.py -q→ 5 passedmake check-ciwas not run (needs the repo lockfile toolchain and the broader suite).Drafted with an AI assistant; I reviewed the diff and ran the tests above.