Skip to content

feat: map IntRange/FloatRange to numeric JSON Schema + PyPI install fix#38

Open
Coding-Dev-Tools wants to merge 3 commits into
masterfrom
cowork/improve-click-to-mcp
Open

feat: map IntRange/FloatRange to numeric JSON Schema + PyPI install fix#38
Coding-Dev-Tools wants to merge 3 commits into
masterfrom
cowork/improve-click-to-mcp

Conversation

@Coding-Dev-Tools

Copy link
Copy Markdown
Owner

Summary

  • Map click.IntRange / FloatRange options to numeric JSON Schema (integer/number) with min/max bounds (closed and open/exclusive). Previously these were introspected as a generic string, dropping the type and bounds — an MCP client was told a bounded numeric option was free-form.
  • Add tests/test_numeric_range_params.py (9 cases: closed bounds, open-ended, exclusive upper bound, multi-valued composition, invocation + out-of-range rejection).
  • Fix README PyPI install docs: package is not on public PyPI (live 404) — use git+ install; honest PyPI badge.

Verification

  • 203 tests pass; ruff check + format clean.

🤖 Generated with Claude Code

cowork-bot and others added 3 commits July 11, 2026 05:50
…dapter

The adapter advertised multiple=True options, variadic (nargs=-1) arguments,
and fixed-tuple (nargs>1) options as a single scalar string, and the handler
stringified their list/tuple value into one garbage CLI argument
(e.g. --tag "['a', 'b']"), so any LLM calling such a tool passed broken args.

- Map multi-valued params to JSON Schema arrays whose items carry the element
  type; pin length (minItems/maxItems) for fixed nargs>1 tuples.
- Expand list/tuple values in the handler: repeat the flag per value for
  multiple=True, emit one flag + N values for nargs>1, and one arg per element
  for variadic positionals. Scalar behavior is unchanged.
- Add tests/test_multi_valued_params.py (10 cases) covering schema shape and
  invocation for all three multi-valued forms plus scalar back-compat.

ruff check + format clean; existing adapter/list-tools/lint tests still pass.
… bounds

Previously IntRange/FloatRange options were introspected as a generic
"string" JSON Schema type, dropping both the integer/number type and
the min/max bounds. MCP clients were therefore told a bounded numeric
option was a free-form string.

- Add _apply_range_bounds() to emit minimum/maximum (or
  exclusiveMinimum/exclusiveMaximum for open bounds) from click's
  IntRange/FloatRange.
- Map IntRange -> integer and FloatRange -> number in
  _element_json_schema().
- Add tests/test_numeric_range_params.py (9 cases: closed bounds,
  open-ended, exclusive upper bound, multi-valued composition,
  invocation + out-of-range rejection).
- 203 tests pass; ruff check + format clean.
@Coding-Dev-Tools

Copy link
Copy Markdown
Owner Author

Council Gate Verdict — APPROVE

Risk level: low | Agreement: 1.0 | Council session: council-362f9036-3a16-4cef-b1f6-cfc733d23792

Per-model scores (rubric: correctness / safety / style / tests / complexity)

  • nvidia/nemotron-3-ultra-550b-a55b: APPROVE (overall 4.8) -- correctness=5.0, safety=5.0, style=5.0, tests=5.0, complexity=4.0
  • nvidia/llama-3.3-nemotron-super-49b-v1.5: APPROVE (overall 5.0) -- correctness=5.0, safety=5.0, style=5.0, tests=5.0, complexity=5.0
  • tencent/hy3:free: APPROVE_WITH_NITS (overall 4.4) -- correctness=4.0, safety=5.0, style=5.0, tests=4.0, complexity=4.0
  • nvidia/nemotron-3-super-120b-a12b: APPROVE (overall 5.0) -- correctness=5.0, safety=5.0, style=5.0, tests=5.0, complexity=5.0

Engraphis reference

Council verdict persisted as mem_01KXKYRG2HVY9E4NDX4KMD1Q6K (workspace Coding-Dev-Tools).

