Repository navigation
[PROJ-1821] Align the Corgi Commons cold-start story - #357
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (28)
WalkthroughThe PR reframes Corgi Commons as an approved governance pilot, documents reviewed policy application, expands OpenAPI contracts, adds snapshot approval tooling, centralizes content-rule matching, updates admin lifecycle controls, and aligns public, demo, documentation, and test surfaces. ChangesCorgi Commons pilot alignment
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PilotParticipant
participant GovernanceUI
participant GovernanceAPI
participant Operator
participant Rescore
PilotParticipant->>GovernanceUI: Submit policy ballot
GovernanceUI->>GovernanceAPI: Close voting window
GovernanceAPI->>Operator: Present aggregated policy for review
Operator->>GovernanceAPI: Approve or reject policy
GovernanceAPI->>Rescore: Apply approved policy and queue rescore
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Notion Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/content-filter-matching.test.ts (1)
1-72: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing edge-case coverage: null text, empty rules, and exclude-side prefix matching.
The suite covers include-keyword whole-word/phrase matching well, but misses cases the source function explicitly branches on:
checkContentRules(null, { includeKeywords: [...], excludeKeywords: [] })→ should return{ passes: false, reason: 'no_text_with_include_filter' }.checkContentRules(null, { includeKeywords: [], excludeKeywords: [] })→ should return{ passes: true }.checkContentRules(text, { includeKeywords: [], excludeKeywords: [] })→ should short-circuit{ passes: true }.- Exclude keywords use
prefixMatch: trueinmatchesKeyword, so a word-based exclude keyword (e.g.'foss') should also match inside a larger word (e.g.'fossil') via the boundary regex — unlike include keywords. Only the substring-fallback path ('18+') is tested for exclude; the word-boundary path's prefix behavior for excludes is unverified.As per path instructions for
**/*.test.ts: "Check for tests that only cover the happy path. Suggest edge cases: empty inputs, boundary values, null/undefined, ... and error conditions."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/content-filter-matching.test.ts` around lines 1 - 72, Add tests in the content filter matching suite for null text with include rules, null text with empty rules, and non-null text with empty rules, asserting the documented results. Also add an exclude-keyword case using a word keyword such as “foss” against “fossil” to verify prefix matching through the boundary-regex path, while preserving the existing include whole-word behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agent/REPO_CONTRACT.md`:
- Around line 121-122: Update Gotcha `#3` in REPO_CONTRACT.md to point frontend
installation instructions at the canonical web-next/ application, or explicitly
list both web-next/ and web/ if setup still requires both. Align the commands
with the frontend targeted by npm run verify and avoid directing new
contributors to web/ alone.
In `@docs/openapi.json`:
- Around line 2314-2373: Update the OpenAPI definitions for
/api/governance/epochs/transition and /api/admin/governance/end-round to remove
the ineffective force parameter and advertised 200 transition payload, and
document their 409 DirectTransitionDisabled response instead. Add a contract
test covering both disabled endpoints and snapshotting their 409 responses.
- Around line 12207-12234: The 400 response for the full-dataset export is
incorrectly documented as application/zip. Update the export route schema near
the full-dataset handler so 200 remains application/zip while 400 uses
application/json, then add a regression test asserting both response content
types.
In `@scripts/approve-demo-snapshot.ts`:
- Around line 68-107: Protect the approved artifact in main by checking whether
options.outputPath already exists before writeFile; refuse to overwrite either a
valid approved manifest or unrelated existing file unless a --force option is
present in argv. Extend parseOptions and its option typing to recognize --force,
preserve current behavior for new paths, and add tests covering both
existing-file rejection and forced overwrite.
In `@scripts/generate-openapi.ts`:
- Around line 72-79: Update the cleanup logic in the finally block of
generate-openapi so app.close(), db.end(), and redis.quit() are executed through
the same Promise.allSettled call. Ensure a failure from any one cleanup
operation does not prevent the remaining application, database, or Redis handles
from being closed.
In `@scripts/update-corgi-commons-record.ts`:
- Around line 32-33: Update the network calls in the script’s login, getRecord,
and putRecord flows to use AbortController-based timeouts (approximately 10–15
seconds), passing the abort signal through each request. Catch aborted requests
and surface a clear timeout error instead of allowing the deployment step to
hang, and preserve distinct handling for swapRecord CID mismatches by reporting
a clear CAS-conflict error.
- Line 1: Migrate the script’s BskyAgent usage to the current `@atproto/api` Agent
and CredentialSession authentication pattern, first confirming the installed API
version’s guidance and exports. Update the initialization and login flow while
preserving the existing getRecord and putRecord behavior, including CAS
semantics.
In `@src/governance/content-rule-matcher.ts`:
- Around line 5-46: Bound the module-level keywordMatcherCache used by
getKeywordMatcher so distinct keyword/prefixMatch combinations cannot grow
indefinitely. Implement a simple maximum-size eviction policy, such as removing
the least-recently-used entry on insertion and refreshing hits, while preserving
matcher reuse and existing matching behavior in matchesKeyword.
In `@src/governance/routes/epochs.ts`:
- Line 190: Align the subscriber_count response contract with its query in the
current epoch endpoint: either join approved_participants so only approved
active subscribers are counted, or rename the field and description to reflect
all active subscribers. Add a regression test for /api/governance/epochs/current
covering both approved-active and pending-active rows, asserting the intended
count and response contract.
In `@src/governance/routes/vote.ts`:
- Line 6: Update the route documentation near the vote handler to describe the
conditional isParticipantApproved gate controlled by config.FEED_PRIVATE_MODE,
unless the intended policy is to enforce approval unconditionally. Add coverage
for active subscribers in default mode and for the approval requirement when
FEED_PRIVATE_MODE is true.
In `@tests/web-next-homepage-anchors.test.ts`:
- Around line 51-58: Expand the landing-page text guard in the test labeled
“labels the landing replay as an illustrative Corgi Commons preview” to include
the source of every component rendered by web-next/app/page.tsx, including
HeroSection, SocialProof, CommunityExamplesSection, BentoSection, FAQSection,
and CTASection, or assert against the rendered route text. Preserve the existing
positive assertions while ensuring “Birders Who Code” is rejected anywhere in
the complete homepage content.
In `@tests/web-next-landing-ctas-render.test.tsx`:
- Line 102: Strengthen the CTA assertion in the render test by retaining the
visible “Try the governance demo” check and also verifying that the rendered CTA
has href="/demo". Add a regression case covering a missing or incorrectly wired
CTA destination, using the existing DemoCTA render/test setup and preserving the
current label assertion.
In `@web-next/app/admin/page.tsx`:
- Around line 181-192: Surface failures for the lifecycle mutations
openMutation, closeMutation, approveMutation, and rejectMutation by deriving a
lifecycle error state from their mutation errors and rendering an “Action failed
— try again.” message in each associated ConfirmModal (or once above the
lifecycle actions). Keep the modal open on failure, clear confirmation only on
success, and preserve the existing invalidation behavior.
In `@web-next/app/demo/__tests__/http-client.test.ts`:
- Line 294: Extend the HTTP client tests around the existing community-name
assertion with table-driven cases for empty/null community identity, missing or
stale corpus provenance, duplicate topic slugs, and null/undefined payload
fields, asserting the exact parser and client error behavior. Add non-2xx
response tests and concurrent-request coverage if the client shares session
state, ensuring mocks reflect the real parser and response handling.
In `@web-next/app/proposals/page.tsx`:
- Line 148: Update the enacted-proposals filtering in the proposals page to
require an explicit enacted/approved status from the epoch-history response,
rather than relying only on closed_at. Trace the epoch-history response type and
API mapping to expose or preserve that status, apply the predicate to the
rendered list, and add a test confirming a closed rejected epoch remains
excluded.
In `@web-next/components/bento-section.tsx`:
- Line 128: Update the sequence arrow visibility in the step layout around the
six-column grid and the arrow-rendering element near lines 136–139 so arrows
remain hidden whenever the layout wraps at md widths; show them only at the xl
single-row breakpoint. Add a responsive regression check covering the md
breakpoint to verify row-ending arrows are not rendered.
---
Outside diff comments:
In `@tests/content-filter-matching.test.ts`:
- Around line 1-72: Add tests in the content filter matching suite for null text
with include rules, null text with empty rules, and non-null text with empty
rules, asserting the documented results. Also add an exclude-keyword case using
a word keyword such as “foss” against “fossil” to verify prefix matching through
the boundary-regex path, while preserving the existing include whole-word
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94b5e157-b2af-4f47-8a8b-3b391d93e9c5
📒 Files selected for processing (68)
README.mddocs/PRD.mddocs/SYSTEM_OVERVIEW.mddocs/agent/REPO_CONTRACT.mddocs/docs-site/openapi.jsondocs/openapi-public.jsondocs/openapi.jsonpackage.jsonscripts/approve-demo-snapshot.tsscripts/demo-snapshot-capture.tsscripts/generate-openapi.tsscripts/publish-feed.tsscripts/update-corgi-commons-record.tssrc/bot/announcements.tssrc/demo/content-rules.tssrc/demo/corpus.tssrc/demo/service.tssrc/feed/demo-snapshot-source.tssrc/feed/server.tssrc/governance/content-filter.tssrc/governance/content-rule-matcher.tssrc/governance/routes/epochs.tssrc/governance/routes/vote.tstests/content-filter-matching.test.tstests/corgi-cold-start-product-story.test.tstests/demo-content-rules.test.tstests/demo-v4-routes.test.tstests/demo-v4-snapshot-source.test.tstests/web-next-demo-fixtures.test.tstests/web-next-homepage-anchors.test.tstests/web-next-landing-ctas-render.test.tsxweb-next/app/about/page.tsxweb-next/app/admin/page.tsxweb-next/app/dashboard/page.tsxweb-next/app/demo/__tests__/demo-copy.test.tsweb-next/app/demo/__tests__/frontend-release-blockers.test.tsxweb-next/app/demo/__tests__/http-client.test.tsweb-next/app/demo/layout.tsxweb-next/app/demo/page.tsxweb-next/app/demo/shadow-demo-api-schemas.tsweb-next/app/demo/shadow-demo-copy.tsweb-next/app/demo/shadow-demo-fixtures.tsweb-next/app/docs/page.tsxweb-next/app/history/page.tsxweb-next/app/how-it-works/page.tsxweb-next/app/not-found.tsxweb-next/app/page.tsxweb-next/app/post/page.tsxweb-next/app/proposals/page.tsxweb-next/app/start/page.tsxweb-next/app/vote/page.tsxweb-next/components/bento-section.tsxweb-next/components/community-examples-section.tsxweb-next/components/demo/community-picker.tsxweb-next/components/demo/live-proof-panel.tsxweb-next/components/demo/vote-panel.tsxweb-next/components/faq-section.tsxweb-next/components/feed/corgi-rank-badge.tsxweb-next/components/footer-section.tsxweb-next/components/header.tsxweb-next/components/hero-section.tsxweb-next/components/how-it-works-replay.tsxweb-next/components/landing-ctas.tsxweb-next/components/replay-teaser.tsxweb-next/components/social-proof.tsxweb-next/e2e/production-hard-refresh.spec.tsweb-next/lib/api/admin.tsweb-next/lib/replay-model.ts
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/demo/corpus.ts (1)
335-352: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winQuality-only reviewer gate failures can still report live health. Capture mode adds a warning but does not downgrade health unless the count gates already failed.
src/demo/corpus.ts#L335-L352: sethealth.statustodegradedwhenevergateFailuresis non-empty.tests/demo-v4-snapshot-source.test.ts#L415-L432: add independent English-share, rich-media, and author-concentration failures, including threshold boundaries.AI agent prompt: Implement the health downgrade and parameterize tests for every reviewer-safe gate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/demo/corpus.ts` around lines 335 - 352, Update the corpus health result near the warning construction so any non-empty gateFailures sets health.status to degraded, including quality-only reviewer gate failures. In tests/demo-v4-snapshot-source.test.ts lines 415-432, parameterize independent English-share, rich-media, and author-concentration failure cases, covering each gate’s threshold boundaries.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agent/REPO_CONTRACT.md`:
- Around line 138-141: Update Gotcha `#3` in REPO_CONTRACT.md to instruct
contributors to install dependencies in web-next as the canonical frontend,
while mentioning web only for legacy compatibility, matching the setup block’s
order and scope.
In `@docs/SYSTEM_OVERVIEW.md`:
- Around line 131-138: Insert a blank line before the governance_audit_log and
subscribers headings in the schema documentation, preserving all existing
heading and paragraph content.
- Around line 166-168: Update the Content-Rule Governance paragraph to use
explicit “include-keyword rules” and “exclude-keyword rules” terminology, while
preserving the existing allowlist behavior, precedence, approved signal and
topic policy, rescoring, and bounded demo scope.
In `@scripts/generate-openapi.ts`:
- Around line 71-83: Update the cleanup logic in the generator’s finally block
so cleanup failures do not replace an error thrown by the main generation flow,
preserving the original “Failed to fetch spec” or parsing error when both
operations fail. Only surface the cleanup AggregateError when the try block
completed successfully, and add coverage for a failed app.inject response
combined with rejected db.end and redis.quit calls.
In `@src/demo/snapshot-capture.ts`:
- Around line 75-81: Update the approvable artifact-writing flow after the
options.report.approvable check to prevent partial outputs: write the manifest
and review sheet sequentially or via temporary files, and on any write failure
remove both final approval artifacts after all in-flight writes have settled.
Preserve report writing and ensure neither manifest nor reviewSheet remains when
either artifact write fails.
In `@src/governance/routes/vote.ts`:
- Around line 244-247: Recheck participant approval within the vote transaction
before the ballot write, using the approved_participants row lock or equivalent
transactional guard so revocation cannot race with INSERT or UPDATE. Update the
vote path around isParticipantApproved and the governance_votes write, and add
tests covering revoke-before-write and concurrent revoke/write behavior.
In `@tests/corgi-cold-start-product-story.test.ts`:
- Around line 160-179: Generalize the OpenAPI contract test around the existing
disabled-lifecycle and full-dataset assertions to load and validate
docs/openapi.json, docs/openapi-public.json, and docs/docs-site/openapi.json.
Apply the transition and media-type checks to every spec, then add structural
equality assertions for shared contracts including /api/governance/vote
requestBody, /api/governance/waitlist, and transparency stats_status.required so
regeneration drift is detected.
In `@tests/governance-query-validation.test.ts`:
- Around line 104-168: Add coverage in the governance epoch tests for a
pending-review epoch with results_approved_at set to null, asserting the
response preserves null, and for /api/governance/epochs/current when the
approved-participant count is '0', asserting subscriber_count is 0. Reuse the
existing Fastify setup and dbQueryMock patterns in the current test suite.
In `@tests/web-next-homepage-anchors.test.ts`:
- Around line 65-69: Extend the assertions for renderedHomepageContent in the
homepage anchor test to require both “Illustrative preview” and “frozen snapshot
sourced from Corgi Commons.” Preserve the existing pageContent assertions and
retired-name check, using the already aggregated ReplayTeaser source content.
In `@web-next/app/admin/page.tsx`:
- Line 253: Update the participation copy and calculation in the relevant admin
page component so they use the approved-participant count contract rather than
feed.subscriberCount, or retain subscriber-specific wording if that count is
intentionally used. Trace the existing approved-participant data source and use
it as the denominator, then add regression coverage where subscriber and
approved-participant totals differ.
- Around line 203-212: Update invalidate in the admin page to return a combined
promise for all three queryClient.invalidateQueries calls, then make each
onSuccess handler in openMutation, closeMutation, approveMutation, and
rejectMutation async and await invalidate before closing the confirmation modal.
Add a delayed-invalidation test verifying lifecycle controls remain disabled and
cannot be resubmitted until the refreshed state is available.
In `@web-next/app/demo/__tests__/demo-copy.test.ts`:
- Line 57: Expand the tests around DISCLOSURE.posts to exercise fallback
presentation rather than only matching the literal copy. Add a focused
fallback-corpus case that verifies the mechanics-fixture label and confirms
snapshot-specific metadata is omitted, while preserving the existing disclosure
contract assertion.
In `@web-next/app/docs/page.tsx`:
- Around line 19-21: The public descriptions use inconsistent Corgi/Bluesky
rendering-boundary wording. In web-next/app/docs/page.tsx lines 19-21, state
that Corgi serves an ordered feed for Bluesky clients to render; in
web-next/app/how-it-works/page.tsx line 39, rename the step to “Render in
Bluesky” and state that Corgi serves the ordered feed. Add a cross-surface
assertion preventing future wording that says Bluesky receives or publishes
posts.
In `@web-next/app/proposals/page.tsx`:
- Around line 30-32: Update policyApplied() so active epochs with an omitted
phase are treated as policy-applied, while preserving the existing behavior for
phase "running" and rejecting other explicitly provided phases. Add table-driven
tests covering active/running, active/omitted, and active/non-running phase
combinations, plus relevant status cases.
---
Outside diff comments:
In `@src/demo/corpus.ts`:
- Around line 335-352: Update the corpus health result near the warning
construction so any non-empty gateFailures sets health.status to degraded,
including quality-only reviewer gate failures. In
tests/demo-v4-snapshot-source.test.ts lines 415-432, parameterize independent
English-share, rich-media, and author-concentration failure cases, covering each
gate’s threshold boundaries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 873b773e-26ad-4622-949f-726559771ecb
📒 Files selected for processing (76)
README.mddocs/PRD.mddocs/SYSTEM_OVERVIEW.mddocs/agent/REPO_CONTRACT.mddocs/docs-site/openapi.jsondocs/lab/demo-shadow-governance-contract.mddocs/openapi-public.jsondocs/openapi.jsonpackage.jsonscripts/approve-demo-snapshot.tsscripts/demo-snapshot-capture.tsscripts/generate-openapi.tsscripts/publish-feed.tsscripts/update-corgi-commons-record.tssrc/admin/routes/export.tssrc/admin/routes/governance.tssrc/bot/announcements.tssrc/demo/content-rules.tssrc/demo/corpus.tssrc/demo/service.tssrc/demo/snapshot-capture.tssrc/feed/demo-snapshot-source.tssrc/feed/server.tssrc/governance/content-filter.tssrc/governance/content-rule-matcher.tssrc/governance/routes/epochs.tssrc/governance/routes/vote.tstests/content-filter-matching.test.tstests/corgi-cold-start-product-story.test.tstests/demo-content-rules.test.tstests/demo-v4-routes.test.tstests/demo-v4-snapshot-source.test.tstests/governance-admin.test.tstests/governance-query-validation.test.tstests/votable-params-record-shape.test.tstests/web-next-demo-fixtures.test.tstests/web-next-homepage-anchors.test.tstests/web-next-landing-ctas-render.test.tsxweb-next/app/about/page.tsxweb-next/app/admin/page.tsxweb-next/app/dashboard/page.tsxweb-next/app/demo/__tests__/demo-copy.test.tsweb-next/app/demo/__tests__/frontend-release-blockers.test.tsxweb-next/app/demo/__tests__/http-client.test.tsweb-next/app/demo/layout.tsxweb-next/app/demo/page.tsxweb-next/app/demo/shadow-demo-api-schemas.tsweb-next/app/demo/shadow-demo-copy.tsweb-next/app/demo/shadow-demo-fixtures.tsweb-next/app/docs/page.tsxweb-next/app/history/page.tsxweb-next/app/how-it-works/page.tsxweb-next/app/not-found.tsxweb-next/app/page.tsxweb-next/app/post/page.tsxweb-next/app/proposals/page.tsxweb-next/app/start/page.tsxweb-next/app/vote/page.tsxweb-next/components/bento-section.tsxweb-next/components/community-examples-section.tsxweb-next/components/demo/community-picker.tsxweb-next/components/demo/live-proof-panel.tsxweb-next/components/demo/vote-panel.tsxweb-next/components/faq-section.tsxweb-next/components/feed/corgi-rank-badge.tsxweb-next/components/footer-section.tsxweb-next/components/header.tsxweb-next/components/hero-section.tsxweb-next/components/how-it-works-replay.tsxweb-next/components/landing-ctas.tsxweb-next/components/replay-teaser.tsxweb-next/components/social-proof.tsxweb-next/e2e/production-hard-refresh.spec.tsweb-next/lib/api/admin.tsweb-next/lib/api/client.tsweb-next/lib/replay-model.ts
💤 Files with no reviewable changes (1)
- src/admin/routes/governance.ts
Summary
Verification
npm run verify(150 files, 1,653 tests plus backend and both frontend builds)npm run docs:verifycd web-next && npm run lint && npx tsc --noEmit && npm run buildgit diff --cached --checkRelease boundary
Linear: PROJ-1821