Skip to content

Conversation

@DrBarnabus
Copy link
Owner

@DrBarnabus DrBarnabus commented Nov 16, 2025

Adds two new methods for writing key strings;

  • ToPartitionKeyString(int throughPartIndex, bool includeTrailingDelimiter = true) in IPrimaryKey<T>
  • ToSortKeyString(int throughPartIndex, bool includeTrailingDelimiter = true) in ICompositePrimaryKey<T>

These methods enable formatting keys up to a specific index, with optional trailing delimiters. This is useful for hierarchical queries and prefix matching in databases like DynamoDB.

The throughPartIndex parameter is zero-based and inclusive, counting only properties and constants (not delimiters). For example, with a key template "{Country}#{County}#{Locality}" and values "UK", "Derbyshire", "Matlock":

  • ToPartitionKeyString(0, false) returns "UK"
  • ToPartitionKeyString(0, true) returns "UK#"
  • ToPartitionKeyString(1, false) returns "UK#Derbyshire"
  • ToPartitionKeyString(1, true) returns "UK#Derbyshire#"

Summary by CodeRabbit

  • New Features

    • Added new method overloads for partition and sort key formatting with support for dynamic through-part formatting and optional trailing delimiter inclusion.
  • Tests

    • Added comprehensive test coverage for the new formatting method overloads, including edge cases and error handling scenarios.

Adds two new methods for writing key strings;
- `ToPartitionKeyString(int throughPartIndex, bool includeTrailingDelimiter = true)`
in `IPrimaryKey<T>`
- `ToSortKeyString(int throughPartIndex, bool
includeTrailingDelimiter = true)` in `ICompositePrimaryKey<T>`

These methods enable formatting keys up to a specific index, with optional
trailing delimiters. This is useful for hierarchical queries and prefix
matching in databases like DynamoDB.

The `throughPartIndex` parameter is zero-based and inclusive, counting only
properties and constants (not delimiters). For example, with a key template
`"{Country}#{County}#{Locality}"` and values "UK", "Derbyshire", "Matlock":
- `ToPartitionKeyString(0, false)` returns `"UK"`
- `ToPartitionKeyString(0, true)` returns `"UK#"`
- `ToPartitionKeyString(1, false)` returns `"UK#Derbyshire"`
- `ToPartitionKeyString(1, true)` returns `"UK#Derbyshire#"`
@coderabbitai
Copy link

coderabbitai bot commented Nov 16, 2025

Walkthrough

This pull request introduces new method overloads to the primary and composite primary key interfaces, enabling dynamic formatting of partition and sort keys up to a specified part index with optional trailing delimiters. The source generator is updated to emit these new overload implementations, and comprehensive test coverage is added across multiple test files. The public API surface is updated accordingly.

Changes

Cohort / File(s) Summary
Interface Extensions
src/CompositeKey/IPrimaryKey1.cs, src/CompositeKey/ICompositePrimaryKey1.cs
Adds new overload methods ToPartitionKeyString(int throughPartIndex, bool includeTrailingDelimiter = true) and ToSortKeyString(int throughPartIndex, bool includeTrailingDelimiter = true) to enable formatted key generation up to specified part indices with optional trailing delimiters. Includes XML documentation and usage examples.
Source Generator Implementation
src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs
Adds dynamic method overloads emission for ToPartitionKeyString and ToSortKeyString in both primary and composite key paths. Introduces private helpers WriteDynamicFormatMethodBodyForKeyParts and BuildFormatStringForKeyParts to support switch-based dynamic formatting. Refactors existing static formatting logic to reuse the new BuildFormatStringForKeyParts helper.
Test Coverage
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
Adds four parameterized test methods: two validating correct formatting for ToPartitionKeyString and ToSortKeyString with specific part indices and delimiter options; two validating exception throwing for invalid part index or delimiter combinations.
Test Coverage
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs
Adds two parameterized test methods for PrimaryKeyWithFastPathFormatting.ToPartitionKeyString: one validating correct formatting with specific part indices and delimiters; one validating exception behaviour for invalid combinations.
Public API Surface
src/CompositeKey/PublicAPI.Unshipped.txt
Registers two new public method signatures: ICompositePrimaryKey<TSelf>.ToSortKeyString(int, bool) and IPrimaryKey<TSelf>.ToPartitionKeyString(int, bool).

