Skip to content

[spark-compete] Narrow bare except in read_json + read_toml helpers in system_map.py to their exhaustive parse + OS error classes#532

Closed
4gjnbzb4zf-sudo wants to merge 1 commit into
vibeforge1111:masterfrom
4gjnbzb4zf-sudo:sentinel/exception-narrow/system-map-read-json-toml-helpers
Closed

[spark-compete] Narrow bare except in read_json + read_toml helpers in system_map.py to their exhaustive parse + OS error classes#532
4gjnbzb4zf-sudo wants to merge 1 commit into
vibeforge1111:masterfrom
4gjnbzb4zf-sudo:sentinel/exception-narrow/system-map-read-json-toml-helpers

Conversation

@4gjnbzb4zf-sudo

@4gjnbzb4zf-sudo 4gjnbzb4zf-sudo commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

{
"schema": "spark-compete-hotfix-v1",
"event": "spark-compete-first-event",
"submission_mode": "public_repo_pr",
"submission_target_url": "#532",
"team": {
"name": "SparkThisUp",
"members": ["ValHallaBuilder", "Baz707", "DanFireDash"],
"github_accounts": ["4gjnbzb4zf-sudo"],
"llm_device_holder": "ValHallaBuilder",
"device_holder_github": "4gjnbzb4zf-sudo"
},
"target_repo": {
"id": "vibeforge1111/spark-cli",
"source": "https://github.com/vibeforge1111/spark-cli",
"owner_surface": "spark-cli"
},
"issue": {
"type": "bug",
"severity": "low",
"title": "read_json and read_toml helpers in system_map.py lie about which class of failure triggered the diagnostic when the bare Exception catches a programming bug",
"actual_behavior": "src/spark_cli/system_map.py exposes two paired helpers used throughout the system-map compile pipeline: read_json(path) (L301) and read_toml(path) (L310). Both wrap a single load operation in except Exception as exc: return None, f'read_json_failed: {type(exc).__name__}: {exc}' (or the toml variant). The diagnostic string asserts the file failed to load -- but the bare Exception folds any unrelated runtime error (AttributeError on a typo, NameError from a refactor, KeyboardInterrupt, MemoryError) into the same read_json_failed: / read_toml_failed: synthetic detail. The operator sees a 'failed to read' line in their system-map output and assumes the file is malformed; the real cause is a programming bug in the helper.",
"expected_behavior": "The diagnostic is preserved for the documented failure conditions (malformed JSON / malformed TOML / unreadable file) and only those. Programming bugs propagate naturally so the operator sees them as a real traceback during compile.",
"repro_steps": [
"gh pr checkout ",
"cd into a working copy and: PYTHONPATH=src python -c \"from spark_cli.system_map import read_json, read_toml; from pathlib import Path; import tempfile, os; d=Path(tempfile.mkdtemp()); (d/'t.json').write_text('not json'); print(read_json(d/'t.json'))\" -> still returns (None, 'read_json_failed: JSONDecodeError: ...') exactly as before",
"Same trio for read_toml: missing file -> (None, 'missing'), malformed -> (None, 'read_toml_failed: TOMLDecodeError: ...'), valid -> (payload, None)",
"Inverse: edit a typo into the read_json try body (e.g. path.read_textZZZ) -- before the change, the AttributeError gets repackaged as read_json_failed: AttributeError: .... After the change, the AttributeError propagates so the operator sees the real bug instead of a misleading 'malformed JSON' line."
],
"affected_workflow": "system_map.py is the system-map compile module driven by spark report system-map and the underlying compile pipeline. read_json and read_toml are paired helpers consumed across the file (manifest probes, repo-board snapshots, capability cards, etc.). When the diagnostic lies about which class of failure happened, downstream compile-failure triage points the operator at the wrong file or assumes data corruption when the actual cause is a code bug in the helper."
},
"evidence": {
"safe_links_only": true,
"before_after_proof": "Two paired try/except patterns. Before: except Exception as exc: return None, f'read_json_failed: {type(exc).__name__}: {exc}'. After: except (json.JSONDecodeError, OSError) as exc: return None, f'read_json_failed: {type(exc).__name__}: {exc}'. read_toml's narrow tuple is (tomllib.TOMLDecodeError, OSError). The except body is byte-identical -- only the catch tuple changes. Each try body is one line: json.loads(path.read_text(encoding=...)) or tomllib.loads(path.read_text(encoding=...)). JSONDecodeError / TOMLDecodeError are the parse-time class for each library; OSError covers FileNotFoundError, PermissionError, IsADirectoryError, UnicodeDecodeError reasons for read_text to fail. Diff: 2 insertions, 2 deletions, one file.",
"links": ["https://github.com//pull/532"],
"forbidden": ["pdf","zip","exe","unknown downloads","shortened links","archives","binaries","tokens","browser cookies","wallet material","raw logs","raw conversations","raw memory","raw patches","private repo maps","private scoring details"]
},
"proposed_fix": {
"approach": "Replace except Exception as exc: with except (json.JSONDecodeError, OSError) as exc: in read_json, and with except (tomllib.TOMLDecodeError, OSError) as exc: in read_toml. Both narrow tuples are exhaustive for the single-operation try body. The except body is byte-identical -- the synthetic 'failed' diagnostic string still names the exception type and message via type(exc).__name__: {exc}, only now it can only be one of the narrow classes, so the diagnostic is no longer a lie.",
"files_expected": ["src/spark_cli/system_map.py"],
"tests_or_smoke": "Manual smoke verified the missing/corrupt/valid trio for both helpers: read_json returns (None,'missing'), (None,'read_json_failed: JSONDecodeError: ...'), (payload, None); read_toml returns the analogous trio. AST parse of system_map.py is clean. Diff is 2 insertions, 2 deletions."
},
"pr": {
"branch": "sentinel/exception-narrow/system-map-read-json-toml-helpers",
"title_prefix": "[spark-compete]",
"author_github": "4gjnbzb4zf-sudo",
"body_must_include": ["packet","team","pr_author","repo","actual_behavior","expected_behavior","repro_steps","before_after_proof","tests_or_smoke","duplicate_notes","risk_notes","review_claim"],
"url": "#532"
},
"review_claim": {
"impact_claim": "low",
"evidence_types": ["redacted_terminal_excerpt"],
"duplicate_notes": "Pre-flight gh pr list --search 'read_json read_toml narrow' returned zero hits. gh pr list --search 'system_map narrow' shows our own #526 (six different sqlite-introspection helpers in the same file). This PR touches L306/L314, two distinct helpers from #526's L651/L941/L983/L2576/L2684/L2742. No competitor PR touches these helpers. Same shape as our own #163 (json-load helpers in spark-intelligence-builder) and #526.",
"risk_notes": "Local scope: one file, two lines changed. The except body is byte-identical pre/post for both helpers. The missing-file fast-path and the valid-file fast-path are unchanged. The synthetic diagnostic string is preserved for the documented failure conditions (malformed JSON, malformed TOML, unreadable file). The only observable behavior change is that programming bugs in the helpers themselves no longer get mis-attributed to file corruption -- which is the goal of the fix.",
"review_state_requested": "pr_review"
}
}

@4gjnbzb4zf-sudo

Copy link
Copy Markdown
Contributor Author

QA write-up — for reviewer eyes

TL;DR. read_json and read_toml in src/spark_cli/system_map.py are the paired file-load helpers used across the system-map compile pipeline. Both wrap a single load operation in except Exception as exc: return None, f"read_json_failed: {type(exc).__name__}: ...". The bare Exception folds any unrelated runtime error (AttributeError on a typo, NameError from a refactor, KeyboardInterrupt) into a synthetic read_json_failed: ... / read_toml_failed: ... diagnostic that lies to the operator about what actually broke — it reads "file is malformed" when the real cause is a code bug in the helper. Narrow each catch to its exhaustive disk + parse class.

Before

def read_json(path: Path) -> tuple[Any | None, str | None]:
    if not path.exists():
        return None, "missing"
    try:
        return json.loads(path.read_text(encoding="utf-8-sig")), None
    except Exception as exc:
        return None, f"read_json_failed: {type(exc).__name__}: {exc}"


def read_toml(path: Path) -> tuple[dict[str, Any] | None, str | None]:
    if not path.exists():
        return None, "missing"
    try:
        return tomllib.loads(path.read_text(encoding="utf-8")), None
    except Exception as exc:
        return None, f"read_toml_failed: {type(exc).__name__}: {exc}"

If a typo lands in either try body (path.read_textZZZ), the AttributeError gets repackaged as a read_json_failed: AttributeError: ... line in the compile output — and the operator goes hunting for a malformed file that doesn't exist.

After

    except (json.JSONDecodeError, OSError) as exc:
        return None, f"read_json_failed: {type(exc).__name__}: {exc}"
    except (tomllib.TOMLDecodeError, OSError) as exc:
        return None, f"read_toml_failed: {type(exc).__name__}: {exc}"

