Skip to content

Conversation

@quettabit
Copy link
Member

No description provided.

@quettabit
Copy link
Member Author

@greptileai review this draft

@greptile-apps
Copy link

greptile-apps bot commented Jan 4, 2026

Greptile Summary

This PR refactors the SDK's public API to prevent misuse by restricting direct field access and improving type safety.

Key Changes:

  • Made configuration fields private: S2Config, BatchingConfig, AppendSessionConfig, and ProducerConfig fields are now private, enforcing builder pattern usage
  • Separated input/output types: Renamed AccessTokenScope to AccessTokenScopeInput for API inputs, and created a separate read-only AccessTokenScope type for outputs
  • Introduced S2DateTime wrapper: Replaced direct time::OffsetDateTime usage with S2DateTime to control the public API surface
  • Replaced boolean with enum: Changed auto_prefix_streams: Option<bool> to AutoPrefixingMode enum (Disabled | Enabled(prefix)), making the API more type-safe and explicit
  • Improved type safety: Changed AppendSessionConfig::with_max_inflight_batches parameter from u32 to NonZeroU32, preventing invalid zero values at compile time
  • Enhanced documentation: Fixed intra-doc links throughout the codebase to use proper Rust doc link syntax
  • Added #[non_exhaustive]: Marked IndexedAppendAck and DeleteOnEmptyConfig as non-exhaustive to allow future evolution
  • Hidden internal features: Added _hidden feature flag for internal-only methods like with_user_agent

Breaking Changes: This is correctly marked as a breaking change (refactor!) as it modifies the public API significantly. Users upgrading will need to:

  • Use builder methods instead of direct field access
  • Replace AccessTokenScope with AccessTokenScopeInput when creating inputs
  • Use S2DateTime instead of time::OffsetDateTime
  • Replace with_auto_prefix_streams(bool) with with_auto_prefixing_mode(AutoPrefixingMode::Enabled(prefix))
  • Pass NonZeroU32 to with_max_inflight_batches

All tests and examples have been updated to demonstrate the new patterns.

Confidence Score: 5/5

  • This PR is safe to merge with high confidence
  • The refactoring is well-executed with comprehensive test coverage. All breaking changes are intentional, properly documented, and the PR is correctly marked with refactor! to signal breaking changes. The changes improve API safety through type restrictions, prevent misuse through encapsulation, and all tests have been updated to verify the new patterns work correctly
  • No files require special attention

Important Files Changed

Filename Overview
src/types.rs Made multiple struct fields private, introduced S2DateTime wrapper type, renamed AccessTokenScope to AccessTokenScopeInput for inputs and created new AccessTokenScope for outputs, replaced auto_prefix_streams with AutoPrefixingMode enum, improved documentation formatting
src/batching.rs Made BatchingConfig fields private with getters now available through builder methods, improved documentation and error messages
src/producer.rs Made ProducerConfig fields private, added #[non_exhaustive] to IndexedAppendAck, improved documentation
src/session/append.rs Made AppendSessionConfig fields private, changed with_max_inflight_batches parameter to NonZeroU32 for type safety
tests/account_ops.rs Updated tests to use new API: AccessTokenScopeInput instead of AccessTokenScope, S2DateTime type, AutoPrefixingMode::Enabled(), and added struct update syntax .. for DeleteOnEmptyConfig
examples/issue_access_token.rs Updated to use AccessTokenScopeInput instead of AccessTokenScope

Sequence Diagram

sequenceDiagram
    participant User
    participant S2Config
    participant S2
    participant AccessTokenScopeInput
    participant IssueAccessTokenInput
    participant BatchingConfig
    participant AppendSessionConfig

    Note over S2Config: Field visibility changes
    User->>S2Config: new(access_token)
    Note over S2Config: Fields now private:<br/>access_token, endpoints,<br/>connection_timeout, etc.
    S2Config-->>User: S2Config instance
    
    Note over AccessTokenScopeInput: Renamed from AccessTokenScope
    User->>AccessTokenScopeInput: from_op_group_perms(perms)
    Note over AccessTokenScopeInput: Fields now private:<br/>basins, streams,<br/>access_tokens, ops
    AccessTokenScopeInput-->>User: AccessTokenScopeInput instance
    
    User->>IssueAccessTokenInput: new(id, scope)
    Note over IssueAccessTokenInput: auto_prefix_streams replaced<br/>with AutoPrefixingMode enum
    IssueAccessTokenInput-->>User: IssueAccessTokenInput
    User->>IssueAccessTokenInput: with_auto_prefixing_mode(mode)
    IssueAccessTokenInput-->>User: Updated input
    
    User->>S2: issue_access_token(input)
    S2-->>User: AccessTokenScope (output type)
    Note over User: Separate input/output types<br/>prevent misuse
    
    Note over BatchingConfig: Configuration safety
    User->>BatchingConfig: new()
    BatchingConfig-->>User: Default config
    User->>BatchingConfig: with_max_batch_bytes(size)
    Note over BatchingConfig: Fields now private,<br/>accessed through builders
    BatchingConfig-->>User: Validated config
    
    Note over AppendSessionConfig: Type safety improvements
    User->>AppendSessionConfig: with_max_inflight_batches(NonZeroU32)
    Note over AppendSessionConfig: NonZeroU32 prevents invalid values
    AppendSessionConfig-->>User: Validated config
Loading

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.

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@quettabit quettabit changed the title [WIP] refactor!: restrict public items for misuse safety Jan 4, 2026
@quettabit quettabit marked this pull request as ready for review January 4, 2026 02:12
@quettabit quettabit requested a review from a team as a code owner January 4, 2026 02:12
@quettabit quettabit merged commit bac1da0 into main Jan 4, 2026
4 checks passed
@quettabit quettabit deleted the qb/revisit-types branch January 4, 2026 02:13
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