Refine repository guidelines and replace symlinks with managed copies#8
Refine repository guidelines and replace symlinks with managed copies#8
Conversation
- Consolidated repository guidelines under the `IGenericRepository<T>` abstraction for consistency and maintainability. - Introduced examples emphasizing type-safe dependency injection and mock-friendly interfaces in services. - Updated TypeScript standards to enforce `Types` suffix for exported type aliases while reserving `I` prefixes for interfaces. - Streamlined repository-specific design principles and templates for clarity.
WalkthroughBoth documentation files undergo significant updates. The TypeScript coding standards documentation shifts to emphasise type-focused organisation with Types suffix files and introduces a generic repository pattern with examples. The repository pattern documentation transitions from per-aggregate-root bespoke repositories to a generic, interface-based approach applicable across languages. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as ProductService
participant DI as Dependency Injector
participant Repo as GenericRepository<br/>(IGenericRepository)
participant DB as IDatabaseService
Client->>Service: Create(productId)
activate Service
Service->>DI: Resolve IGenericRepository<IProduct>
DI->>Repo: Instantiate in constructor
Repo->>DB: Inject IDatabaseService
Service->>Repo: getById(productId)
activate Repo
Repo->>DB: Query collection
DB-->>Repo: Product entity
deactivate Repo
Repo-->>Service: IProduct | null
Service-->>Client: IProduct
deactivate Service
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR refines the repository pattern guidelines and TypeScript coding standards by consolidating around a generic IGenericRepository<T> pattern instead of per-entity repository classes. The changes introduce clearer conventions for type naming (Types suffix for type aliases) and demonstrate interface-centric instantiation where services own repository creation.
Key Changes:
- Added
Typessuffix requirement for exported type aliases (distinct fromIprefix for interfaces) - Refactored repository examples to use generic
IGenericRepository<T>with typed instances per aggregate - Updated service constructor injection pattern to instantiate repositories internally while depending on interface types
- Clarified that related interfaces/types can share a
*Types.tsfile
Issues Found:
- Constructor signature mismatch between
repository-pattern.mdexample (line 52-59) andcoding-standards.mdimplementation—the generic example uses an options object{ collection: 'products' }while the TypeScript implementation expects positional parameters('products', db)
Confidence Score: 3/5
- This PR is safe to merge after fixing the constructor signature inconsistency
- Documentation changes are well-structured and align with clean architecture principles. The single syntax error (mismatched constructor signatures between shared and TypeScript-specific examples) needs correction to prevent confusion, but won't cause runtime issues since these are documentation files. No other logical errors or security concerns identified.
- Pay attention to
templates/shared/rules/repository-pattern.mdline 52-59 to ensure the constructor example matches the TypeScript implementation
Sequence Diagram
sequenceDiagram
participant Service as ProductService
participant Repo as IGenericRepository
participant Impl as GenericRepository
participant DB as IDatabaseService
Note over Service: Constructor Injection
Service->>Impl: new GenericRepository(collection, db)
Impl-->>Service: store as IGenericRepository interface
Note over Service,DB: Runtime Data Access
Service->>Repo: getById(id)
Repo->>Impl: delegate to concrete
Impl->>DB: readDocDataAtPath(path)
DB-->>Impl: doc data
Impl-->>Repo: entity or null
Repo-->>Service: IProduct or null
Service->>Service: null check and throw if needed
Note over Service: Test Double Pattern
Service->>Service: replace field with mock
Service->>Repo: getById(id)
Repo-->>Service: mock entity
2 files reviewed, 1 comment
| ```typescript | ||
| export class ProductService { | ||
| private readonly products: IGenericRepository<IProduct>; | ||
|
|
||
| constructor() { | ||
| this.products = new GenericRepository<IProduct>({ collection: 'products' }); | ||
| } | ||
| } |
There was a problem hiding this comment.
syntax: The constructor signature here doesn't match the TypeScript implementation in coding-standards.md. This example passes { collection: 'products' } as a single options object, but coding-standards.md:489 shows new GenericRepository<IProduct>('products', db) with separate positional arguments (collection string and db dependency). The generic GenericRepository<T> constructor at coding-standards.md:426-429 confirms it expects (collection: string, db: IDatabaseService).
| ```typescript | |
| export class ProductService { | |
| private readonly products: IGenericRepository<IProduct>; | |
| constructor() { | |
| this.products = new GenericRepository<IProduct>({ collection: 'products' }); | |
| } | |
| } | |
| export class ProductService { | |
| private readonly products: IGenericRepository<IProduct>; | |
| constructor(db: IDatabaseService) { | |
| this.products = new GenericRepository<IProduct>('products', db); | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: templates/shared/rules/repository-pattern.md
Line: 52:59
Comment:
**syntax:** The constructor signature here doesn't match the TypeScript implementation in `coding-standards.md`. This example passes `{ collection: 'products' }` as a single options object, but `coding-standards.md:489` shows `new GenericRepository<IProduct>('products', db)` with separate positional arguments (`collection` string and `db` dependency). The generic `GenericRepository<T>` constructor at `coding-standards.md:426-429` confirms it expects `(collection: string, db: IDatabaseService)`.
```suggestion
export class ProductService {
private readonly products: IGenericRepository<IProduct>;
constructor(db: IDatabaseService) {
this.products = new GenericRepository<IProduct>('products', db);
}
}
```
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
templates/languages/typescript/rules/coding-standards.md (1)
268-284: Clarify the Types naming convention exception for interface files.The section correctly states "Append
Typesto exportedtypealiases" and distinguishes types from interfaces. However, the exception at lines 370–384 notes that small interfaces can live together in a*Typesfile. This could be clearer: the filename (e.g.,PaginationTypes.ts) containsTypes, but the interfaces inside still use theIprefix. This is correct but the dual naming (filename suffixTypes, interface prefixI) might confuse developers unfamiliar with the pattern.Consider adding a clarifying note: "The filename suffix
Typesindicates the file holds reusable, related type definitions; individual interface exports within still follow theIprefix convention."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
templates/languages/typescript/rules/coding-standards.md(2 hunks)templates/shared/rules/repository-pattern.md(6 hunks)
🧰 Additional context used
🪛 LanguageTool
templates/languages/typescript/rules/coding-standards.md
[uncategorized] ~270-~270: Possible missing comma found.
Context: ...es Append Types to exported type aliases and never prefix them with I: ```t...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (1)
templates/languages/typescript/rules/coding-standards.md (1)
484-489: Aligns well with the documented pattern; verify testability approach in practice.The ProductService example correctly demonstrates the Service Composition Rule from
repository-pattern.md: injecting infrastructure dependencies (db, logger) and instantiating the generic repository inside the constructor. The field is typed as the interface (line 482) and the concrete implementation is created internally (line 489).However, ensure that your test strategy (mocking or dependency injection override) aligns with the pattern. Since
productsisprivate, verify that tests can reliably inject mocks without needing reflection or non-standard test utilities.
| ### Service Composition Rule | ||
|
|
||
| - Services must depend on the `IGenericRepository<T>` interface but instantiate the concrete implementation inside the service constructor (or equivalent setup). This keeps the dependency type-safe while making the instantiation explicit: the interface is “injected” by virtue of being the field type, and the service owns the `new` call. | ||
| - Example (TypeScript): | ||
|
|
||
| ```typescript | ||
| export class ProductService { | ||
| private readonly products: IGenericRepository<IProduct>; | ||
|
|
||
| constructor() { | ||
| this.products = new GenericRepository<IProduct>({ collection: 'products' }); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| - Tests can replace `this.products` with a mock that satisfies `IGenericRepository<IProduct>` because the rest of the code only talks to the interface. |
There was a problem hiding this comment.
Clarify the testability claim for private repository fields.
Line 62 states "Tests can replace this.products with a mock", but products is declared as private readonly in the example (line 54). Private fields cannot be accessed or replaced by external test code without reflection or access to protected/internal scope.
Either:
- Mark the field as
protectedif tests should access it directly (reduces encapsulation) - Clarify that test doubles require dependency injection of the interface (contradicting the "service owns the
newcall" approach) - Use test-specific patterns like dependency setters or partial mocking frameworks
🤖 Prompt for AI Agents
templates/shared/rules/repository-pattern.md around lines 47 to 62: the current
text claims "Tests can replace `this.products` with a mock" but the example
declares `private readonly products`, which prevents direct replacement; update
the guidance to clarify testability by either (a) change the example to use
`protected` (or non-private) if tests are expected to directly replace the
field, (b) explicitly state that to keep the service owning the `new` call tests
must instead inject a mock via a constructor parameter or a setter (and modify
the example to show that pattern), or (c) mention advanced options like
reflection/partial-mocking frameworks if you want to keep the field private;
pick one approach and adjust the example and prose so they are consistent.
1 participant
Description
This pull request encompasses the following changes:
Documentation Update for Repository and TypeScript Patterns:
IGenericRepository<T>pattern to streamline consistency and maintainability.Typessuffix for type aliases while retainingIprefixes for interfaces.Code Refactoring:
allowJsin distributed packages, triggered during thepreparescript execution..gitignoreto exclude/coverage/,/dist/, and temporary dev artifacts.Chore Tasks:
/coverageto ensure better project hygiene.Checklist
Summary by CodeRabbit
Release Notes