Grant expansion: Remove expandable annotation after processing expandable grant.

[ggreer]

When compacting syncs, we spend a lot of time expanding grants that have already been expanded. So now we remove the expandable annotation after putting the grant in the graph. Also improved the grant expand test to validate that we get the correct number of grants and that expandable annotations are removed.

This annotation is only used by the expansion code. Nothing else checks whether it exists, so it's safe to delete.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved tracking of grant expansion state to prevent redundant expansion operations.

• Performance

  • Optimized batch processing of grants for more efficient handling of large-scale synchronization operations.

• Tests

  • Enhanced test coverage and validation for grant expansion and synchronization scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This pull request refactors grant expansion persistence by extracting putGrantsInChunks into a public package-level function in the expand package, then introduces batched processing in the syncer to collect grants with expandable annotations, strip those annotations, and persist them in chunks during the sync operation.

Changes

Cohort / File(s)	Summary
Expander function refactoring pkg/sync/expand/expander.go	Promotes private method putGrantsInChunks to public package-level function PutGrantsInChunks with added store ExpanderStore parameter; updates all callsites and error messages to reflect new signature and location.
Syncer grant batching pkg/sync/syncer.go	Integrates batched grant processing after entitlement graph building: collects grants with GrantExpandable annotations, removes the annotation, updates grant records, and persists via expand.PutGrantsInChunks with batch size 10000; adds final flush with size 0.
Syncer test updates pkg/sync/syncer_test.go	Replaces manual context creation with test context; introduces grantData type; updates expansion logic to build expected grant count incrementally; adds validation for grant annotation state post-sync; expands external resource matching tests; refactors loops to range-based constructs.

Sequence Diagram

sequenceDiagram
    participant Syncer as Syncer
    participant Graph as Graph Builder
    participant Store as ExpanderStore
    participant Batch as PutGrantsInChunks

    Syncer->>Graph: Build entitlement graph
    Graph-->>Syncer: Graph complete

    loop For each fetched grant
        Syncer->>Syncer: Check for GrantExpandable annotation
        alt Has GrantExpandable
            Syncer->>Syncer: Remove annotation<br/>Update grant record
            Syncer->>Syncer: Accumulate in updatedGrants batch
        end
        
        alt Batch reaches size 10000
            Syncer->>Batch: PutGrantsInChunks(ctx, store, batch, 10000)
            Batch->>Store: PutGrants(ctx, grants...)
            Store-->>Batch: Success/Error
            Batch-->>Syncer: Updated grants
            Syncer->>Syncer: Reset batch
        end
    end

    alt Remaining grants in batch
        Syncer->>Batch: PutGrantsInChunks(ctx, store, remaining, 0)
        Batch->>Store: PutGrants(ctx, grants...)
        Store-->>Batch: Success/Error
        Batch-->>Syncer: Updated grants
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

• pkg/sync/syncer.go: The new batched grant processing logic with annotation removal and multiple PutGrantsInChunks calls; verify correct handling of batch boundaries, error propagation, and state consistency across batches
• pkg/sync/expand/expander.go: Ensure all callers of the refactored PutGrantsInChunks function properly pass the store parameter and that the function's behavior remains unchanged
• pkg/sync/syncer_test.go: Extensive test modifications including context handling changes, new validation assertions for grant annotation state, and test data setup; verify test logic remains correct after refactoring and that new assertions properly validate the batch processing behavior

Possibly related PRs

• Don't force checkpoint during grant expansion. #550: Modifies the grant-expansion persistence path in syncer.go to refactor and centralize batched PutGrants calls, sharing the same code-path for persisting expanded grants
• Refactor Expansion for Testability  #585: Modifies the putGrantsInChunks implementation in expander.go, directly related to the function extraction performed here
• Make expanded grants immutable. #278: Handles expanded grants' annotations during sync/expansion (marking grants with GrantImmutable), complementary to this PR's removal of GrantExpandable annotations

Suggested reviewers

• kans

---

🐰 In the garden of grants, we now batch with care,
Stripping expandable tags from the air,
Public functions dance, refactored with flair,
In chunks of ten thousand, persistence laid bare! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.88% 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
Title check	✅ Passed	The title clearly and specifically describes the main change: removing the expandable annotation from grants after processing, which aligns with the PR's core objective to avoid re-expanding grants during compacting syncs.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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 ggreer/remove-expand-annotation

---

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

[kans]

If we ever do implement a two pass system, we could handle this bit then.

[kans]

I wonder if we should capture that the grant was expandable but we already did the work.