fix(sleep): skip assistant.message events with non-dict data - #232
Open
pravit-amp wants to merge 1 commit into
Open
fix(sleep): skip assistant.message events with non-dict data#232pravit-amp wants to merge 1 commit into
pravit-amp wants to merge 1 commit into
Conversation
CopilotCliBackend._parse_jsonl_response assumed the data field of an assistant.message event was an object, so a truthy non-dict value raised AttributeError from the field access. That escaped the per-line try, which only wraps json.loads, and killed the parse of the entire stream. Port the isinstance guard already used by parse_copilot_jsonl in skillopt/model/copilot_backend.py, which was hardened in 5497a31 but did not reach this vendored copy. The wider except clause is kept, since json.loads raises RecursionError rather than JSONDecodeError on deeply nested payloads. Fixes the pre-existing failure in tests/test_sleep_engine.py::TestCopilotBackend::test_parse_jsonl_ignores_excessively_nested_json Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
pravit-amp please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
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.
Closes #231
Problem
CopilotCliBackend._parse_jsonl_responsetreated thedatafield of anassistant.messageevent as an object:A truthy non-dict value (
"text",5, a non-empty list) raisesAttributeErroron the field access. That happens outside the per-linetry, which only wrapsjson.loads, so a single malformed line aborts the parse of the whole stream and the backend returns nothing for that call.data: []slipped through only because[] or {}falls back to{}.Fix
Port the guard that
skillopt/model/copilot_backend.py::parse_copilot_jsonlalready applies to the same event format:That guard landed in 5497a31 ("harden and deduplicate JSONL parsing"), which removed this exact line from
copilot_backend.pyandcodex_harness.pyand folded all three copies into one helper.skillopt_sleep/backend.pypredates it and was left behind, since the sleep package deliberately keeps zero dependency on the research package.I kept the vendored copy rather than importing the shared helper, to preserve that decoupling. The docstring now records the relationship and asks for the two to be kept in sync, matching how
skillopt_sleep/gate.pydocuments its own vendoring of the validation gate.One intentional difference from the research-package copy: the wider
except (ValueError, RecursionError, TypeError)stays.json.loadsraisesRecursionError, notJSONDecodeError, on a deeply nested payload, so narrowing it to match would reintroduce a crash the sleep copy already handles. The docstring calls this out so it does not read as accidental drift.Tests
Added two focused cases to
TestCopilotBackend:test_parse_jsonl_skips_non_dict_data_without_losing_streamplaces a baddatavalue between two good messages and asserts"first\nsecond"still comes through, so a malformed event costs only its own line. Runs as subtests over"text",5,[1,2]andtrue. This mirrors the existing coverage intests/test_copilot_exec_backend.pyfor the research-package parser.test_parse_jsonl_ignores_non_object_top_levelcovers non-object top-level lines.This also fixes the pre-existing failure in
test_parse_jsonl_ignores_excessively_nested_json. That test builds a 2000 deep nested array expectingRecursionErrorfromjson.loads, but on Python 3.14 it parses fine, reaches the field access and hits theAttributeErrorinstead.Verification
154 passed, 1 skippedacrosstests/test_sleep_engine.pyandtests/test_copilot_exec_backend.py.1097 passed, 10 skipped, 2 failed, down from 3 failures onmain.ruff checkon both changed files reports 17 errors before and after, none on the touched lines. They are pre-existing (import tempfile, shutil, stat, trailing whitespace in tests, an unusedloggingimport) and left alone to keep the diff reviewable.No documentation changes, so
mkdocs build --strictwas not run.Environment
macOS 15, Python 3.14.6, branched from
mainat 9c776fc