Skip to content

Reject non-object catalog manifest rows - #3965

Open
maziyarpanahi wants to merge 18 commits into
masterfrom
fix/issue-3562-catalog-row-shapes
Open

maziyarpanahi wants to merge 18 commits into
masterfrom
fix/issue-3562-catalog-row-shapes

Conversation

@maziyarpanahi

@maziyarpanahi maziyarpanahi commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

Description

Reject non-object JSONL rows at catalog parsing so the public coherence check returns an actionable diagnostic instead of an AttributeError. Diagnostics contain physical line numbers only, not row values or paths.

This follows the report and immutable reproduction shared by Belal Embaby. The fix was reproduced against current upstream and implemented in existing tracked source/test files; the contributor's fork was not modified.

Type of Change

  • Bug fix
  • Test addition/improvement

Changes Made

  • Validate the parsed JSON row before downstream mapping access.
  • Extend the existing coherence tests for array/scalar/null rows, physical blank-line accounting, normal CLI exit status, empty objects, malformed JSON and unchanged label validation.
  • Document the value-free diagnostic and update the changelog.

Testing

Current frozen head: 0d1cbb0f34b6b0afaad21191fa44a7192dd5a815. This branch normally merges the reviewed SDK prerequisite #3971 at 5e635dd707ebd74c2d0806b0cb6415b53ab4d7b0, preserving the original commits. The only issue-specific integration edit moves this PR's existing changelog text unchanged into the existing Fixed section.

Completed validation on this exact source, using Python 3.13.11 and a private environment:

  • Frozen dev environment preparation; make format, make lint, make format-check, and make type-check: passed. The configured type check covers four modules; formatting introduced no changes.
  • Focused issue/compatibility tests plus shared SDK, wheel-budget, CI-diagnostic and bounded benchmark-ID regressions: 160 passed.
  • Actionlint, immutable action-reference validation, repository policy, dependency-license checks and built-wheel license checks: passed.
  • uv build produced the sdist and wheel; the wheel-size/install-profile gate passed: 7,036,403 / 7,739,665 bytes.
  • High-severity/medium-confidence Bandit gate: passed, zero high-severity findings and no files skipped. Lower-severity findings are not represented as zero.
  • Current core-environment pip_audit_gate.py: passed, no unreviewed vulnerabilities found. This does not qualify optional dependency profiles or grant an exception.
  • git diff --check: passed.

The full local suite on this post-SDK head is pending, and hosted checks on this pushed head remain pending. This is not merge or release clearance. Full discovery and normal failure semantics are retained; no skip, maxfail or failure-masking override was added.

Historical local full-suite receipt, from earlier prepared commit 6b397cf4c073c00581399170835ebe0e166aa21f: 23,125 passed, 222 skipped, 24 warnings, zero failures/errors, 410.23s. That earlier candidate contained SDK ancestor 755f0eeebee2ba58764438263561ae077909e45c; it does not certify the new head above. Skips and warnings are reported as such, not counted as passed executions. The current full-suite result will replace this pending status when complete.

#3971 is a shared prerequisite, not part of the issue's original implementation. Its dependency/build changes remain separately scoped. This PR must pass its own final local and hosted gates and wait for prerequisite integration before merge.

Documentation

  • Updated the relevant public docstring and CHANGELOG.md.

Code Quality

  • Canonical formatting/lint/type gates passed.
  • Performed a separate diff review and checked accepted-input/negative controls.
  • No unrelated formatting changes or new tracked files.

Dependencies

Checklist

  • Read the contributing guide, Code of Conduct and maintainer guide.
  • One focused issue implementation commit is retained; shared prerequisite integration is recorded separately. The contributor's original fork remains unchanged.

Related Issues

Closes #3562

Screenshots/Examples

Not applicable; backend/evaluation contract fix with synthetic unit coverage.

@maziyarpanahi maziyarpanahi added this to the v3.0 milestone Oct 3, 2026
@maziyarpanahi maziyarpanahi added bug Something isn't working roadmap-v3 OpenMed V3 roadmap backlog labels Oct 3, 2026
@maziyarpanahi maziyarpanahi self-assigned this Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Thank you for the focused contribution for #3562. Its implementation and regression coverage already landed on master through #3650, and the issue is closed. I verified the release merge is present and the corresponding regressions still pass on current master.

This PR is held as superseded; the resolved issue provides no remaining implementation scope for another merge. I am retaining the original branch and discussion. Any additional behavior should be scoped to a new maintainer-backed issue before further changes or merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working roadmap-v3 OpenMed V3 roadmap backlog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Report non-object catalog rows without crashing the coherence gate

2 participants