feat(policies): add 12 experimental cowboyd review policies#162
Conversation
Add code review policies extracted from cowboyd's historical review patterns across thefrontside org (1,405 comments from 1,124 PRs). New experimental policies: - Type-Driven Design: no escaping through any - Structured Concurrency: explicit task lifetimes - Minimal APIs: small surface, platform alignment - Deterministic Tests: lifecycle invariants, not timing - Documentation: first-class API surface - Backwards Compatibility: explicit deprecation paths - Naming Consistency: Effection vocabulary - Correctness Invariants: validate and test edge cases - Composable Units: single responsibility helpers - Start With Why: intent-first review rubric - Ergonomics: helpers preserve scope semantics - Focused PRs: no mixing features with formatting All policies are Experimental (advisory only) for initial rollout. CodeRabbit config updated to reference new policies.
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds many new experimental policy documents under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
12-32:⚠️ Potential issue | 🟡 MinorInclude TSX files in the TypeScript policy path.
Right now, only
**/*.tsreceives the policy instructions, so.tsxfiles would be skipped. Consider widening the glob.💡 Suggested update
- - path: "**/*.ts" + - path: "**/*.{ts,tsx}"Based on learnings: Applies to **/*.{ts,tsx} : Use explicit return types on public functions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 12 - 32, The path_instructions entry currently targets only "*.ts" so .tsx files are skipped; update the glob in the path_instructions block (the entry with path: "**/*.ts") to include TSX (for example "**/*.{ts,tsx}" or add a separate "**/*.tsx" entry) so TSX files are processed by the TypeScript policy; while here also add the suggested policy reminder (e.g., "Use explicit return types on public functions") into the instructions string so the rule applies to both .ts and .tsx sources.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.policies/ergonomics.md:
- Around line 59-61: The generator delegation uses non-standard spacing ("yield
*") in the expression that assigns transformed2; update the syntax to use the
normalized form "yield*" without a space where mapStream is delegated (the
expression involving yield and mapStream/processItem), ensuring consistency with
other uses of yield* in the codebase and preserving the existing semantics of
transformed2 and processItem.
In @.policies/start-with-why.md:
- Around line 49-51: Remove the stray opening fenced code block that breaks
Markdown rendering by deleting the unmatched triple backticks before the "###
Compliant: Code with explanatory comments" heading so the heading is rendered
normally; ensure there is no leftover lone fence elsewhere in the same section
and that the heading text "### Compliant: Code with explanatory comments" stands
alone as a normal Markdown heading.
In @.policies/structured-concurrency.md:
- Around line 44-55: The example in useConnection uses until(...) but never
imports until; update the imports at the top (where resource and Operation are
imported) to also import until so that until is available for use in the
generator-based resource (function useConnection) and its yield* until(new
Promise(...)) call; ensure the import line includes until alongside resource and
type Operation.
---
Outside diff comments:
In @.coderabbit.yaml:
- Around line 12-32: The path_instructions entry currently targets only "*.ts"
so .tsx files are skipped; update the glob in the path_instructions block (the
entry with path: "**/*.ts") to include TSX (for example "**/*.{ts,tsx}" or add a
separate "**/*.tsx" entry) so TSX files are processed by the TypeScript policy;
while here also add the suggested policy reminder (e.g., "Use explicit return
types on public functions") into the instructions string so the rule applies to
both .ts and .tsx sources.
- Fix yield* spacing (was 'yield *', now 'yield*') - Replace 'any' with proper types in naming-consistency example - Fix invalid assertion syntax in correctness-invariants tests - Complete the cleanup test example with proper structure - Add missing 'until' import in structured-concurrency - Fix markdown code block formatting in start-with-why - Rename useConnection to useHttpClient in correctness-invariants to avoid confusion with structured-concurrency's canonical example
- Vary domain examples to avoid repeating useConnection everywhere: - naming-consistency: useCache instead of useConnection - backwards-compatibility: useSession instead of useWebSocket - correctness-invariants: useHttpClient (from previous commit) - Add cross-reference notes to related policies: - deterministic-tests: extends no-sleep-test-sync - correctness-invariants: links to structured-concurrency, deterministic-tests - composable-units: links to structured-concurrency for resource pattern - ergonomics: notes tension with minimal-apis - documentation: links to backwards-compatibility - Add policy relationships diagram to index.md showing: - Lifecycle/concurrency policy hierarchy - API design tension (minimal vs ergonomics) - Code quality relationships
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.policies/correctness-invariants.md:
- Around line 81-91: The test "cleans up on halt" is wrong because closed is set
after useHttpClient instead of in a cleanup/finally block so halting the task
prevents it from running; change the test body so the resource cleanup sets
closed = true inside a finally block within the spawned generator (or assert via
useHttpClient's cleanup if that function exposes it) — update the spawn
generator (the function passed to spawn) to perform the cleanup in a finally and
adjust the comment to accurately describe that cleanup runs from the finally
block when task.halt() is called.
In @.policies/index.md:
- Around line 47-61: The fenced diagram block at the top-level diagram is
missing a language tag which triggers markdownlint MD040; update the
triple-backtick fence that wraps the ASCII diagram (the block starting with
"Lifecycle & Concurrency (core)") to include a language identifier such as text
(e.g., change ``` to ```text) so the fence is labeled and the lint error is
resolved.
In @.policies/start-with-why.md:
- Around line 60-81: Remove the duplicated “Compliant: Code with explanatory
comments” block and fix the malformed Markdown fence so the example compiles as
a single fenced code block; locate the duplicate/example around the function*
race<T>(ops: Operation<T>[]) example and ensure only one triple-backtick fence
surrounds the TypeScript snippet (ending after the return winner; }); line) and
delete the extra closing fence and repeated block so the markdown renders
correctly.
---
Duplicate comments:
In @.policies/ergonomics.md:
- Around line 60-62: The generator delegation uses the spaced form "yield *"
which is inconsistent with the repo style; update occurrences (e.g., the
expression producing transformed2 that calls mapStream with processItem and
inputStream) to use the compact "yield*" syntax for delegation (so use yield*
mapStream(processItem, inputStream)) and ensure any other uses of generator
delegation (mapStream, map, processItem, inputStream, transformed/transformed2)
follow the same "yield*" convention.
- Fix yield* spacing in ergonomics.md (was 'yield *', now 'yield*')
- Include TSX files in CodeRabbit path instructions (**/*.{ts,tsx})
- correctness-invariants.md: fix cleanup test to use finally block - index.md: add 'text' language tag to diagram fence (MD040) - start-with-why.md: remove duplicate Compliant block and fix fence Session-ID: ses_hZNu7J8b
Summary
This PR adds 12 experimental code review policies extracted from cowboyd's historical review patterns across the thefrontside organization.
Methodology
.policies/template.mdformatNew Policies (all Experimental)
anyPolicy State
All new policies are set to Experimental (advisory only). This means:
Changes
.policies/.policies/index.mdwith new policies section.coderabbit.yamlto reference experimental policiesFuture Work
Once these policies are validated:
thefrontside/.githubfor org-wide adoptionSummary by CodeRabbit
Documentation
Chores