Skip to content

Conversation

@hugocasa
Copy link
Collaborator

@hugocasa hugocasa commented Jan 6, 2026

  • fix(schema): preserve user-defined JSON schema for Python list[dict]
  • fix(schema): preserve all fields for untyped lists, not just properties
  • nits and publish

devdattatalele and others added 4 commits December 3, 2025 02:19
Fixes issue where JSON schema properties manually defined in the UI are
lost when saving Python scripts with list[dict] or untyped array parameters.

Changes:
- Preserve all items fields (properties, required, additionalProperties, etc.)
- Preserve items.type instead of hardcoding "object"
- Preserve type for untyped parameters using nullish coalescing
- Add type safety check for items preservation

The Python parser cannot infer object properties from list[dict] annotations.
This fix preserves user-defined schema fields when parser cannot infer structure.

Fixes #7209
Address bot feedback for consistency. The untyped list branch now preserves
all user-defined fields (required, additionalProperties, enum, etc.) just
like the record[] branch, instead of only preserving properties.

This ensures users who define required fields or enum values for untyped
list parameters don't lose that data on save.

Related to #7209
@claude
Copy link
Contributor

claude bot commented Jan 6, 2026

Claude finished @hugocasa's task in 2m 32s —— View job


PR Review: fix(schema): preserve user-defined JSON schema for Python list[dict] parameters

  • Analyze PR changes and context
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security implications
  • Provide comprehensive feedback

Summary

This PR fixes an issue where user-defined JSON schema properties for Python list[dict] parameters were being lost when the schema was re-inferred. The fix ensures that fields like properties, required, additionalProperties, enum, resourceType, contentEncoding, and description are preserved in the items object when the parser cannot infer the structure.

Changes Overview

File Change
parse-schema.ts Added ITEMS_PRESERVED_FIELDS constant and refactored duplicate code
tsconfig.json Updated module settings from CommonJS to ES2022
package.json files Version bumps
.gitignore Added entry to ignore .npmrc

Code Quality Assessment

Strengths:

  1. DRY Principle Applied - The ITEMS_PRESERVED_FIELDS constant eliminates code duplication between the two branches that handle untyped lists (cli/windmill-utils-internal/src/parse/parse-schema.ts:46-54).

  2. Type Safety - The use of as (keyof SchemaProperty["items"])[] provides proper typing for the preserved fields array, and the non-null assertion newS.items! is appropriate here since newS.items is always set before the loop.

  3. Consistent Formatting - Quote style changed from single to double quotes for consistency with the rest of the codebase.

Observations:

  1. Module System Update - The tsconfig.json changes (module: "ES2022", moduleResolution: "bundler") modernize the build configuration, which is appropriate for an ESM-first approach.

  2. Field Preservation Logic - The preserved fields (properties, required, additionalProperties, enum, resourceType, contentEncoding, description) cover the essential schema properties that users might customize.


Potential Issues

  1. Type Definition Mismatch - See inline comment on parse-schema.ts:54. The ITEMS_PRESERVED_FIELDS array includes "additionalProperties", "required", and "description" but these are not defined in the SchemaProperty["items"] interface. This works at runtime but creates a type inconsistency.

Recommendations

  1. Update the items interface to include all preserved fields for complete type safety (see inline comment).

  2. Consider adding unit tests for the argSigToJsonSchemaType function to verify that schema preservation works correctly for various edge cases.


Security Implications

No security concerns identified. This change only affects how schema metadata is preserved during type inference.

Performance Considerations

The change has negligible performance impact. The forEach loop over 8 preserved fields is minimal overhead compared to the overall schema processing.


Verdict

Approve with minor suggestions - The core fix is correct and addresses the schema preservation issue. The refactoring to use a shared constant improves maintainability. The only recommended improvement is updating the TypeScript interface to include all preserved fields for complete type safety.


Testing Instructions

To verify the changes, navigate to the Script Editor in Windmill, create a Python script with a list[dict] parameter (e.g., items: list[dict]). In the schema editor, customize the array's items properties (add properties with specific fields, set required, add a description, etc.). Save and re-open the script, or trigger a schema re-inference by modifying the code. Verify that your custom schema properties are preserved and not overwritten with empty defaults.

@cloudflare-workers-and-pages
Copy link

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: d6d0d53
Status: ✅  Deploy successful!
Preview URL: https://a2f45076.windmill.pages.dev
Branch Preview URL: https://fix-preserve-python-array-sc.windmill.pages.dev

View logs

"resourceType",
"contentEncoding",
"description",
] as (keyof SchemaProperty["items"])[];
Copy link
Contributor

Choose a reason for hiding this comment

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

The ITEMS_PRESERVED_FIELDS array includes "additionalProperties", "required", and "description" which are not defined in the SchemaProperty["items"] interface (lines 20-26). While this works at runtime, it creates a type inconsistency.

Consider updating the items interface to include these fields:

Suggested change
] as (keyof SchemaProperty["items"])[];
] as (keyof SchemaProperty["items"])[];
// Note: Consider adding required?, additionalProperties?, and description?
// to the items interface above for complete type safety

Alternatively, update the interface definition around line 20:

items?: {
  type?: "string" | "number" | "bytes" | "object" | "resource";
  contentEncoding?: "base64";
  enum?: EnumType;
  resourceType?: string;
  properties?: { [name: string]: SchemaProperty };
  required?: string[];
  additionalProperties?: boolean;
  description?: string;
};

@rubenfiszel rubenfiszel merged commit 7877999 into main Jan 6, 2026
30 checks passed
@rubenfiszel rubenfiszel deleted the fix/preserve-python-array-schema-7209-clean branch January 6, 2026 18:42
@github-actions github-actions bot locked and limited conversation to collaborators Jan 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants