Skip to content

fix(worker): use json.dumps for redis.publish payloads#118

Merged
JohnRDOrazio merged 1 commit intodevfrom
fix/issue-111-json-dumps-pubsub
Apr 28, 2026
Merged

fix(worker): use json.dumps for redis.publish payloads#118
JohnRDOrazio merged 1 commit intodevfrom
fix/issue-111-json-dumps-pubsub

Conversation

@JohnRDOrazio
Copy link
Copy Markdown
Member

@JohnRDOrazio JohnRDOrazio commented Apr 27, 2026

Summary

Replaces 11 hand-rolled f-string JSON payloads in `worker.py` with `json.dumps({...})`. The f-string approach worked by accident for fields holding only UUIDs and ints, but the `*_failed` handlers interpolated `str(e)` directly — exception messages routinely contain `"` and `\n` (e.g. `'foo' has no attribute "bar"`), which produced unparseable JSON and broke any subscriber that does `json.loads` on the payload.

What changed

Affected handlers in `ontokit/worker.py`:

  • `lint_started` / `lint_complete` / `lint_failed`
  • `normalization_started` / `_complete` / `_failed` / `_status` (×2)
  • `remote_check_started` / `_complete` / `_failed`

The newer `index_`, `consistency_`, and `duplicates_*` handlers were already using `json.dumps` — this brings the rest in line.

Channel names, message types, and field names are unchanged, so existing subscribers continue to work without modification.

Closes #111

Test plan

  • New regression test `test_lint_failure_payload_is_valid_json_with_quoted_error` invokes `run_lint_task` with a `RuntimeError` whose message contains both `"` and `\n`, then asserts the published `lint_failed` payload round-trips through `json.loads`.
  • All 1503 tests pass
  • mypy strict + ruff lint + ruff format clean

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed data serialization to properly handle error messages containing special characters (quotes, newlines).
    • Enhanced consistency in system event handling across linting and normalization operations.
  • Tests

    • Added regression test for linting task error handling.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cc3d0e9a-42b7-47a7-bcf2-55e0b5db0e57

📥 Commits

Reviewing files that changed from the base of the PR and between 978883d and 866e27f.

📒 Files selected for processing (2)
  • ontokit/worker.py
  • tests/unit/test_worker.py

📝 Walkthrough

Walkthrough

The changes replace f-string JSON payload construction in Redis publish calls with json.dumps() to properly escape special characters and ensure valid JSON serialization across lint, normalization, and remote check task event payloads.

Changes

Cohort / File(s) Summary
Payload Serialization Refactor
ontokit/worker.py
Replaced f-string JSON formatting with json.dumps() across 11 event publish locations (lint task status/complete/failed, normalization status/started/complete/failed and per-project cron status, remote check started/complete/failed). Ensures proper escaping of exception strings, consistent string conversion of run_id, correct JSON boolean types for dry_run/needs_normalization/has_changes, and normalizes missing optional values like commit_hash to empty strings.
Regression Test
tests/unit/test_worker.py
Added unit test for run_lint_task failure path verifying that Redis lint_failed event payload remains JSON-parseable when error messages contain double quotes and newline characters, confirming the fix prevents malformed JSON emission.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

A rabbit refactors with care and precision,
From f-strings to json.dumps with clear vision,
Escaping those quotes in exception's embrace,
Now Redis pubsub can parse every case! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately and concisely describes the main change: replacing f-string JSON formatting with json.dumps for redis.publish payloads in worker.py.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #111: replaces 11 f-string JSON payloads with json.dumps across lint, normalization, and remote_check handlers, ensuring proper escaping of error messages with quotes/newlines, and adds a regression test.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #111: converting f-string payloads to json.dumps, normalizing run_id to strings, fixing boolean fields, and adding a regression test. No schema, channel name, or subscriber changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-111-json-dumps-pubsub

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Replaces 11 hand-rolled f-string JSON payloads in worker.py with
json.dumps({...}). The f-string approach worked by accident for fields
holding only UUIDs and ints, but the *_failed handlers interpolated
str(e) directly — exception messages routinely contain quotes and
newlines (e.g. `'foo' has no attribute "bar"\nstacktrace`), which
produced unparseable JSON and broke any subscriber that does
json.loads on the payload.

Affected handlers (worker.py):
- lint_started / lint_complete / lint_failed
- normalization_started / _complete / _failed / _status (×2)
- remote_check_started / _complete / _failed

The newer index_*, consistency_*, and duplicates_* handlers were
already using json.dumps; this brings the rest in line.

Channel names, message types, and field names are unchanged so existing
subscribers continue to work without modification.

Adds test_lint_failure_payload_is_valid_json_with_quoted_error: invokes
run_lint_task with a RuntimeError whose message contains both `"` and
`\n`, then asserts the published lint_failed payload round-trips through
json.loads. Locks in the regression.

Closes #111

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohnRDOrazio JohnRDOrazio force-pushed the fix/issue-111-json-dumps-pubsub branch from 5f11dcc to 866e27f Compare April 28, 2026 06:16
@JohnRDOrazio
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@JohnRDOrazio JohnRDOrazio merged commit 2e7afb7 into dev Apr 28, 2026
13 checks passed
@JohnRDOrazio JohnRDOrazio deleted the fix/issue-111-json-dumps-pubsub branch April 28, 2026 07:24
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.

Use json.dumps for redis.publish payloads in worker.py

1 participant