Skip to content

docs: add CODESTYLE.md#469

Merged
conorluddy merged 4 commits into
mainfrom
docs/codestyle
Feb 28, 2026
Merged

docs: add CODESTYLE.md#469
conorluddy merged 4 commits into
mainfrom
docs/codestyle

Conversation

@conorluddy
Copy link
Copy Markdown
Owner

Summary

  • Adds CODESTYLE.md to the repo root with the project's engineering principles and conventions.
  • Covers philosophy, naming, functions, error handling, module organisation, testing, observability, agentic patterns, context optimisation, and project navigation.
  • Includes a checklist for use during code review and development.

πŸ€– Generated with Claude Code

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
Copy link
Copy Markdown

codecov Bot commented Feb 28, 2026

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 83.71%. Comparing base (e6c314b) to head (dc54020).
⚠️ Report is 2 commits behind head on main.

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.
πŸ“’ Have feedback on the report? Share it here.

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review

Summary

Well-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:

  • AHA Over DRY β€” the "third duplication" heuristic is a useful concrete rule
  • Guard clauses over nesting β€” directly applicable to patterns in this codebase
  • Progressive disclosure ordering (types β†’ public API β†’ helpers) β€” a clear, enforceable convention
  • ensure* idempotency patterns β€” good naming convention for agent-facing mutations

Aspirational vs descriptive β€” needs clarification

Two 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:

Use Result<T, E> types for expected failures. Reserve exceptions for truly unrecoverable situations.

The codebase currently uses thrown errors throughout (e.g. throw new BadRequestError(...), throw new ForbiddenError(...)). This guidance conflicts with existing patterns. Consider marking it as aspirational, or removing it if there is no near-term plan to migrate.

Agentic Patterns β€” Observable side effects:

Mutations return { data, changes[], warnings[] } so agents can verify their actions.

No service in the codebase returns this shape. Same suggestion β€” clarify if this is aspirational.


Minor suggestions

Naming section: The codebase uses MW (for middleware) as a top-level namespace import. If abbreviations are banned, worth a note like "except established project-level aliases such as MW, SERVICES, CONTROLLERS".

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.


Overall

Approve with suggestions. The document is genuinely useful and well-written. The main ask is to clarify the Result<T, E> and observable-side-effects sections so contributors are not confused about what is current convention vs. what is aspirational.

- 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>
@conorluddy conorluddy enabled auto-merge (squash) February 28, 2026 15:35
@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review: docs: add CODESTYLE.md

Summary

The document is well-structured and covers the right ground. A few suggestions below.


Content βœ…

The guiding principles are clear and internally consistent:

  • AHA over DRY is a good antidote to premature abstraction β€” worth keeping prominent.
  • Progressive Disclosure as a file-layout principle (types β†’ public API β†’ helpers) is practical and directly actionable.
  • Agentic Patterns section is a differentiator β€” most style guides don't address this. The idempotent-operations and machine-parseable-errors guidance is useful.
  • Context Optimisation section is coherent and specific.

Minor Suggestions

Error handling aspirational note β€” the Result<T, E> pattern is flagged as aspirational in the error handling section, which is honest. It might be worth also noting the current thrown-error pattern in the checklist so reviewers know what to apply today vs. what to aim for.

Testing trophy reference β€” "Testing trophy" is a term associated with Kent C. Dodds' model. The current description ("mostly integration tests") is accurate but could add a one-line rationale so it doesn't read as an arbitrary preference: e.g., "integration tests catch more real failure modes with less coupling to implementation detail than unit tests alone."

"300-line guideline per file" β€” this is a useful signal, but the document might benefit from a note that the reason to split is cognitive load / context window size, not line count itself. Splitting a cohesive 350-line file into two coupled 175-line files is worse.

Anti-patterns list β€” "token bloat" and "inverted file disclosure" are jargon-dense for a shared style guide. Consider a one-line gloss for each, or link to the relevant section.


Version Conflict

