Repository navigation
fix(mcp): guard against None optional params in measure_drift handler - #325
Merged
Merged
Conversation
…Q00#275) When MCP clients send `null` for optional array parameters (`constraint_violations`, `current_concepts`), `dict.get(key, [])` returns `None` because the key exists — it just has a null value. This causes `TypeError: object of type 'NoneType' has no len()` at the `log.info()` call before the drift engine is even reached. Replace `.get(key, [])` with `.get(key) or []` to handle both missing keys and explicit null values. Closes Q00#275 Co-Authored-By: Claude Opus 4.6 <[email protected]>
![ouroboros-agent[bot]](https://avatars.githubusercontent.com/in/3091302?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Reviewing commit
50d5bacfor PR #325
Blocking Findings
No in-scope blocking findings remained after policy filtering.
Non-blocking Suggestions
| # | File:Line | Category | Finding |
|---|---|---|---|
| 1 | src/ouroboros/mcp/tools/evaluation_handlers.py:137 | API contract | arguments.get(... ) or [] fixes the None case, but it also silently normalizes other falsy non-array values like "" or 0 to []. If you want to preserve the declared ARRAY contract more strictly, normalize only None here and let other invalid types fail fast or be validated explicitly. |
| 2 | tests/unit/mcp/tools/test_definitions.py:1205 | Test gap | The new regression test covers constraint_violations=None, but it does not assert the current_concepts=None outcome. Adding an assertion around ontology_drift would lock in the intended behavior for both optional fields. |
Design Notes
This is a narrowly scoped fix with an appropriate regression test. The handler remains tolerant of omitted optional inputs, and the change addresses the concrete runtime failure path without affecting the main drift calculation flow.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Sathvik-1007
pushed a commit
to Sathvik-1007/ouroboros
that referenced
this pull request
Apr 17, 2026
…Q00#275) (Q00#325) When MCP clients send `null` for optional array parameters (`constraint_violations`, `current_concepts`), `dict.get(key, [])` returns `None` because the key exists — it just has a null value. This causes `TypeError: object of type 'NoneType' has no len()` at the `log.info()` call before the drift engine is even reached. Replace `.get(key, [])` with `.get(key) or []` to handle both missing keys and explicit null values. Closes Q00#275 Co-authored-by: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #275 —
ouroboros_measure_driftcrashes withTypeError: object of type 'NoneType' has no len()when optional array parameters arenull.Root Cause
MeasureDriftHandler.handle()usesarguments.get("constraint_violations", [])to read optional parameters. When an MCP client sends an explicit JSONnullfor these fields (rather than omitting the key),.get()returnsNonebecause the key exists — the default[]is only used when the key is missing.This
Nonethen hitslen(constraint_violations_raw)in thelog.info()call (line 144), causing theTypeErrorbefore the drift engine is even reached.Fix
Replace
.get(key, [])with.get(key) or []for both optional array parameters. Theor []pattern handles both cases:None or []→[]null→None or []→[]Changes
src/ouroboros/mcp/tools/evaluation_handlers.py: 2-line fix forconstraint_violationsandcurrent_conceptsparameter handlingtests/unit/mcp/tools/test_definitions.py: Add regression test that passesNonefor both optional params and verifies successful drift measurementNote on issue comments
The issue comments suggest the crash is in
drift.py:155,213(Jaccard similarity calculation), but those lines compute set operations (&,|) which always produce sets, neverNone. The actual crash occurs earlier in the handler'slog.info()call. The drift calculation functions already haveif not x:guards that safely handleNone.Test plan
TestMeasureDriftHandler::test_handle_none_optional_params— new test, passesruff checkclean🤖 Generated with Claude Code