Skip to content

perf(workbench): avoid copying unused dictionary items - #1661

Draft
mldangelo-oai wants to merge 1 commit into
mainfrom
mldangelo-oai/bounded-finding-preview
Draft

mldangelo-oai wants to merge 1 commit into
mainfrom
mldangelo-oai/bounded-finding-preview

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Summary

Dictionary previews create a list of every item before selecting the first 20. Use itertools.islice to enforce the existing limit without allocating that temporary list.

Changes

  • Import islice and use it for the dictionary preview loop.
  • Preserve insertion order, field limits, byte budgets, depth limits and text handling.

One production file changes. There is no net line-count change or dependency change.

Testing

Opened as a draft: local validation and independent review are incomplete.

  • git diff --check: passed.
  • node --experimental-strip-types .github/scripts/check_plugin_source_compatibility.mts: passed against the complete source checker. This does not replace the required compiled invocation.
  • node sdk/typescript/scripts/check-plugin-source.mjs: passed.
  • Focused pytest and required Ruff check could not start because their modules are unavailable.
  • The original SDK finding-preview/report-limit tests had three setup failures because the generated bundled plugin is absent.
  • SDK build and remaining required checks were not run because build dependencies are unavailable.
  • Independent review is pending. No clean review receipt is claimed.

Risk and rollout

The production caller projects freshly decoded JSON dictionaries. For that path, the change retains the first-20-item order and projection behavior, including invalid-Unicode handling. It changes no CLI arguments, response schemas, persisted data or resource limit.

Custom dictionary subclasses with side effects during iteration, or concurrent mutation, could observe different behavior; the identified production path does not supply those objects. The benefit is avoiding an unnecessary allocation, not a claimed production benchmark improvement. Complete the required validation and review before merging.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant