Skip to content

fix: narrow JSON parse exception in analyze_bi.py - #116

Open
andrewwhitecdw wants to merge 3 commits into
Agent-Field:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/analyze-bi-json-except
Open

fix: narrow JSON parse exception in analyze_bi.py#116
andrewwhitecdw wants to merge 3 commits into
Agent-Field:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/analyze-bi-json-except

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

Bug

analyze_bi.py used a bare except: while parsing JSONL lines, which could mask KeyboardInterrupt and hide malformed input.

Fix

Catch json.JSONDecodeError explicitly.

Test

Added tests/test_diagrams_analyze_bi.py with a regression test that loads the script, feeds it a JSONL file containing an invalid line, and asserts valid lines are still parsed.

Verification

AGENTFIELD_SERVER=http://localhost:9999 pytest tests/test_diagrams_analyze_bi.py -v passes (1 test).

analyze_bi.py used a bare  while parsing JSONL lines, which
could mask KeyboardInterrupt and hide malformed input. Catch
json.JSONDecodeError explicitly instead.

Adds a regression test that loads the script, feeds it a JSONL file with
one invalid line, and asserts valid lines are still parsed.

Signed-off-by: Andrew White <andrewh@cdw.com>
The module imported nbformat at top level, but nbformat is not a
declared dependency, so tests importing the module failed in CI with
ModuleNotFoundError. Move the import into build_notebook(), the only
function that uses it.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

The regression test previously only checked that malformed JSON lines
are skipped. It did not verify the actual bug fix: narrowing the bare
 to  so non-JSON exceptions are
no longer swallowed.

Add a second test that monkeypatches  to raise
and asserts the exception propagates through .
@andrewwhitecdw

Copy link
Copy Markdown
Contributor Author

Repair update: the regression test now verifies the actual fix. In addition to checking that lines are skipped, it asserts that a non-JSON exception () raised inside propagates through instead of being swallowed by the previous bare . This addresses the adversarial audit finding that the original test did not exercise the narrowed exception clause.

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.

2 participants