Both this branch and fix/security-critical (#466) bump package.json from 0.3.6 to 0.3.8. Whichever merges second will get a conflict. Coordinate version bumps across the stack when these are merged in order.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review: PR 469 - docs: add CODESTYLE.md

Overall: Approve

The guide is well-structured, appropriately dense, and covers the right concerns for an AI-assisted codebase. A few notes below.


What is good

Philosophy 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 merging

Version 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

  • The 300-line file guideline and 50-line function guideline are good defaults, though the codebase already has a few files that exceed 300 lines. Worth a follow-up pass to bring outliers in line.
  • The Project Navigation section references CLAUDE.md at root β€” consider adding a note that AGENTS.md exists alongside it, since that is the entry point for AI agents in this repo.
  • 'No abbreviations: customer not cust, configuration not cfg β€” established project-level aliases (MW, SERVICES, CONTROLLERS) are exceptions' β€” this exception carve-out is pragmatic but could lead to scope creep. Worth periodically reviewing whether new aliases added in the future are genuinely project-level or just habit.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review

Well-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 done

Progressive 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.


Suggestions

Relationship 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:

  • Adding a one-line reference in AGENTS.md: 'See CODESTYLE.md for full style guidelines'
  • Or noting in CODESTYLE.md that AGENTS.md is the entry point for agents

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

  • The comment 'No abbreviations: customer not cust, configuration not cfg - established project-level aliases (MW, SERVICES, CONTROLLERS) are exceptions' is good - it pre-empts the obvious question.
  • 'Token bloat' in the Anti-Patterns list is a nice nod to the agentic context without being heavy-handed.

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.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review β€” PR #469: docs: add CODESTYLE.md

Well-structured and thoughtful. The document is dense without being bloated β€” it practices what it preaches.


βœ… Structure and philosophy

The 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 conventions

Specific 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 section

The 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 distinction

Clearly 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

  • AHA Over DRY β€” the acronym is expanded inline (Avoid Hasty Abstractions) which is good. Optionally adding a source attribution (Kent C. Dodds) would help readers who want to dig deeper.
  • TypeScript strict mode β€” the rule is stated but doesn't reference the tsconfig.json settings that enforce it. A note like 'enforced via strict: true in tsconfig' would close that loop.
  • 300-line file guideline β€” a few existing files exceed this already. Noting that the guideline applies to new code (not retroactively) would prevent confusion.
  • Version bump β€” package.json is bumped to 0.3.8 here, but fix(security): hash password on user update; remove broken XSRF middlewareΒ #466 already set it to 0.3.8. Worth confirming the version won't double-increment once all PRs in the chain merge.

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.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review

Strengths

The 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.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review: docs: add CODESTYLE.md

Good addition. A few notes on content and scope.


Strengths

  • Checklist at the bottom is genuinely useful for both code review and self-review. Items are specific enough to be actionable.
  • AHA Over DRY is a healthy principle for a project at this stage β€” premature abstraction is a common failure mode.
  • Context Optimisation section is a thoughtful addition given the project's agentic use case; the token-economy framing is practical.
  • Progressive Disclosure principle is well-defined and consistent with how the rest of the codebase is organised.

Issues

1. "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. CLAUDE.md is referenced but does not exist (medium)

The "Project Navigation" section says:

CLAUDE.md at root: Entry points, key patterns, common tasks. Under 200 lines.

There is no CLAUDE.md at the repo root. This creates a broken reference and a gap for anyone (or any agent) following the navigation conventions this document establishes. A minimal CLAUDE.md would make the guidance self-consistent.

3. Aspirational items are easy to miss (low)

"Error Handling" and "Agentic Patterns" include *(aspirational)* markers inline, but these are easy to overlook at a skim. A brief callout block at the top of each section (e.g., > Current: thrown errors | Goal: Result types) would make the current-vs-target distinction immediately visible.

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 *.types.ts files. Worth either aligning the guideline with the actual current pattern, or explicitly marking this as the target state for new code only.


Notes

  • 106 lines is a good length β€” dense without being exhausting.
  • Naming conventions (boolean prefixes, verb-first functions, units in names) are consistent with the existing codebase.
  • The Anti-Patterns section is a useful negative complement to the checklist at the end.

… branch coverage note

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review: PR 469 - add CODESTYLE.md

A well-considered style guide. The philosophy section is strong and the checklist is immediately practical.

Positives

  • AHA over DRY is the right framing: naming the failure mode (Hasty Abstraction) rather than just saying dont abstract is more actionable.
  • The Agentic Patterns section is a genuinely useful addition for a codebase that uses AI agents. Most style guides skip this entirely.
  • The Context Optimisation section is sharp. The principle that every token code consumes in an agent context window is a token it cannot use for reasoning is highly practical.
  • The checklist maps cleanly to the preceding sections and is useful at review time.

Issues

1. 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

  • Add a stub AGENTS.md in a follow-up (or this PR) to prevent the broken navigation reference.
  • The Project Navigation section is the strongest candidate for a CLAUDE.md excerpt, as it is the most useful to an AI agent starting cold on the repo.

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
@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

Code Review: docs: add CODESTYLE.md

A solid, well-considered style guide. A few observations:


Strengths

  • AHA over DRY is the right heuristic for this kind of codebase β€” calling it out explicitly is useful.
  • Naming conventions are specific and actionable (units in names, boolean prefixes, verb-first functions).
  • Agentic Patterns section is forward-thinking and practical, especially the idempotent operations and contract-first design points.
  • Checklist at the end is genuinely useful for PR review β€” concrete enough to actually apply.
  • Context Optimisation section is a nice addition given the project's stated goals around AI-assisted development.

Minor observations

1. AGENTS.md is referenced but doesn't exist
The "Project Navigation" section says:

AGENTS.md at root: AI agent entry point β€” setup commands, architecture overview, and code style summary. AGENTS.md links to it.

There's no AGENTS.md in this PR or the repo. Either create a stub alongside this, or soften the language to mark it as a goal (like the "README.md per major module" entry already does).

2. CLAUDE.md is referenced but doesn't exist either
Same section: "CLAUDE.md at root: Entry points, key patterns, common tasks. Under 200 lines." β€” also not present in the repo. Both of these forward-references would be less confusing with a clear "(not yet created)" note.

3. Testing philosophy vs. current practice
The Testing section says "mostly integration tests" (testing trophy model). The current test suite appears to lean more unit-test-heavy. This aspirational direction is worth stating, but if there's a gap between what the guide says and what new contributors observe in the codebase, it can cause confusion. A brief "(this is the goal; current coverage skews toward unit tests)" note would align expectations.


Summary

This is a clean, well-written guide. The main actionable item is resolving the forward-references to files that don't exist yet (AGENTS.md, CLAUDE.md). Everything else is ready to merge.

πŸ€– Generated with Claude Code

@conorluddy conorluddy merged commit 0e683e6 into main Feb 28, 2026
11 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Feb 28, 2026

PR Review: docs: add CODESTYLE.md

Clean, well-structured document that codifies conventions the codebase already follows. A few observations:


Positives

  • Philosophy section is concise and opinionated β€” "AHA Over DRY" and "Aggressive Minimalism" set a clear tone that matches the actual codebase.
  • Naming conventions are specific β€” including units (delayMs, maxRetries) and boolean prefixes (isValid, hasPermission) are the kinds of details that matter day-to-day.
  • Agentic patterns section β€” thoughtful addition given the AI-assisted workflow. Idempotent operations, explicit state machines, and contract-first design are all sensible constraints.
  • Checklist at the end β€” practical and easy to reference during review.
  • AGENTS.md integration noted β€” the cross-reference between CODESTYLE.md ↔ AGENTS.md ↔ CLAUDE.md is a good layered context strategy.

Suggestions

"80% line coverage minimum" vs AGENTS.md "90%+"
The Testing section says "80% line coverage minimum" but AGENTS.md and CLAUDE.md both reference β‰₯90% type coverage (not line coverage) and the pre-commit hook enforces 90%. Worth clarifying: is 80% the line coverage floor (Codecov) and 90% the type coverage floor (type-coverage CLI)? If so, state both explicitly to avoid confusion.

Aspirational items are not flagged consistently
Some items are clearly aspirational (Result<T, E> migration, Observable side effects), and some sections mark them with italics, but others don't. A consistent *(aspirational)* label on all forward-looking items would help readers know what's enforced today vs. what's a goal.

package.json version bump
The diff shows a version bump in package.json alongside a docs-only change. This seems to be a side-effect of a branch/merge sequence rather than intentional. Worth double-checking before merge.


Summary: Approve with the note about the coverage number discrepancy β€” that's the only thing that could actively mislead contributors. Everything else is solid.

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.

1 participant