Posted automatically by the council-gate PR review cron. Auth/security PRs are reviewed at risk_level=high.

@Coding-Dev-Tools Coding-Dev-Tools added the council-approved Council gate approved this PR (APPROVE / APPROVE_WITH_NITS) label Jul 15, 2026

@Coding-Dev-Tools Coding-Dev-Tools left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Pre-PR Code Review — REQUEST_CHANGES (merge conflict)

Verdict: REQUEST_CHANGES — resolve conflicts before merge. (Code content itself is APPROVE-quality.)

The diff extends the prior #37 multi-valued work cleanly:

  • IntRange/FloatRange now map to numeric JSON Schema with correct open/closed minimum/exclusiveMinimum bounds.
  • Multi-valued params (multiple=True, nargs>1, variadic nargs=-1) map to array with items carrying the element type; fixed tuples pin minItems/maxItems.
  • Handler expansion correctly repeats flags for multiple=True, emits one-flag+N-values for nargs>1, and expands variadic positionals element-by-element. Tuple defaults converted tuple->list.
  • 193 new lines of tests (numeric range + multi-valued) — strong coverage.

Blocker: mergeable_state=dirty — the branch has merge conflicts against master and cannot be merged as-is. Please rebase/merge master into cowork/improve-click-to-mcp and resolve the conflicts, then confirm CI is green. After that this PR is approve-ready.

(Formal APPROVE withheld per fleet self-approval embargo + <6h/<3-contributor gate; single cowork-bot author.)

@Coding-Dev-Tools Coding-Dev-Tools left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Pre-PR Code Review — Verdict: 🔴 REQUEST_CHANGES (rebase — duplicate of already-merged #37)

Reviewed by Pre-PR Code Analyzer. Scope: in-scope Coding-Dev-Tools repo (excl. hermes/openclaw/openhuman).

Verdict: The new code is sound and well-tested, but the PR is CONFLICTING / DIRTY (mergeStateStatus: DIRTY, mergeable: CONFLICTING) and duplicates changes already merged via PR #37. It cannot be merged as-is. This is a branch-hygiene block, not a code-quality block.

Root cause: Head branch cowork/improve-click-to-mcp includes commit 569535fe ("fix multi-valued Click params", 2026-07-11) which was already merged to master as PR #37 on 2026-07-14. The branch is now 1 commit behind master and collides on exactly the files #37 touched: click_to_mcp/adapter.py and tests/test_multi_valued_params.py (the latter already exists on master — the PR re-adds it as a new file → conflict).

What the diff adds (reviewed favorably — this is the real value):

  • IntRange/FloatRange Click options now map to integer/number JSON Schema with minimum/maximum (closed) or exclusiveMinimum/exclusiveMaximum (open bounds), via new _apply_range_bounds / _element_json_schema. Open-ended ranges correctly omit the unset bound.
  • Composes correctly with multi-valued params (--tags multiple + IntRange → array of {type:integer, minimum, maximum}).
  • Honest docs: README/SKILL/CHANGELOG now point install at git+https://... since the package is not on public PyPI (was a live 404).
  • 2 strong new test files: test_numeric_range_params.py (bounds, open/closed, out-of-range rejection) and test_multi_valued_params.py (note: this filename collides with #37's merged test).

Required to unblock (REQUEST_CHANGES):

  1. Rebase cowork/improve-click-to-mcp onto current master, dropping the commits that #37 already merged (the multi-valued adapter.py changes and the duplicate tests/test_multi_valued_params.py).
  2. Keep only the IntRange/FloatRange additions; rename or merge your multi-valued test into the existing tests/test_multi_valued_params.py to avoid a file collision.
  3. Re-run ruff check . + pytest tests/ and confirm green.

Once rebased/conflict-free, this is an APPROVE-pending candidate (subject to the merge-gate: ≥3 reviewers / age ≥6h). The IntRange/FloatRange logic itself is correct and needs no change.

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

Labels

council-approved Council gate approved this PR (APPROVE / APPROVE_WITH_NITS)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants