Feat/92 new#117
Conversation
- Added a new naming standards configuration interface and rules for various object types (catalog, schema, table, view, column). - Introduced a validation mechanism for names against defined patterns, providing suggestions for violations. - Created a modal to warn users when naming standards are violated during renames and adds, allowing them to proceed or use suggestions. - Integrated naming standards settings into the project settings panel, enabling users to apply templates for common naming conventions. - Updated the ColumnGrid and Sidebar components to utilize the new naming validation logic during column and object renames/adds. - Enhanced UI with appropriate styles for naming standards settings and warning modal.
- Add NamingStandardsConfig and NamingRule classes to manage naming conventions. - Introduce validate_name command to validate object names against defined patterns. - Enhance existing validation logic to include naming standards checks. - Implement strict mode for naming standards, affecting both new and existing objects. - Update VS Code extension to support naming standards configuration in project settings. - Create UI components for managing naming standards in the settings panel. - Add async validation for name checks during object creation and renaming. - Introduce collapsible sections in the settings UI for better organization. - Update styles for new UI components and ensure consistent user experience. - Document the naming standards feature and its implementation plan.
…onfiguration handling
…th toggle functionality
…case and pascalcase presets - Removed camelcase and pascalcase naming presets from the CLI and configuration. - Updated the CLI command for loading naming templates to reflect the changes. - Enhanced the UI to inform users that Unity Catalog stores object names in lowercase. - Improved error messaging for empty naming rule patterns. This commit focuses on simplifying the naming standards management and improving user guidance in the settings panel.
- Introduced a new validation mechanism for object names against naming standards in the CLI. - Added the `--naming` flag to the `validate` command, allowing users to validate a single object name. - Implemented the `_run_validate_naming` function to handle naming validation logic and output results. - Updated the existing `validate` function to incorporate naming validation, ensuring required parameters are checked. - Removed the deprecated `validate-name` command to streamline the CLI interface. This commit focuses on improving the user experience by providing robust validation for naming standards directly within the CLI.
- Updated the CLI reference documentation to include new commands for managing naming standards. - Added detailed descriptions for the `schemax naming` command group, including options for toggling strict mode and applying naming rules. - Expanded the `schemax validate` command documentation to reflect the new naming validation features, including error reporting for naming violations. - Introduced a new section on Naming Standards, linking to the comprehensive guide for user reference. This commit improves the documentation to support the recent enhancements in naming standards management within the CLI, ensuring users have clear guidance on utilizing these features.
…ebar - Added warnings for unexpected errors during naming validation in the `validate` command, improving user feedback. - Updated `ColumnGrid` and `Sidebar` components to include a modal for naming warnings, allowing users to proceed with suggestions or original names. - Introduced a `proceedLabel` prop for customizable button labels in the naming warning modal, enhancing UI flexibility. - Improved error handling and user experience by providing clear options for handling naming violations. This commit enhances the naming validation process and user interaction within the UI, ensuring a smoother experience when managing object names.
There was a problem hiding this comment.
Pull request overview
Adds project-level naming standards (regex rules) to SchemaX and wires enforcement across the Python CLI/SDK, VS Code Designer UI, and documentation.
Changes:
- Introduces
settings.namingStandardsconfig (rules per object type +applyToRenames+strictMode) with Python validation/suggestion helpers and CLI management commands. - Integrates naming checks into
validate,sql, andapply, and adds a VS Code Designer settings UI plus add/rename enforcement flows. - Updates extension/webview messaging to validate names via the Python backend and expands docs/release notes accordingly.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| plan.md | Implementation plan for naming standards feature (design/architecture notes). |
| packages/vscode-extension/tests/webview/components/UnityTargetSettings.test.tsx | Updates assertion text after UI heading/description restructuring. |
| packages/vscode-extension/tests/webview/components/Sidebar.test.tsx | Mocks naming validation hook/modal for Sidebar tests. |
| packages/vscode-extension/tests/webview/components/ColumnGrid.test.tsx | Extends store mock + mocks naming validation hook/modal for ColumnGrid tests. |
| packages/vscode-extension/tests/unit/architecture-fitness.test.ts | Adjusts rule to allow pythonBackend.run usage only in extension.ts. |
| packages/vscode-extension/src/webview/utils/useNameValidation.ts | New hook to request name validation via extension host messaging. |
| packages/vscode-extension/src/webview/styles.css | Adds collapsible section styles + naming standards settings + naming warning modal styles. |
| packages/vscode-extension/src/webview/models/unity.ts | Adds NamingStandardsConfig/NamingRule to webview project model. |
| packages/vscode-extension/src/webview/components/settings/UnityTargetSettings.tsx | Removes duplicated section wrapper/heading; keeps description + env list UI. |
| packages/vscode-extension/src/webview/components/settings/NamingStandardsSettings.tsx | New Naming Standards settings editor + template presets + toggles. |
| packages/vscode-extension/src/webview/components/settings/CollapsibleSection.tsx | New reusable collapsible section wrapper used by Project Settings panel. |
| packages/vscode-extension/src/webview/components/Sidebar.tsx | Adds naming validation to add/rename flows + warning modal + pending UI state. |
| packages/vscode-extension/src/webview/components/ProjectSettingsPanel.tsx | Adds collapsible sections, naming standards editor section, and save-time validation. |
| packages/vscode-extension/src/webview/components/NamingWarningModal.tsx | New shared modal for non-blocking naming violations (rename/add warning path). |
| packages/vscode-extension/src/webview/components/ColumnGrid.tsx | Adds naming validation to add/rename column flows + warning modal + pending UI state. |
| packages/vscode-extension/src/webview/App.tsx | Updates refresh button label/tooltip to reflect validation behavior. |
| packages/vscode-extension/src/storage-v4.ts | Adds naming standards types to persisted project settings model. |
| packages/vscode-extension/src/extension.ts | Adds validate-on-refresh option; applies naming config via CLI on save; adds validate-name message handler. |
| packages/python-sdk/tests/unit/test_naming_config.py | Unit tests for naming config read/write/apply/template CLI helpers. |
| packages/python-sdk/tests/unit/test_naming.py | Unit tests for naming core logic, suggestions, bulk validation, and strict-mode behavior. |
| packages/python-sdk/src/schemax/core/naming.py | New naming standards core (config model, validate, suggest, bulk validation). |
| packages/python-sdk/src/schemax/commands/validate_name.py | New command function to validate a single name against configured rules. |
| packages/python-sdk/src/schemax/commands/validate.py | Adds naming validation (warnings vs errors under strict mode) + shared preflight helper. |
| packages/python-sdk/src/schemax/commands/sql.py | Runs preflight validation before SQL generation; prints warnings. |
| packages/python-sdk/src/schemax/commands/naming_config.py | New naming config management helpers + presets for CLI/UI integration. |
| packages/python-sdk/src/schemax/commands/apply.py | Runs preflight validation before apply; blocks on errors. |
| packages/python-sdk/src/schemax/commands/init.py | Exports validate_name_command. |
| packages/python-sdk/src/schemax/cli.py | Adds validate --naming mode and a new schemax naming command group. |
| package-lock.json | Removes optional notifier dependencies (lockfile changes). |
| docs/schemax/docs/reference/release-notes.mdx | Adds release notes entry for naming standards feature (0.2.12). |
| docs/schemax/docs/reference/cli.mdx | Documents naming standards CLI commands + validate --naming. |
| docs/schemax/docs/guide/naming-standards.mdx | New guide documenting config shape, UI, CLI, templates, and strict mode. |
| CLAUDE.md | Adds repository working agreements and commands for Claude Code usage. |
| .claude/commands/ui-design.md | Adds a detailed webview UI design system reference for consistent UI changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| }; | ||
|
|
||
| const handleEnableRule = (key: ObjectTypeName, enabled: boolean) => { | ||
| if (!enabled && !config?.[key]) return; |
| const validate = useCallback( | ||
| (name: string, objectType: string): Promise<NameValidationResult> => { | ||
| setPending(true); | ||
| return new Promise<NameValidationResult>((resolve) => { | ||
| const handler = (event: MessageEvent) => { | ||
| if (event.data?.type === 'name-validation-result') { | ||
| window.removeEventListener('message', handler); | ||
| setPending(false); | ||
| resolve(event.data.payload as NameValidationResult); | ||
| } | ||
| }; | ||
| window.addEventListener('message', handler); | ||
| vscode.postMessage({ type: 'validate-name', payload: { name, objectType } }); | ||
| }); |
| case "validate-name": { | ||
| const { name, objectType } = message.payload as { name: string; objectType: string }; | ||
| outputChannel.appendLine( | ||
| `[SchemaX] validate-name: name="${name}" type="${objectType}"` | ||
| ); | ||
| try { | ||
| const envelope = await pythonBackend.runJson<{ | ||
| valid: boolean; | ||
| name: string; | ||
| objectType: string; | ||
| error: string | null; | ||
| suggestion: string | null; | ||
| pattern: string | null; | ||
| description: string | null; | ||
| }>( | ||
| "validate-name", | ||
| ["validate", "--naming", "--name", name, "--type", objectType, workspaceFolder.uri.fsPath], | ||
| workspaceFolder.uri.fsPath | ||
| ); | ||
| const result = envelope.data ?? { valid: true }; | ||
| currentPanel?.webview.postMessage({ | ||
| type: "name-validation-result", | ||
| payload: result, | ||
| }); |
| """Raise ValueError if any rule has an empty or whitespace-only pattern.""" | ||
| for key in VALID_OBJECT_TYPES: | ||
| rule = config.get_rule(key) | ||
| if rule is not None and (not rule.pattern or not rule.pattern.strip()): | ||
| raise ValueError(f"Naming rule for '{key}' has an empty pattern. Pattern is required.") | ||
|
|
||
|
|
| Args: | ||
| name: The object name to validate. | ||
| object_type: One of ``catalog``, ``schema``, ``table``, ``view``, ``column``. | ||
| workspace: Path to the ``.schemax/`` project directory. |
| <button | ||
| type="button" | ||
| className="collapsible-section__header" | ||
| onClick={handleToggle} | ||
| aria-expanded={open} | ||
| aria-controls={`collapsible-${title.replace(/\s+/g, "-")}`} | ||
| > | ||
| <i | ||
| className={`codicon ${open ? "codicon-chevron-down" : "codicon-chevron-right"}`} | ||
| aria-hidden="true" | ||
| /> | ||
| <h3 className="collapsible-section__title">{title}</h3> | ||
| </button> | ||
| {open && ( | ||
| <div id={`collapsible-${title.replace(/\s+/g, "-")}`} className="collapsible-section__body"> | ||
| {children} |
… components - Refactored the `validate.py` file to use a private alias for the `warnings` module, enhancing clarity. - Streamlined the `openDesigner` function in `extension.ts` by consolidating multi-line statements into single lines for better readability. - Simplified the `getTargetConfig` function in `storage-v4.ts` by removing unnecessary line breaks. - Enhanced the `PythonBackendClient` class by cleaning up the logging statements for command results. - Improved formatting in various components, including `ColumnGrid`, `NamingWarningModal`, and `ProjectSettingsPanel`, to maintain consistent code style. - Updated CSS transitions in `styles.css` for better visual effects. This commit focuses on enhancing code quality and consistency, making it easier to read and maintain.
|
|
||
| import json | ||
| import traceback | ||
| import warnings as _warnings |
There was a problem hiding this comment.
Unnecessary alias. import warnings as _warnings — there's no name collision with warnings in this module (the local ValidationError and other names don't conflict). The underscore prefix misleadingly suggests a private module-level name. Consider just import warnings.
There was a problem hiding this comment.
have to check this again.. The make command failed last time with collisions and hence had to change do aliasing.
| getDefaultTargetConfig: jest.fn(() => null), | ||
| })); | ||
|
|
||
| jest.mock('../../../src/webview/utils/useNameValidation', () => ({ |
There was a problem hiding this comment.
Naming validation behavior is entirely mocked out. The useNameValidation mock always returns { valid: true } and NamingWarningModal is stubbed to null, so none of the new add/rename naming flows are actually tested. The same applies to ColumnGrid.test.tsx.
This PR adds ~350 lines of async naming validation logic across Sidebar.tsx and ColumnGrid.tsx (strict mode blocking adds, warn-mode modals on rename, suggestion acceptance). The existing tests for these components cover add/rename/drop flows in detail — but with naming mocked away, there's no test that:
- Adding a non-compliant name in strict mode shows an error and blocks the add
- Adding a non-compliant name in warn mode shows the
NamingWarningModal - Accepting a suggestion from the modal calls
addCatalog/renameTablewith the suggested name - The "proceed anyway" path still performs the add/rename with the original name
NamingStandardsSettings.tsx (233 lines) and NamingWarningModal.tsx (63 lines) also have no component tests, breaking the pattern where every component in tests/webview/components/ has a corresponding test file.
| }); | ||
|
|
||
| test('runtime source tree has no direct PythonBackendClient.run workflow usage', () => { | ||
| test('runtime source tree has no direct PythonBackendClient.run workflow usage except extension.ts', () => { |
There was a problem hiding this comment.
Architecture fitness test weakened to accommodate a design inconsistency. This PR removes the assertion that extension.ts must not use pythonBackend.run() (was line 94 on main) and exempts extension.ts from the global "no direct .run() outside backend/" check here. Both constraints existed on main — both are relaxed.
The reason: the naming apply call in extension.ts uses pythonBackend.run() instead of runJson():
const applyResult = await pythonBackend.run(
["naming", "apply", "--json", namingJson, workspaceFolder.uri.fsPath],
workspaceFolder.uri.fsPath
);Every other CLI interaction in extension.ts (import, sql, validate-name, runtime-info, snapshot.validate, workspace-state) goes through runJson with typed envelopes. validate-name in this same PR correctly uses runJson. The naming apply command is the only one that doesn't.
Rather than weakening the fitness test, naming apply should support --json envelope output and be called via runJson like everything else. This keeps the architectural constraint intact and gives the extension typed error handling instead of parsing stderr.
|
Every other command ( At minimum:
|
|
|
||
| const validate = useCallback( | ||
| (name: string, objectType: string): Promise<NameValidationResult> => { | ||
| setPending(true); |
There was a problem hiding this comment.
Per-action subprocess spawn for single-name validation. Each call here triggers a round-trip through the extension host to pythonBackend.runJson(["validate", "--naming", ...]), which spawns a new Python process, imports the schemax module tree, reads project.json from disk, compiles the regex, runs re.fullmatch() on one string, and exits. This happens on every add and every rename in the UI.
I understand this follows the Python-first architecture — all business logic lives in Python, the webview is a display layer. That's the right call for batch validation during validate/sql/apply. But for single-name checks in an interactive UI loop, the cold process start (~200-500ms) creates a noticeable lag (the button shows "Checking..." while it waits).
The webview already has the naming config in memory via project.settings.namingStandards in the Zustand store, including the regex patterns. Consider validating locally in the webview using new RegExp(pattern).test(name) for instant interactive feedback, while keeping Python as the authoritative validator during validate/sql/apply. Python still owns the logic and the patterns — the webview just caches what Python already computed. This is similar to how LSP architectures work: the server sends diagnostic config, the client applies it locally for responsiveness.
| resolve(event.data.payload as NameValidationResult); | ||
| } | ||
| }; | ||
| window.addEventListener("message", handler); |
There was a problem hiding this comment.
Message listener leak and no timeout. If the Python process hangs or the extension host crashes, this Promise never resolves — the addEventListener handler is never cleaned up, and pending stays true forever, leaving the "Add" button permanently disabled at "Checking...". Consider adding a timeout (e.g. 5s) that rejects the promise and removes the listener.
Also, concurrent validations can race: the handler matches on event.data?.type === "name-validation-result" with no correlation ID. If two validations fire before the first returns (e.g. rapid rename), the first response resolves whichever Promise registered its listener first — which may not correspond to the right request. The second listener then waits for a response that was already consumed. A simple requestId in the payload would fix this.
- Removed the CLAUDE.md and plan.md files as they are no longer needed for project documentation. - Introduced a new command `naming templates` in the CLI to list built-in preset ids and descriptions for naming standards. - Updated the `naming apply` command to `naming set-config`, allowing users to set a full naming configuration from JSON or stdin. - Enhanced the `naming` command group documentation to reflect the new `templates` command and updated `set-config` functionality. - Improved the `validate_name` command to utilize the updated naming standards configuration and provide better validation feedback. - Added tests for the new `naming templates` command and ensured that preset descriptions match the defined presets. This commit focuses on refining the naming standards management process, enhancing user experience through improved CLI commands and documentation.
Summary
Motivation / Context
Type of Change
What Changed
Provider / Surface Area Impact
Affected Surfaces
Providers
Behavior Changes
Before
After
Examples
# CLI examples if relevantBackward Compatibility
Breaking Change Details (if any)
Data / State / Migration Notes
Details:
Testing
Automated
make allpassedManual
Test Evidence
Observability / UX
Security / Privacy / Compliance
Details:
Performance Considerations
Details:
Risks & Mitigations
Follow-ups / Out of Scope
Checklist
Reviewer Notes