Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 7, 2025

This PR implements arbitrary layers functionality to generalize the current hardcoded prepend/normal/append layer system, addressing the requirements in the issue.

New Features

Arbitrary Layer Configuration

  • CLI Options: Added --layers and --fallback-layer options for runtime configuration
  • Custom Ordering: Layers are processed in the order specified, enforcing dependency constraints
  • Default Configuration: Maintains existing behavior with ["prepend", "normal", "append"] layers and "normal" fallback

New Header Format

Added support for the new -- layer: <layer_name> header format:

-- name: my_function
-- layer: business_logic
-- requires: my_schema

CREATE FUNCTION my_function() ...

Layer Ordering Enforcement

The system now enforces that nodes in lower-index layers cannot depend on nodes in higher-index layers:

# This will fail with a clear error message
-- name: early_layer_file
-- layer: first  
-- requires: late_layer_file  # Error: first layer cannot depend on second layer

Backward Compatibility

The implementation maintains complete backward compatibility:

  • ✅ Legacy -- is_initial maps to "prepend" layer
  • ✅ Legacy -- is_final maps to "append" layer
  • ✅ Files without layer declarations use the configured fallback layer
  • ✅ All existing CLI options and behavior preserved
  • ✅ No breaking changes to existing functionality

Usage Examples

Custom Layers

topcat -i src/ -o output.sql --layers="setup,core,finalization" --fallback-layer="core"

Mixed Legacy and New Syntax

-- Old syntax still works
-- name: legacy_prepend
-- is_initial

-- New syntax provides more flexibility  
-- name: custom_layer_file
-- layer: middleware

Technical Implementation

  • Dynamic Graph System: Replaced hardcoded 3-graph system with HashMap<String, DiGraph<FileNode, ()>>
  • Layer Index Validation: Dependencies are validated using layer indices to enforce ordering constraints
  • Enhanced Error Messages: Clear, descriptive error messages for layer-related validation failures
  • Comprehensive Testing: Added unit tests covering all new functionality and edge cases

Testing

  • ✅ All existing tests continue to pass
  • ✅ New unit tests for layer parsing, validation, and error handling
  • ✅ Integration tests with real SQL files demonstrating functionality
  • ✅ Backward compatibility verified with existing test data

The implementation successfully delivers all requirements while maintaining the stability and usability of the existing system.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits July 7, 2025 18:04
Co-authored-by: joshainglis <1281131+joshainglis@users.noreply.github.com>
Co-authored-by: joshainglis <1281131+joshainglis@users.noreply.github.com>
Copilot AI changed the title [WIP] Add functionality to declare arbitrary layers, essentially generalizing the current prepend, normal append layers. It should enforce that a node in a layer with a lower index cannot depend on a node in a layer with a higher index. These layers should be declared either in a config file or via cli options as a list of names. A fallback layer must be defined in the config for when a node does not explicitly declare a layer. The header format should be -- layer: <layer_name>. If not declared explicitly, the default layers will be ["prepend", "normal", "append"] with "normal" as the fallback layer. Add support for arbitrary layers with configurable ordering and header format Jul 7, 2025
Copilot AI requested a review from joshainglis July 7, 2025 18:10
@joshainglis
Copy link
Owner

Looks like cargo fmt -- --check returned a non-zero exit status. Please run cargo fmt and check that cargo fmt -- --check passes on your changes

Co-authored-by: joshainglis <1281131+joshainglis@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 7, 2025

Looks like cargo fmt -- --check returned a non-zero exit status. Please run cargo fmt and check that cargo fmt -- --check passes on your changes

Fixed the formatting issues by running cargo fmt. The code now passes cargo fmt -- --check and all tests continue to pass. Changes committed in fb49670.

Copilot AI requested a review from joshainglis July 7, 2025 18:24
@joshainglis joshainglis marked this pull request as ready for review July 7, 2025 18:26
@joshainglis joshainglis merged commit f5952ee into main Jul 7, 2025
5 checks passed
@joshainglis joshainglis deleted the copilot/fix-193e0f20-9b8a-4495-9660-e19579823991 branch July 7, 2025 18:27
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.

2 participants