Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- 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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- 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
Uh oh!
There was an error while loading. Please reload this page.
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