Skip to content

Make referenced optional Python RPC requests omittable - #2556

Open
kondv wants to merge 1 commit into
github:mainfrom
kondv:dev/kondv/python-get-quota-optional
Open

Make referenced optional Python RPC requests omittable#2556
kondv wants to merge 1 commit into
github:mainfrom
kondv:dev/kondv/python-get-quota-optional

Conversation

@kondv

@kondv kondv commented Sep 7, 2026

Copy link
Copy Markdown

Fixes #1946

Problem

account.getQuota declares its params as a $ref to AccountGetQuotaRequest, and that referenced definition already carries the optional-request wrapper anyOf: [{ not: {} }, { ... }]. The Python generator's optional-params check only inspected the unresolved $ref, so it never saw the wrapper and emitted a required parameter:

result = await client.rpc.account.get_quota()
# TypeError: ServerAccountApi.get_quota() missing 1 required positional argument: 'params'

Callers had no values to supply, so the only workaround was to pass an empty request object.

Change

Resolve the params schema before the existing wrapper check, so a wrapper reached through a $ref is detected exactly like an inline one.

This corrects the five methods whose requests are declared that way — models.list, account.getQuota, plugins.marketplaces.refresh, sessions.list, and sessions.stopRemoteControl — and updates the Python quota example that documented the workaround.

The change is limited to that lookup. It adds no dependency and no general schema-validity inference, and it does not alter required requests, required-nullable fields, dataclass defaults, or how a supplied request is serialized. Other language generators are untouched.

Validation

  • npm run generate:python, rerun to confirm byte-identical output
  • python/test_rpc_generated.py — 12 passing; full Python unit suite — 407 passing
  • nodejs npm run typecheck; shared-codegen, typescript-codegen, client-api-codegen — 28 passing
  • ruff check and ruff format --check
  • Live A/B against the real Copilot CLI 1.0.83, using wheels built from each tree and installed into separate clean virtual environments outside the repository:
    • Unpatched main raised ServerAccountApi.get_quota() missing 1 required positional argument: 'params', matching the report.
    • The patched wheel returned real quota snapshots for chat, completions, and premium_interactions.
    • Controls in both arms: an explicit AccountGetQuotaRequest() still succeeded, and required-params account.login still rejected omission.

@kondv
kondv requested a review from a team as a code owner September 7, 2026 05:17
Copilot AI balanced review requested due to automatic review settings September 7, 2026 05:17

Copilot AI 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.

🟡 Changes recommended

Two moderate required-null handling issues remain unresolved in Python code generation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Enables omission of Python RPC request objects when allowed by the original schema while preserving required nullable fields.

Changes:

  • Adds draft-2019-09 schema validation for request optionality.
  • Regenerates Python RPC APIs and adds regression tests.
  • Updates dependencies and quota documentation.
File summaries
File Review
scripts/codegen/python.ts Two moderate issues: required nulls are dropped in optional session requests, and required opaque-JSON fields remain optional.
scripts/codegen/package.json Adds AJV dependency.
scripts/codegen/package-lock.json Locks codegen dependencies.
python/test_rpc_generated.py Adds generated signature and serialization tests.
python/copilot/generated/rpc.py Updates generated Python RPC models and methods.
nodejs/test/python-codegen.test.ts Tests schema evaluation and code generation.
nodejs/package.json Adds AJV for codegen tests.
nodejs/package-lock.json Locks test dependencies.
docs/features/usage-and-billing.md Documents argument-free quota retrieval.
Review details

Files not reviewed (2)

  • nodejs/package-lock.json: Generated file
  • scripts/codegen/package-lock.json: Generated file

Suppressed comments (1)

scripts/codegen/python.ts:3957

  • The server-method path has the same optional-wrapper/required-nullable interaction: filtering None here drops a required field when the request object is supplied explicitly. Select params.to_dict() when preserveRequiredNulls is true, while retaining {} for an omitted request.
            if (paramsOptional) {
                lines.push(`        params_dict = {k: v for k, v in params.to_dict().items() if v is not None} if params is not None else {}`);
            } else if (preserveRequiredNulls) {
                lines.push(`        params_dict = params.to_dict()`);
  • Files reviewed: 6/9 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/codegen/python.ts Outdated
Comment thread scripts/codegen/python.ts
@kondv
kondv force-pushed the dev/kondv/python-get-quota-optional branch from eb61822 to fd63cf1 Compare September 7, 2026 12:45
`account.getQuota` declares its params as a `$ref` to `AccountGetQuotaRequest`,
whose referenced schema already carries the Zod optional wrapper. The Python
generator's optional-params check only inspected the unresolved `$ref`, so the
generated method required an argument that callers have no values to supply.

Resolve the params schema before the existing wrapper check so referenced
optional requests are detected the same way inline ones already are.
@kondv
kondv force-pushed the dev/kondv/python-get-quota-optional branch from fd63cf1 to d5da672 Compare September 8, 2026 16:47
@kondv kondv changed the title Make optional Python RPC requests omittable Make referenced optional Python RPC requests omittable Sep 8, 2026
@kondv
kondv requested a balanced review from Copilot September 8, 2026 18:59

Copilot AI 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.

🟢 Approval recommended

The focused generator fix is covered by regression tests with no unresolved issues.

Review details
  • Files reviewed: 3/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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.

What 's the new usage of get_quota?

2 participants