The diagnostic is preserved for the documented failure conditions (malformed JSON, malformed TOML, unreadable file). Programming bugs propagate so the operator gets a real traceback during compile.

Smoke

$ PYTHONPATH=src python3 -c "
from spark_cli.system_map import read_json, read_toml
from pathlib import Path
import tempfile
d = Path(tempfile.mkdtemp())

p = d / 't.json'
p.write_text('not json')
print('corrupt json ->', read_json(p))
p.write_text('{\"a\":1}')
print('valid json ->', read_json(p))
print('missing ->', read_json(d / 'missing.json'))

p2 = d / 't.toml'
p2.write_text('not = toml = bad')
print('corrupt toml ->', read_toml(p2))
p2.write_text('a = 1')
print('valid toml ->', read_toml(p2))
"
corrupt json -> (None, 'read_json_failed: JSONDecodeError: Expecting value: line 1 column 1 (char 0)')
valid json -> ({'a': 1}, None)
missing -> (None, 'missing')
corrupt toml -> (None, 'read_toml_failed: TOMLDecodeError: Invalid value (at line 1, column 7)')
valid toml -> ({'a': 1}, None)

What didn't change

One file, two lines changed. Except body byte-identical pre/post for both helpers. The missing-file fast-path and the valid-file success-path are unchanged. The diagnostic string still names the exception class and message; it just can no longer name a class that has nothing to do with file IO or parsing.

Pattern-match: same shape as our own #163 (json-load helpers in spark-intelligence-builder) and #526 (sqlite-introspection helpers in the same file).

…to (json.JSONDecodeError, OSError) / (tomllib.TOMLDecodeError, OSError)

In src/spark_cli/system_map.py, read_json (L301) and read_toml (L310)
are paired helpers that load a config-style file from disk and return
(parsed_payload, error_string). Both catch bare 'except Exception:'
and serialize the exception type into the failure-detail string.

The bare Exception folds any unrelated runtime error (AttributeError
on a typo, NameError from a refactor, KeyboardInterrupt) into a
synthetic 'read_json_failed: ...' / 'read_toml_failed: ...' diagnostic
that lies to the operator about what actually broke -- it looks like
the file was malformed when really the surrounding code has a bug.

Narrow each to its exhaustive disk + parse class:
  read_json: (json.JSONDecodeError, OSError)
  read_toml: (tomllib.TOMLDecodeError, OSError)

Smoke: corrupt/valid/missing trio confirmed for both helpers, success
and documented-failure paths byte-identical pre/post.
@4gjnbzb4zf-sudo 4gjnbzb4zf-sudo force-pushed the sentinel/exception-narrow/system-map-read-json-toml-helpers branch from 8e567a9 to be716ad Compare June 2, 2026 00:45
@vibeforge1111

Copy link
Copy Markdown
Owner

Thanks, this was useful. We maintainer-adopted this exception-boundary improvement in #609 and merged it into master at 5e60f9a12c40438779a4aa1e0b9293e1fed7ce72.

What shipped:

  • JSON/TOML helpers now catch parse and OS errors instead of broad Exception.
  • Spawner PRD trace scanning catches line JSON decode failures and file OS failures at the right boundary.
  • git helper subprocess failures catch expected subprocess/OS errors.
  • revoke-all active mission pause write failures catch expected OS write errors.

Verification on the adopted PR:

  • focused system-map and revoke-all tests passed
  • full tests/test_system_map.py passed
  • full tests/test_cli.py passed
  • compileall, git diff --check, registry-pin verification, CodeQL, scorecard, secret-scan, and test-and-audit all passed

Source credit is recorded for this PR, and #609 itself should not receive participant credit. Points are still gated because this PR body had spark-compete-hotfix-v1 schema text but not a clean fenced JSON packet that the validator can extract. To unlock points review, add a clean fenced JSON packet in a PR comment or body update, then validate it at https://compete.sparkswarm.ai/api/packet/validate.

Closing this PR as adopted into #609. No code action is needed unless you believe #609 missed part of the issue.

@vibeforge1111

Copy link
Copy Markdown
Owner

R22/R23 final scoring update: this PR did receive final R22/R23 public leaderboard credit. Earlier gate, credit-review, or points-lock wording described the pre-final review state, not a final rejection or permanent zero. Final R22/R23 credit for this PR: 48 points. Thanks for the contribution.

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