Skip to content

Refactor sync data processing to handle PUSH and PULL modes with impr…#1653

Merged
jokob-sk merged 2 commits into
mainfrom
next_release
May 24, 2026
Merged

Refactor sync data processing to handle PUSH and PULL modes with impr…#1653
jokob-sk merged 2 commits into
mainfrom
next_release

Conversation

@jokob-sk
Copy link
Copy Markdown
Collaborator

@jokob-sk jokob-sk commented May 24, 2026

…oved error handling for JSON payloads

Summary by CodeRabbit

  • Bug Fixes

    • Improved sync artifact processing: skips invalid or malformed JSON files and more accurately extracts node names from various filename formats (including decoded/encoded variants), preventing processing interruptions.
  • Tests

    • Expanded tests for sync protocol: added coverage for malformed JSON, missing fields, pipe-delimited/empty files, and diverse filename formats and counters.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1e635fd3-3083-4bad-9e38-b514b97fbe52

📥 Commits

Reviewing files that changed from the base of the PR and between b7cffe8 and b5d2806.

📒 Files selected for processing (2)
  • .github/skills/code-standards/SKILL.md
  • test/plugins/test_sync_protocol.py

📝 Walkthrough

Walkthrough

Hub filename parsing now extracts node names from a revised dot-segment for decoded/encoded files and wraps JSON loading to skip invalid or structurally missing payloads. Tests mirror the filename logic and add coverage to ensure non-JSON or missing-data payloads are skipped without crashing.

Changes

Sync artifact parsing robustness

Layer / File(s) Summary
Hub-side sync parsing with JSON error handling
front/plugins/sync/sync.py
Hub receive processing changes filename parsing to use parts[3] for decoded/encoded files (otherwise parts[1]) and wraps JSON loading in try/except to skip files with invalid JSON or missing data key instead of failing.

Test updates and JSON robustness

Layer / File(s) Summary
Node-name mirror and Mode 3 filename tests
test/plugins/test_sync_protocol.py
Mirror _node_name_from_filename updated to use parts[3] for filenames containing decoded/encoded; Mode 3 filename tests expanded to cover PUSH decoded/encoded patterns, underscores, counters >1, and multiple plugin prefixes.
JSON parsing helper and Mode 3 JSON-skip tests
test/plugins/test_sync_protocol.py
Adds _parse_sync_payload helper and TestMode3JsonSkip tests asserting valid JSON payloads parse correctly and that pipe-delimited, missing-data, or empty files raise json.JSONDecodeError/KeyError so callers can skip them.

Code standards

Layer / File(s) Summary
No inline imports rule
.github/skills/code-standards/SKILL.md
Adds a coding-standard bullet forbidding inline imports and requiring all imports at the top of files.

🐰 From the warren of code, a steadier flow,
When JSON turns bad, we gracefully go—
No crashes from files that lack shape or form,
Our sync now resilient, our tests keep it warm!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is partially related to the changeset; it mentions refactoring sync data processing with PUSH/PULL modes and error handling, which aligns with the main changes, but the title is truncated and incomplete. Complete the full title to clearly convey the complete change. The truncated title (ending with 'impr…') makes it difficult to assess if it fully summarizes the main objective.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 next_release

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/plugins/test_sync_protocol.py`:
- Around line 490-539: Remove the inline imports of the json module inside the
helper and tests: in _parse_sync_payload() (the local "import json as _json"),
and in test_valid_sync_payload_is_parsed() and
test_json_without_data_key_raises_key_error() (their local "import
json")—instead rely on the module-level json import already present; update any
local references (e.g., _json.load → json.load or _json → json) accordingly so
no inline imports remain.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 88b8852f-a20a-4e6c-aa1e-39ec291ea958

📥 Commits

Reviewing files that changed from the base of the PR and between 0387d74 and b7cffe8.

📒 Files selected for processing (2)
  • front/plugins/sync/sync.py
  • test/plugins/test_sync_protocol.py

Comment thread test/plugins/test_sync_protocol.py
@jokob-sk jokob-sk merged commit a95aaa4 into main May 24, 2026
5 of 6 checks passed
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