Sequence Diagram

sequenceDiagram
    participant Caller
    participant GeneratedClass
    participant FormatLogic

    Caller->>GeneratedClass: ToPartitionKeyString(throughPartIndex, includeTrailingDelimiter)
    activate GeneratedClass
    
    Note over GeneratedClass: Validate throughPartIndex
    
    alt Valid throughPartIndex
        GeneratedClass->>FormatLogic: BuildFormatStringForKeyParts(throughPartIndex)
        activate FormatLogic
        FormatLogic->>FormatLogic: Switch on index & assemble<br/>format string from parts
        FormatLogic-->>GeneratedClass: format string
        deactivate FormatLogic
        
        GeneratedClass->>FormatLogic: Format key parts using<br/>constructed format string
        activate FormatLogic
        Note over FormatLogic: Handle trailing delimiter
        FormatLogic-->>GeneratedClass: formatted string
        deactivate FormatLogic
        
        GeneratedClass-->>Caller: return formatted key
    else Invalid throughPartIndex
        GeneratedClass-->>Caller: throw InvalidOperationException
    end
    
    deactivate GeneratedClass
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Source generator emitter logic: The dynamic formatting via switch statements and helper methods introduces new control flow that requires careful review to ensure correct format string construction and part selection across both primary and composite key paths.
  • Test parameterisation: Verify that all test cases cover the boundary conditions properly (e.g., throughPartIndex=0, 1, 2, and invalid indices).
  • Interface consistency: Confirm that both IPrimaryKey<TSelf> and ICompositePrimaryKey<TSelf> method signatures and documentation remain aligned with their implementations in the emitter.

Poem

🐰 With indices dancing and delimiters neat,
Key formatting now dynamic and sweet,
Through parts we can parse with precision so bright,
Each partition and sort key aligned just right!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: Add partial key formatting methods' accurately describes the main change: adding new methods for partial/dynamic key formatting with specific part index control.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch partial-format-methods

Tip

