Skip to content

Comments

feat(policies): add 12 experimental cowboyd review policies#162

Merged
taras merged 5 commits intomainfrom
feat/cowboyd-review-policies
Feb 18, 2026
Merged

feat(policies): add 12 experimental cowboyd review policies#162
taras merged 5 commits intomainfrom
feat/cowboyd-review-policies

Conversation

@taras
Copy link
Member

@taras taras commented Feb 17, 2026

Summary

This PR adds 12 experimental code review policies extracted from cowboyd's historical review patterns across the thefrontside organization.

Methodology

  1. Extracted 1,678 review comments from 1,124 PRs where cowboyd reviewed/commented
  2. Filtered to 1,405 high-signal comments (removed LGTM, empty, <20 chars)
  3. Clustered using GPT 5.2 to identify recurring patterns
  4. Validated each policy with @effection-pro for Effection-specific applicability
  5. Documented using the existing .policies/template.md format

New Policies (all Experimental)

Policy Description
Type-Driven Design Types should force correct usage; no escaping through any
Structured Concurrency Task lifetimes must be explicit; no fire-and-forget async work
Minimal APIs Keep public API surface small; align with platform standards
Deterministic Tests Assert on lifecycle invariants, not elapsed time
Documentation User-visible changes require docs; document lifecycle semantics
Backwards Compatibility Be explicit about breaking changes; deprecate intentionally
Naming Consistency Use Effection vocabulary consistently
Correctness Invariants Validate inputs; test success/error/halt paths
Composable Units Extract focused helpers with single responsibility
Start With Why Ask for intent before prescribing changes (review rubric)
Ergonomics Ergonomic helpers that preserve structured concurrency semantics
Focused PRs No mixing feature changes with formatting/refactors

Policy State

All new policies are set to Experimental (advisory only). This means:

  • CodeRabbit will surface them as feedback
  • They do NOT block merges
  • We can promote to Recommended/Strict based on feedback

Changes

  • Added 12 new policy markdown files in .policies/
  • Updated .policies/index.md with new policies section
  • Updated .coderabbit.yaml to reference experimental policies

Future Work

Once these policies are validated:

  1. Promote high-value policies to Recommended
  2. Consider adding to thefrontside/.github for org-wide adoption
  3. Build automated GitHub Action for policy enforcement

Summary by CodeRabbit

  • Documentation

    • Added many new policy documents covering compatibility, composability, correctness, deterministic tests, documentation, ergonomics, focused PRs, naming, minimal APIs, structured concurrency, type-driven design, and review intent.
    • Reworked policy index into clear "Established" and "Experimental" sections with verification checklists, examples, and guidance.
  • Chores

    • Updated configuration and pre-merge guidance to surface new policies and treat experimental policies as advisory (non-blocking).
    • Broadened file-path matching to include TSX files.

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.
@coderabbitai
Copy link

coderabbitai bot commented Feb 17, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

Adds many new experimental policy documents under .policies/ and updates the CodeRabbit review configuration (.coderabbit.yaml) to include Established and Experimental policy sections, broaden path globs to include TSX, and treat experimental policy violations as advisory (non-blocking) in pre-merge checks.

Changes

Cohort / File(s) Summary
CodeRabbit configuration & policy index
.coderabbit.yaml, .policies/index.md
Expanded path glob to include *.tsx; added "Established Policies" and "Experimental Policies (cowboyd Review Patterns)"; marked experimental policies as advisory/non-blocking and adjusted pre_merge_checks and package.json review guidance.
Backwards compatibility
.policies/backwards-compatibility.md
New experimental policy specifying breaking-change classifications, deprecation paths, examples, verification checklist, and migration guidance.
Composable units & correctness
.policies/composable-units.md, .policies/correctness-invariants.md
Added experimental guidelines for small single-responsibility helpers and for encoding assumptions via validation/exhaustive handling, with examples and checklists.
Deterministic tests & structured concurrency
.policies/deterministic-tests.md, .policies/structured-concurrency.md
New experimental policies enforcing lifecycle-driven, non-flaky tests (no sleeps) and structured concurrency practices (explicit task lifetimes, no fire-and-forget).
API design, ergonomics & naming
.policies/minimal-apis.md, .policies/ergonomics.md, .policies/type-driven-design.md, .policies/naming-consistency.md
Added experimental policies on minimal/interoperable APIs, ergonomic "happy path" APIs, type-driven design, and naming conventions aligned with Effection vocabulary.
PR/process, intent & docs
.policies/focused-prs.md, .policies/start-with-why.md, .policies/documentation.md
New experimental policies for focused PRs, intent-first review rubric, and treating documentation as a first-class API surface; each includes rules, examples, and verification checklists.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main addition: 12 new experimental code review policies extracted from cowboyd's patterns.
Description check ✅ Passed The description provides comprehensive motivation, detailed methodology, clear policy table, state explanation, and future work; exceeds template requirements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main
Policy Compliance ✅ Passed PR successfully adds 12 experimental policies with proper documentation structure and updates .coderabbit.yaml correctly without violating Strict or Recommended policies.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/cowboyd-review-policies

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟡 Minor

Include TSX files in the TypeScript policy path.

Right now, only **/*.ts receives the policy instructions, so .tsx files 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
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@taras taras merged commit 370f12a into main Feb 18, 2026
9 of 10 checks passed
@taras taras deleted the feat/cowboyd-review-policies branch February 18, 2026 13:50
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