docs: add CODESTYLE.md#469
Conversation
Codifies the project's engineering principles β progressive disclosure, aggressive minimalism, AHA over DRY, agentic patterns, and context optimisation. Includes a checklist for use during code review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #469 +/- ##
==========================================
+ Coverage 83.34% 83.71% +0.37%
==========================================
Files 111 109 -2
Lines 1837 1781 -56
Branches 305 295 -10
==========================================
- Hits 1531 1491 -40
+ Misses 198 190 -8
+ Partials 108 100 -8 β View full report in Codecov by Sentry. π New features to boost your workflow:
|
Code ReviewSummaryWell-structured document. A couple of points worth addressing to avoid confusion against the current codebase. Content qualityβ Strong overall. The document is well-organised, dense without being verbose, and follows its own "token-aware docs" principle. The checklist at the end is particularly useful for code review. Highlights worth keeping:
Aspirational vs descriptive β needs clarificationTwo sections describe patterns that the current codebase does not follow. Without clarification, these read as current standards rather than goals, which could confuse contributors: Error Handling:
The codebase currently uses thrown errors throughout (e.g. Agentic Patterns β Observable side effects:
No service in the codebase returns this shape. Same suggestion β clarify if this is aspirational. Minor suggestionsNaming section: The codebase uses Testing section: Consider specifying which coverage metric (line, branch, statement) since these diverge significantly in practice. The 80% figure is more actionable with that context. Project Navigation section: No module-level READMEs currently exist. Worth noting this as a goal rather than present state. OverallApprove with suggestions. The document is genuinely useful and well-written. The main ask is to clarify the |
- Error Handling: mark Result<T,E> as aspirational vs the current thrown-error pattern - Agentic Patterns: mark observable side effects as aspirational - Naming: note that established project aliases (MW, SERVICES, CONTROLLERS) are exceptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review:
|
Code Review: PR 469 - docs: add CODESTYLE.mdOverall: ApproveThe guide is well-structured, appropriately dense, and covers the right concerns for an AI-assisted codebase. A few notes below. What is goodPhilosophy section is strong β Progressive Disclosure, AHA over DRY, and Aggressive Minimalism are opinionated but well-reasoned positions that will meaningfully constrain future decisions. Distinction between current and aspirational patterns β clearly marking Result<T,E> types and Observable side effects as aspirational prevents confusion about what the codebase actually does today. Good practice. Agentic Patterns section β token-aware documentation, semantic compression, and explicit state machines are relevant and practical for AI-assisted development workflows. The review checklist β actionable and scannable. This format works well as a gate during code review. Issues to fix before mergingVersion conflict: package.json is bumped to 0.3.8, which is the same target version as PR 466. Once the security PRs are merged this will conflict. Please rebase onto the merged stack before merging this PR. Minor notes
|
Code ReviewWell-structured style guide. The philosophy section sets a clear tone and the checklist is practical for both human reviewers and AI agents. What is well doneProgressive disclosure principle - Files answering 'what is this?' in the first 10 lines, with types -> public API -> private helpers ordering, aligns well with the existing codebase structure. AHA over DRY - Explicitly naming this principle is useful. The 'wait for the 3rd duplication' rule is a concrete heuristic rather than a vague guideline. Agentic patterns section - The machine-parseable errors, idempotent operations, and contract-first design guidelines are well-suited to a project that uses AI agents. The token-aware docs and context boundary guidance is practical. Checklist format - The checklist at the end is directly usable during code review, which is the right format for a style doc. SuggestionsRelationship to AGENTS.md not explicit Both CODESTYLE.md and AGENTS.md (the Code Style section starting at line 155) cover naming conventions, error handling, and test patterns. Right now they are partially redundant. Consider either:
Without this, agents loading AGENTS.md may miss CODESTYLE.md, and humans may wonder which is authoritative. Aspirational items need clearer labelling The Result<T, E> migration, Observable side effects, and branded types sections are marked 'aspirational' but this is easy to miss. Consider a consistent visual marker (e.g., a [future] tag or a dedicated 'Aspirational / Future Direction' subsection) so contributors know what reflects current practice vs what is a goal. Function length guideline placement The '50-line guideline' is mentioned in the Functions section but not in the Checklist. Since the Checklist is what people will actually scan during review, consider adding: 'Functions under 50 lines (trigger for refactor, not hard limit)'. Minor notes
Verdict: Approve. The one action worth taking before merge is clarifying the relationship between this file and AGENTS.md to avoid redundancy and ensure agents load the right context. |
Code Review β PR #469: docs: add CODESTYLE.mdWell-structured and thoughtful. The document is dense without being bloated β it practices what it preaches. β Structure and philosophyThe progressive-disclosure principle described in the document is reflected in how the document itself is organised: philosophy first, specifics below. The Anti-Patterns and Checklist sections at the bottom are particularly useful as quick-reference tools during review. β Naming conventionsSpecific and actionable (activeUsers not users, include units, no abbreviations with carve-outs for established project aliases like MW and SERVICES). These are the kinds of conventions that are easy to follow consistently once written down. β Agentic Patterns sectionThe framing around idempotency, explicit state machines, and machine-parseable errors is valuable for a codebase that involves AI-assisted development. The ensure* pattern for idempotent operations is a good concrete convention to establish. β Current vs aspirational distinctionClearly marking the Result<T, E> error handling approach as aspirational (rather than the current pattern) avoids confusion and sets realistic expectations. This is the right way to document evolving conventions. Minor observations
Verdict: Approve β a useful, readable addition. The document is opinionated in the right ways and draws a clear line between current practice and aspirational direction. |
Code ReviewStrengthsThe document is well-structured, dense, and actionable. The philosophy section (Progressive Disclosure, AHA Over DRY, Aggressive Minimalism) captures the project's observed conventions accurately. The Agentic Patterns and Context Optimisation sections are a particularly valuable addition for this AI-assisted codebase. The checklist at the end is immediately usable during review. Issues'CLAUDE.md at root' reference is incorrect: The Project Navigation section says 'CLAUDE.md at root: Entry points, key patterns, common tasks. Under 200 lines.' β but this repo uses AGENTS.md, not CLAUDE.md. Update the reference to AGENTS.md to avoid confusion. Coverage threshold inconsistency: CODESTYLE.md states '80% coverage minimum' but AGENTS.md states '90%+ coverage required' (and the pre-commit type-coverage check enforces 90%). The test coverage (80%) and type coverage (90%) thresholds are different metrics, but the documents should be explicit about which threshold applies to which metric to avoid ambiguity. package-lock.json not updated: package.json is bumped to 0.3.8 but package-lock.json is not changed in this PR. Run npm install to sync them before merging. Version bump will conflict: This PR bumps from 0.3.6 (main) to 0.3.8, which conflicts with the version bumps in the stacked security PRs (#466β#468). Rebase onto the tip of #468 before merging, or merge the security PRs first and rebase this one. |
Code Review: docs: add CODESTYLE.mdGood addition. A few notes on content and scope. Strengths
Issues1. "AHA" is not defined on first use (low) The principle is listed as "AHA Over DRY" but "AHA" (Avoid Hasty Abstractions) is never spelled out. Worth expanding inline for readers unfamiliar with the term. 2. The "Project Navigation" section says:
There is no 3. Aspirational items are easy to miss (low) "Error Handling" and "Agentic Patterns" include 4. File disclosure order does not match current practice (low) The document says "Types β public API β private helpers, always in that order." Many existing files have types at the bottom or in separate Notes
|
β¦ branch coverage note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review: PR 469 - add CODESTYLE.mdA well-considered style guide. The philosophy section is strong and the checklist is immediately practical. Positives
Issues1. Version bump is 3 jumps for a docs-only change package.json goes from 0.3.6 to 0.3.9. This PR stacks on unmerged PRs #468 and #470, so the version will conflict when those merge first. A docs-only PR should not need any version bump. Consider removing it or normalising to a single patch bump after the stack resolves. 2. AGENTS.md is referenced as if it exists There is no AGENTS.md in the repo. CODESTYLE.md states that AGENTS.md is the AI agent entry point at root and that CODESTYLE.md is the full style reference that AGENTS.md links to. If this is aspirational, phrase it as planned or future. As written, a contributor following the navigation guide will immediately hit a missing file. 3. Aspirational items could be flagged inline The Error Handling and Agentic Patterns sections mix current practice with aspirational goals without always marking the difference. A contributor cannot tell which patterns are enforced now versus what the team is working towards. Even a simple (aspirational) suffix on those bullets would help. Suggestions
Summary: The content is high quality and the project benefits from having it. Fix the AGENTS.md reference before merging and remove the unnecessary version bump. |
# Conflicts: # .github/type-coverage.json # package.json
Code Review: docs: add CODESTYLE.mdA solid, well-considered style guide. A few observations: Strengths
Minor observations1.
There's no 2. 3. Testing philosophy vs. current practice SummaryThis is a clean, well-written guide. The main actionable item is resolving the forward-references to files that don't exist yet ( π€ Generated with Claude Code |
PR Review: docs: add CODESTYLE.mdClean, well-structured document that codifies conventions the codebase already follows. A few observations: Positives
Suggestions"80% line coverage minimum" vs AGENTS.md "90%+" Aspirational items are not flagged consistently
Summary: Approve with the note about the coverage number discrepancy β that's the only thing that could actively mislead contributors. Everything else is solid. |
Summary
CODESTYLE.mdto the repo root with the project's engineering principles and conventions.π€ Generated with Claude Code