Skip to content

fix: parse DeepSchema hook target files as YAML, not JSON - #349

Merged
nhorton merged 2 commits into
mainfrom
fix/deepschema-write-hook-yaml-parsing
Apr 8, 2026
Merged

fix: parse DeepSchema hook target files as YAML, not JSON#349
nhorton merged 2 commits into
mainfrom
fix/deepschema-write-hook-yaml-parsing

Conversation

@nhorton

@nhorton nhorton commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

The DeepSchema PostToolUse hook (deepschema_write) reported a false-positive File is not valid JSON: Expecting value: line 1 column 1 (char 0) on every Write/Edit of .deepreview files (and any other dot-prefixed filename).

Root cause: the hook keyed off Path.suffix to decide between yaml.safe_load and json.loads. For a file literally named .deepreview, Python treats the leading dot as a hidden-file marker and Path(".deepreview").suffix == "", so the file fell through to the JSON branch and json.loads failed on the first character of valid YAML.

Fix: parse both the target file and the referenced JSON Schema file with yaml.safe_load unconditionally. YAML is a superset of JSON, so both formats are accepted regardless of file extension. This mirrors the approach #338 shipped for the workflow quality gate (jobs/mcp/quality_gate.py) — which this bug pre-dated but #338 did not also fix, because deepschema_write.py had its own parallel code path.

  • DW-REQ-011.7.3 updated to match the new behavior.
  • New regression test (test_json_schema_validation_of_yaml_file_with_no_extension) covers the user-reported case with a .myappconfig target (chosen over .deepreview to avoid collision with the bundled standard deepreview schema).
  • Two pre-existing tests in tests/unit/test_deepschema_write_hook.py were checking the old "File is not valid JSON" / "schema file isn't JSON" contracts; both rewritten for the new semantics, plus a new guard: if the referenced JSON Schema parses to something that isn't an object or boolean (e.g., a bare YAML string), return Cannot read JSON Schema: not a JSON Schema object before invoking the validator (which would otherwise crash with a jsonschema.SchemaError).
  • Follow-up commit addresses a DRY finding from python_code_review: extracts _write_name_required_schema(tmp_path, slug, matcher) helper used by all 5 JSON-Schema-validation tests, and converts three function docstrings to leading comments so the traceability # THIS TEST VALIDATES ... comments remain the first thing inside the function body (matches the convention from feat: add requirements traceability annotations and new LA-REQ tests #346).

json.loads sweep

Per request, I audited all remaining json.loads / json.load call sites in src/ after this fix. 13 remain — none read user-authored content that could reasonably be YAML:

Call site What it parses Why it stays JSON
cli/install.py:52, setup/claude.py:26 ~/.claude/settings.json Claude Code's own settings file; JSON per spec
cli/jobs.py:56, jobs/mcp/state.py (7 calls) .deepwork/tmp/sessions/.../state.json Internal state serialization, written by deepwork itself
hooks/wrapper.py:273 Hook stdin from Claude Code Protocol-defined JSON
review/schema.py:17 Bundled deepreview_schema.json Package resource, always JSON
jobs/schema.py:18 Bundled job.schema.json Package resource, always JSON
deepschema/schema.py:13 Bundled deepschema.schema.json Package resource, always JSON

Test plan

  • uv run pytest tests/unit/deepschema/test_write_hook.py tests/unit/test_deepschema_write_hook.py — 43 passed
  • uv run pytest (full suite) — 1230 passed (coverage 97.95% matches pre-existing main baseline)
  • uv run ruff check && uv run ruff format --check && uv run mypy on changed files — clean
  • DeepWork reviews: 12 reviews in iter1, 7 reviews in iter2 after addressing findings, all PASS

🤖 Generated with Claude Code

nhorton and others added 2 commits April 8, 2026 17:07
The deepschema_write PostToolUse hook used Path.suffix to decide between
yaml.safe_load and json.loads. For files like `.deepreview` whose entire
name is treated as a hidden-file marker, Path.suffix is "" — so the hook
fell through to json.loads and reported "File is not valid JSON" on every
write/edit, even though the file was a perfectly valid YAML .deepreview.

Fix: parse the target file (and the referenced JSON Schema file) with
yaml.safe_load unconditionally. YAML is a superset of JSON, so both
formats are accepted regardless of file extension. Mirrors the approach
PR #338 already shipped for the workflow quality gate
(jobs/mcp/quality_gate.py).

Also handle the SchemaError case that becomes possible once a parser
accepts free-form text: if the schema file parses to something that
isn't a JSON Schema object or boolean, return "Cannot read JSON Schema:
not a JSON Schema object" before invoking the validator.

Updates DW-REQ-011.7.3 to match. Adds a regression test for dot-prefixed
files (the user-reported case) and revises two existing tests that were
asserting on the old "File is not valid JSON" / "schema isn't JSON"
contracts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract `_write_name_required_schema(tmp_path, slug, matcher)` helper
  in test_write_hook.py and use it across all 5 JSON-Schema-validation
  tests (removes ~90 lines of fixture duplication).
- Convert function docstrings to leading comments on three tests so the
  traceability `# THIS TEST VALIDATES A HARD REQUIREMENT` comments
  remain the first thing inside the function body (matches the repo
  convention established in PR #346).
- deepschema_write.py: cast `schema_data` (typed `dict | bool` after the
  spec-compliant shape check) to `dict[str, Any]` for the call to
  `validate_against_schema`; `jsonschema.validate` accepts bool schemas
  at runtime but our wrapper's type signature only declares dict.

All 43 tests in the affected files still pass. ruff + mypy clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nhorton
nhorton added this pull request to the merge queue Apr 8, 2026
Merged via the queue into main with commit 267b03c Apr 8, 2026
5 checks passed
@nhorton
nhorton deleted the fix/deepschema-write-hook-yaml-parsing branch April 8, 2026 23:31
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.

1 participant