📝 Customizable high-level summaries are now available!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide custom instructions to shape the summary (bullet lists, tables, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example:

"Create a concise high-level summary as a bullet-point list. Then include a Markdown table showing lines added and removed by each contributing author."


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.67%. Comparing base (7bbc92b) to head (c6b6b89).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...iteKey.SourceGeneration/SourceGenerator.Emitter.cs 93.54% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   91.55%   91.67%   +0.12%     
==========================================
  Files          34       34              
  Lines        1503     1525      +22     
  Branches      246      249       +3     
==========================================
+ Hits         1376     1398      +22     
- Misses         59       60       +1     
+ Partials       68       67       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (7)
src/CompositeKey/IPrimaryKey`1.cs (1)

23-37: Document exception behaviour for invalid index/delimiter combinations

The overload and example read clearly, but the contract doesn’t mention that certain throughPartIndex/includeTrailingDelimiter combinations are invalid and will throw (per the tests and generated implementation). It would be helpful to explicitly document that an InvalidOperationException is thrown when the index is out of range for the available key parts, or when includeTrailingDelimiter is true but there is no following delimiter for that part. This will make the behaviour of the new overload predictable for consumers.

src/CompositeKey/ICompositePrimaryKey`1.cs (1)

17-31: Align sort-key overload docs with actual exception behaviour

This overload mirrors the partition-key overload, including the same implicit constraints on throughPartIndex and includeTrailingDelimiter. As with the partition key, it would be good to call out that invalid combinations (e.g. index outside the available sort-key parts, or includeTrailingDelimiter == true when there is no following delimiter) result in an InvalidOperationException, so callers know what to expect.

src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (2)

278-319: Tests correctly pin partial formatting semantics (minor naming nit)

The parametrised test nicely exercises all valid combinations of throughPartIndex/includeTrailingDelimiter for PrimaryKeyWithFastPathFormatting and matches the intended behaviour of counting only value/constant parts and optionally including the following delimiter. One small readability nit: the parameter name compositeKey is misleading for a primary key; renaming it to primaryKey would better reflect the type and avoid confusion with the composite-key tests.


321-329: Invalid-combination test is good; consider a negative-index case

The invalid-case test asserting InvalidOperationException for (3, true) and (4, false) is a good guard for “too-far” and “no trailing delimiter” scenarios. If you want to be exhaustive, you could add a negative throughPartIndex case (e.g. -1) to document that such inputs are also rejected in the same way, but this is optional given the current coverage.

src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (1)

54-55: Dynamic partial-format generation matches the intended semantics (with a small edge-case to consider)

The new WriteDynamicFormatMethodBodyForKeyParts plus its use in both primary and composite key paths looks solid:

  • throughPartIndex is derived by incrementing keyPartIndex only for non-delimiter parts, so it counts exactly properties/constants as documented.
  • For each value/constant part N, you emit (N, false) (up to that part) and, if a delimiter immediately follows, (N, true) including that delimiter; this lines up with the tests’ expectations and the interface comments.
  • The default arm throwing InvalidOperationException cleanly handles out-of-range indices and unsupported delimiter combinations, and the message is explicit enough.

Centralising format-string construction in BuildFormatStringForKeyParts (and reusing it from WriteFormatMethodBodyForKeyParts) also removes duplication and guarantees that full-key and partial-key paths stay consistent.

One edge case to be aware of: if a key template ever produced consecutive DelimiterKeyParts with no value/constant between them, this loop would emit multiple switch cases with the same (throughPartIndex, true) pattern but different format strings, causing a compile-time “duplicate pattern” error in the generated code. If such templates are impossible or already rejected by your analyzers, you’re fine; otherwise, you may want to either disallow them at validation time or skip additional delimiter-only cases for the same keyPartIndex.

Also applies to: 140-145, 576-580, 588-617, 619-635

src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (2)

67-75: Consider adding test coverage for negative indices.

The boundary tests appropriately validate that trailing delimiters at the last index and out-of-bounds indices throw InvalidOperationException. However, consider adding test cases for negative throughPartIndex values (e.g., -1) to ensure they're also handled correctly.

Example test case to add:

[Theory, AutoData]
public static void CompositePrimaryKey_ToPartitionKeyString_WithNegativePartIndex_ShouldThrowInvalidOperationException(
    CompositePrimaryKey compositeKey)
{
    var act = () => compositeKey.ToPartitionKeyString(-1, false);
    act.ShouldThrow<InvalidOperationException>();
}

123-131: Consider adding test coverage for negative indices.

Similar to the partition key tests, consider adding test cases for negative throughPartIndex values to ensure complete boundary validation.

Example test case to add:

[Theory, AutoData]
public static void CompositePrimaryKey_ToSortKeyString_WithNegativePartIndex_ShouldThrowInvalidOperationException(
    CompositePrimaryKey compositeKey)
{
    var act = () => compositeKey.ToSortKeyString(-1, false);
    act.ShouldThrow<InvalidOperationException>();
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7bbc92b and c6b6b89.

📒 Files selected for processing (6)
  • src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (2 hunks)
  • src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (1 hunks)
  • src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (6 hunks)
  • src/CompositeKey/ICompositePrimaryKey1.cs` (1 hunks)
  • src/CompositeKey/IPrimaryKey1.cs` (1 hunks)
  • src/CompositeKey/PublicAPI.Unshipped.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-28T19:57:32.503Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.503Z
Learning: In CompositeKey projects, test files use global usings configuration (likely through ImplicitUsings or similar mechanisms) for common imports like xUnit and Shouldly, eliminating the need for explicit using directives in individual test files.

Applied to files:

  • src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs
  • src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
📚 Learning: 2025-08-28T19:57:32.503Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.503Z
Learning: In CompositeKey projects, test files use global usings (typically in GlobalUsings.cs files) for common imports like xUnit and Shouldly, so individual test files don't need explicit using directives for these frameworks.

Applied to files:

  • src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs
  • src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
📚 Learning: 2025-08-28T19:57:32.503Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.503Z
Learning: In CompositeKey projects, test files use global usings (typically in GlobalUsings.cs files or via ImplicitUsings in project files) for common imports like xUnit and Shouldly, so individual test files don't need explicit using directives for these frameworks.

Applied to files:

  • src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs
  • src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
📚 Learning: 2025-08-28T19:57:32.504Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.504Z
Learning: CompositeKey test projects use MSBuild's <Using Include="..." /> elements in .csproj files combined with <ImplicitUsings>enable</ImplicitUsings> in Directory.Build.props to automatically include Xunit and Shouldly namespaces, eliminating the need for explicit using statements in test files.

Applied to files:

  • src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
🧬 Code graph analysis (3)
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (2)
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (16)
  • Theory (9-16)
  • Theory (18-27)
  • Theory (29-38)
  • Theory (40-65)
  • Theory (67-75)
  • Theory (77-86)
  • Theory (88-121)
  • Theory (123-131)
  • Theory (133-145)
  • Theory (147-152)
  • Theory (154-166)
  • Theory (168-173)
  • Theory (175-187)
  • Theory (189-195)
  • Theory (197-209)
  • Theory (211-217)
src/CompositeKey/IPrimaryKey`1.cs (2)
  • ToPartitionKeyString (21-21)
  • ToPartitionKeyString (37-37)
src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (1)
src/CompositeKey.SourceGeneration/Core/SourceWriter.cs (5)
  • SourceWriter (7-120)
  • StartBlock (31-38)
  • WriteLine (46-46)
  • WriteLine (48-52)
  • EndBlock (40-44)
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (3)
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (16)
  • Theory (9-16)
  • Theory (18-25)
  • Theory (27-34)
  • Theory (36-44)
  • Theory (46-51)
  • Theory (53-61)
  • Theory (63-69)
  • Theory (85-92)
  • Theory (94-101)
  • Theory (103-110)
  • Theory (112-123)
  • Theory (125-130)
  • Theory (132-143)
  • Theory (145-151)
  • Theory (171-178)
  • Theory (180-187)
src/CompositeKey/IPrimaryKey`1.cs (2)
  • ToPartitionKeyString (21-21)
  • ToPartitionKeyString (37-37)
src/CompositeKey/ICompositePrimaryKey`1.cs (2)
  • ToSortKeyString (15-15)
  • ToSortKeyString (31-31)
🔇 Additional comments (5)
src/CompositeKey/PublicAPI.Unshipped.txt (1)

2-3: API surface entries match interface contracts

The new PublicAPI entries for ToSortKeyString(int, bool) and ToPartitionKeyString(int, bool) (with includeTrailingDelimiter = true) match the interface signatures and intended behaviour, so the public surface looks consistent.

src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (2)

407-415: Nice clean-up of unused pattern variable

Changing the ParseType.String case pattern to drop the unused part variable simplifies the match without altering behaviour and removes potential compiler warnings.


665-670: Comment and attribute emission remain clear and accurate

The updated comment around GeneratedCodeAttribute and the emitted attribute itself look correct and keep the intent of the generated class annotation clear.

src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (2)

40-65: LGTM! Comprehensive test coverage for partition key formatting.

The test correctly validates partial partition key formatting with different parameter combinations. The switch statement properly handles all tested scenarios, and the expected string formats align with the template {GuidValue}#{DecimalValue}.


88-121: LGTM! Thorough test coverage for sort key formatting.

The test comprehensively validates partial sort key formatting, including testing middle parts with both delimiter options. All expected string formats correctly match the template Constant~{EnumValue}@{StringValue}, and the switch statement properly handles all tested scenarios.

@DrBarnabus DrBarnabus merged commit 42f9674 into main Nov 16, 2025
4 checks passed
@DrBarnabus DrBarnabus deleted the partial-format-methods branch November 16, 2025 16:51
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