Skip to content

Conversation

@dmarek-flex
Copy link
Contributor

@dmarek-flex dmarek-flex commented Oct 22, 2025

Greptile Overview

Updated On: 2025-10-22 19:15:24 UTC

Greptile Summary

This PR adds validation for the run_only field in component modelers by checking for duplicate entries and verifying that all specified indices correspond to valid port/mode combinations. The implementation extracts matrix indices construction logic into a new abstract static method _construct_matrix_indices_monitor in the base class, which is then implemented by both ModalComponentModeler and TerminalComponentModeler. A pydantic validator _validate_run_only in AbstractComponentModeler uses this method to validate user-provided run_only values during object construction, before cached properties are initialized. The refactoring maintains backward compatibility while enabling early error detection with clear error messages.

Changed Files
Filename Score Overview
tidy3d/plugins/smatrix/component_modelers/base.py 4/5 Added _validate_run_only validator and abstract static method _construct_matrix_indices_monitor to enable validation of run_only field for duplicate and invalid indices
tidy3d/plugins/smatrix/component_modelers/modal.py 5/5 Extracted matrix indices construction into static method _construct_matrix_indices_monitor implementing base class abstract method, delegated from cached property
tidy3d/plugins/smatrix/component_modelers/terminal.py 5/5 Refactored matrix_indices_monitor to use new static method _construct_matrix_indices_monitor that handles both LumpedPort and WavePort types
tests/test_plugins/smatrix/test_component_modeler.py 5/5 Added two test functions validating run_only uniqueness and membership constraints for ModalComponentModeler
tests/test_plugins/smatrix/test_terminal_component_modeler.py 5/5 Added three comprehensive test functions covering run_only validation for duplicates, invalid indices, and WavePort compatibility in TerminalComponentModeler
CHANGELOG.md 4/5 Added changelog entry documenting new validation feature for run_only field under "Added" category

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as the validation logic is well-tested and maintains backward compatibility
  • Score reflects solid implementation with comprehensive tests, though the changelog entry could be categorized as "Changed" rather than "Added" since it improves existing functionality, and there's minor concern about the interaction between @staticmethod and @abstractmethod decorators in the validation context
  • Pay close attention to tidy3d/plugins/smatrix/component_modelers/base.py to verify the abstract static method is correctly accessible during pydantic validation

Sequence Diagram

sequenceDiagram
    participant User
    participant AbstractComponentModeler
    participant Validator
    participant HelperMethod
    
    User->>AbstractComponentModeler: Create/update modeler with run_only
    AbstractComponentModeler->>Validator: @pd.validator("run_only")
    
    alt run_only is None
        Validator->>AbstractComponentModeler: return val (skip validation)
    else run_only has value
        Validator->>Validator: Check for duplicates
        alt duplicates found
            Validator->>User: raise SetupError("duplicate entries")
        end
        
        Validator->>Validator: Get ports from values
        alt ports is None
            Validator->>AbstractComponentModeler: return val
        else ports exist
            Validator->>HelperMethod: _construct_matrix_indices_monitor(ports)
            HelperMethod->>Validator: return valid_indices set
            Validator->>Validator: Check membership of run_only indices
            alt invalid indices found
                Validator->>User: raise SetupError("not present in matrix_indices_monitor")
            else all indices valid
                Validator->>AbstractComponentModeler: return val
            end
        end
    end
    
    AbstractComponentModeler->>User: Validation complete
Loading

Context used:

  • Rule from dashboard - Use changelog categories correctly: "Fixed" for bug fixes, "Changed" for modifications to existing f... (source)

@dmarek-flex dmarek-flex self-assigned this Oct 22, 2025
@dmarek-flex dmarek-flex changed the title fix: improve validation 'run_only' field in component modelers FXC-3800 fix: improve validation 'run_only' field in component modelers Oct 22, 2025
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/plugins/smatrix/component_modelers/base.py (100%)
  • tidy3d/plugins/smatrix/component_modelers/modal.py (100%)
  • tidy3d/plugins/smatrix/component_modelers/terminal.py (100%)

Summary

  • Total: 33 lines
  • Missing: 0 lines
  • Coverage: 100%

@dmarek-flex dmarek-flex force-pushed the dmarek/FXC-3800-validate-run-only branch from c0b3c62 to 1380398 Compare October 23, 2025 03:05
@dmarek-flex dmarek-flex enabled auto-merge October 23, 2025 03:06
@dmarek-flex dmarek-flex force-pushed the dmarek/FXC-3800-validate-run-only branch from 1380398 to ed0214d Compare October 23, 2025 13:06
@dmarek-flex dmarek-flex added this pull request to the merge queue Oct 23, 2025
Merged via the queue into develop with commit 95a15af Oct 23, 2025
25 checks passed
@dmarek-flex dmarek-flex deleted the dmarek/FXC-3800-validate-run-only branch October 23, 2025 14:15
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.

3 participants