Skip to content

fix(catalog-ui): reject invalid numeric settings - #325

Merged
kstonekuan merged 2 commits into
Hebbian-Robotics:mainfrom
Ftgn-dpA:fix/issue-297-catalog-ui-validation
Aug 31, 2026
Merged

kstonekuan merged 2 commits into
Hebbian-Robotics:mainfrom
Ftgn-dpA:fix/issue-297-catalog-ui-validation

Conversation

@Ftgn-dpA

Copy link
Copy Markdown
Contributor

Summary

Reject invalid Catalog UI numeric settings at construction, before they can reach socket setup or the polling wait loop.

Closes #297

Why

CatalogUiSettings previously relied only on range comparisons. Because bool subclasses int, port=True was accepted as port 1; floating-point ports were also accepted. For the polling interval, NaN, positive infinity, and True bypassed the existing non-positive check. Positive infinity can later raise OverflowError inside Event.wait.

This change keeps valid behavior unchanged while enforcing the field contracts described in the issue:

  • port is an int other than bool, in the inclusive range 1-65535.
  • catalog_poll_interval_seconds is an int or float other than bool, finite, and greater than zero.
  • IntEnum port values remain accepted.

Validation

  • python -m pytest -q tests/test_catalog_ui.py — 20 passed
  • ruff check src/hflow/catalog_ui.py tests/test_catalog_ui.py — passed
  • ruff format --check src/hflow/catalog_ui.py tests/test_catalog_ui.py — 2 files already formatted
  • ty check src/hflow/catalog_ui.py tests/test_catalog_ui.py — passed
  • The new validation cases were run against unmodified main first and failed for the reported boundaries.

Checklist

  • I added or updated outcome-focused tests for changed business logic.
  • Documentation is unchanged because valid behavior, flags, formats, and requirements are unchanged.
  • I ran Ruff check/format and ty in the WSL development environment.
  • I ran the relevant pytest suite.
  • I did not add recordings, generated media, credentials, private URLs, or runtime artifacts.
  • I preserved stored-data compatibility; this only validates settings at construction.

@github-actions

Copy link
Copy Markdown

👋 Hi @Ftgn-dpA — thank you so much for your first contribution to HFlow!

A maintainer will review your pull request as soon as possible. In the meantime:

💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game.

We are excited to have you here and appreciate your help making the project better! 🙌

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, merging. Welcome.

Validated on your branch: ruff check, ruff format --check, ty check clean, 1267 passed / 6 skipped.

Pushed one fixup, 4c494d0: hoisted replace and IntEnum to module scope. Nothing wrong with it, just house style.

Testing the wrong-type cases through replace() was a good instinct. It keeps ty honest about the direct constructor while still exercising the guard, and since replace() re-runs __post_init__, the invariant now covers mutation too, not only construction.

Worth knowing where this actually earns its keep: the CLI already declares --port with type=int and has no flag for the poll interval, so argparse was covering the command line. The hole was SDK callers building CatalogUiSettings directly, which is why the boundary belongs on the dataclass.

@kstonekuan
kstonekuan merged commit 0445520 into Hebbian-Robotics:main Aug 31, 2026
7 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.

Catalog UI settings accept invalid numeric values

2 participants