docs: archive stale docs, fix references, add missing READMEs#1429
Merged
docs: archive stale docs, fix references, add missing READMEs#1429
Conversation
- Archive 5 stale/completed docs (audit reports, demo guides, old auth integration) - Delete obsolete shared/migrations/README.md (abandoned shared-schema architecture) - Rewrite event-router README from old utilization-metering-consumer description - Replace utilization-metering-consumer references with event-router in production deployment runbook - Fix dead starlark-patterns.md links to manifest-design-patterns.md - Update testcontainers guide and BIAN checklist from PostgreSQL to CockroachDB - Add cross-reference from saga-validation-failure runbook to validation guide - Add missing READMEs for financial-gateway and identity services
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocumentation reorganizations: archival docs added, shared migrations README removed, multiple guides switched from PostgreSQL/TestContainers to CockroachDB/GORM, UtilizationMeteringConsumer renamed/reworked to EventRouter across runbooks and service docs, and new READMEs added for Financial Gateway and Identity services. Changes
Sequence Diagram(s)sequenceDiagram
participant Kafka as Kafka (domain topics)
participant EventRouter as Event Router
participant ControlPlane as Control Plane (Saga Trigger)
participant Position as PositionKeeping / Billing
participant Handler as Domain Handler
Kafka->>EventRouter: Publish domain event
EventRouter->>EventRouter: CEL filter & route decision
alt matches saga trigger
EventRouter->>ControlPlane: Request saga execution (idempotent)
ControlPlane->>Handler: Execute saga steps / invoke services
Handler-->>ControlPlane: Saga result
ControlPlane->>Position: Record measurement / billing
Position-->>ControlPlane: Ack
ControlPlane-->>EventRouter: Saga triggered / status
else matches billing handler
EventRouter->>Position: Forward event for billing processing
Position-->>EventRouter: Ack
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/runbooks/production-deployment.md (1)
373-390:⚠️ Potential issue | 🟠 MajorDependency map is missing Event Router → Control Plane.
Line 389 currently shows only
EventRouter -> PositionKeeping, but the runbook text says Event Router also triggers sagas via Control Plane. The map should include that edge to avoid deployment-order confusion.Suggested doc fix
subgraph Tier4["Tier 4: Edge and Supporting Services"] GW["Gateway :8080"] AW["AuditWorker :8080"] ER["EventRouter :8080"] AC["AuditConsumers :8080"] end + CP["ControlPlane :<grpc-port>"] @@ GW --> MI ER --> PK + ER --> CP🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/runbooks/production-deployment.md` around lines 373 - 390, The diagram is missing the Event Router → Control Plane edge; update the mermaid block to add the connection from EventRouter (ER) to Control Plane (CA) so it reflects that Event Router triggers sagas via the Control Plane (e.g., add a line like "ER --> CA" or "EventRouter --> ControlPlane" to the graph where other edges are declared).
🧹 Nitpick comments (2)
docs/archive/README.md (1)
15-18: Consider explicit links for successor docs.For Line 15 and Line 18, using relative markdown links (not plain text/backticks) to the replacement docs/runbook will make this archive index more robust and easier to navigate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/archive/README.md` around lines 15 - 18, Replace the plain/backticked references to successor docs with explicit relative markdown links: change the mention of services/README.md and the "production deployment runbook" to proper [text](relative/path/to/services/README.md) and [text](relative/path/to/runbook.md) links, and update the entries for DEMO_GUIDE.md and authentication-integration.md so they link directly to their replacement pages (e.g., [DEMO guide](relative/path/to/demo.md) and [authentication integration](relative/path/to/identity-service-doc.md)); ensure the displayed link text remains descriptive and the target paths are correct for the repo structure.docs/runbooks/production-deployment.md (1)
499-502: Add explicit post-deploy Event Router health verification.You wait for deployment availability at Line 501, but there’s no explicit readiness probe check for Event Router in the verification flow. Adding one here would make rollout validation more complete.
Suggested addition
# Deploy Event Router kubectl apply -k services/event-router/k8s/ kubectl wait --for=condition=Available deployment/event-router -n production --timeout=120s +kubectl port-forward -n production svc/event-router 8080:8080 & +curl -f http://localhost:8080/ready🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/runbooks/production-deployment.md` around lines 499 - 502, Add an explicit readiness probe check after the existing "kubectl wait --for=condition=Available deployment/event-router -n production --timeout=120s" step by verifying pod readiness for the deployment/event-router (e.g., wait for all pods with the event-router selector to be Ready or use "kubectl rollout status deployment/event-router") and fail the run if any pod readiness/readyCount check does not reach expected replicas within a timeout; update the post-deploy block in the doc to call this readiness check referencing the deployment/event-router and the existing kubectl wait line so rollout validation includes health probes, not just availability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/testcontainers-usage.md`:
- Around line 3-5: The guide's header claims CockroachDB-first and shared
utility usage via testdb.SetupCockroachDB but the examples still reference
PostgreSQL types (postgres.PostgresContainer, PostgresRepository,
SetupTestContainer, postgres_* filenames); either update all example symbols and
filenames to Cockroach equivalents (e.g.,
CockroachContainer/CockroachRepository, call testdb.SetupCockroachDB everywhere,
and rename postgres_* examples to cockroach_*) or change the intro lines to
state the guide covers a mixed/legacy PostgreSQL-to-Cockroach transition and
explicitly call out which examples remain PostgreSQL (listing
postgres.PostgresContainer, PostgresRepository, SetupTestContainer, postgres_*).
Ensure consistency between the header and body and update any references to
testdb.SetupCockroachDB to match the chosen approach.
---
Outside diff comments:
In `@docs/runbooks/production-deployment.md`:
- Around line 373-390: The diagram is missing the Event Router → Control Plane
edge; update the mermaid block to add the connection from EventRouter (ER) to
Control Plane (CA) so it reflects that Event Router triggers sagas via the
Control Plane (e.g., add a line like "ER --> CA" or "EventRouter -->
ControlPlane" to the graph where other edges are declared).
---
Nitpick comments:
In `@docs/archive/README.md`:
- Around line 15-18: Replace the plain/backticked references to successor docs
with explicit relative markdown links: change the mention of services/README.md
and the "production deployment runbook" to proper
[text](relative/path/to/services/README.md) and
[text](relative/path/to/runbook.md) links, and update the entries for
DEMO_GUIDE.md and authentication-integration.md so they link directly to their
replacement pages (e.g., [DEMO guide](relative/path/to/demo.md) and
[authentication integration](relative/path/to/identity-service-doc.md)); ensure
the displayed link text remains descriptive and the target paths are correct for
the repo structure.
In `@docs/runbooks/production-deployment.md`:
- Around line 499-502: Add an explicit readiness probe check after the existing
"kubectl wait --for=condition=Available deployment/event-router -n production
--timeout=120s" step by verifying pod readiness for the deployment/event-router
(e.g., wait for all pods with the event-router selector to be Ready or use
"kubectl rollout status deployment/event-router") and fail the run if any pod
readiness/readyCount check does not reach expected replicas within a timeout;
update the post-deploy block in the doc to call this readiness check referencing
the deployment/event-router and the existing kubectl wait line so rollout
validation includes health probes, not just availability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ccbed7ec-3dd9-4c51-b8d5-b8db4b69035c
📒 Files selected for processing (16)
docs/archive/ARCHITECTURE.mddocs/archive/DEMO_GUIDE.mddocs/archive/README.mddocs/archive/audit-analysis-findings.mddocs/archive/authentication-integration.mddocs/archive/panic-audit-inventory.mddocs/guides/new-bian-service-checklist.mddocs/guides/starlark-built-ins-reference.mddocs/guides/testcontainers-usage.mddocs/runbooks/production-deployment.mddocs/runbooks/saga-validation-failure.mdservices/event-router/README.mdservices/event-router/k8s/README.mdservices/financial-gateway/README.mdservices/identity/README.mdshared/migrations/README.md
💤 Files with no reviewable changes (1)
- shared/migrations/README.md
… system services/README.md: - Rename Utilization Metering Consumer section to Event Router with saga triggering - Add missing services to architecture diagram (financial-gateway, operational-gateway, identity, mcp-server) - Add service description sections for all new services - Update gRPC and Kafka tables for event-router - Add new services to port reference table README.md: - Replace Starlark/CEL code examples with API usage examples (more approachable) - Expand architecture table from 8 to 14 services to reflect current system - Link to services/README.md for full architecture diagram
Contributor
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 `@README.md`:
- Around line 43-45: The README's curl example uses a literal placeholder "{id}"
in the URL (/v1/current-accounts/{id}/deposits) which can be copied verbatim and
break in shells; update the snippet to use a shell-substitutable variable (e.g.,
replace "{id}" with a variable like $ID) and show a one-line instruction to set
that variable before running the curl command so users can immediately
substitute the account id in the example.
In `@services/README.md`:
- Around line 88-90: The README has inconsistent RPC naming between EventRouter
→ ControlPlane: one place uses TriggerSaga and another uses ExecuteSaga; pick a
single RPC name and update both occurrences so they match (e.g., rename
TriggerSaga to ExecuteSaga or vice versa) and ensure the diagram line
"EventRouter -->|\"TriggerSaga (gRPC)\"| ControlPlane" and the textual reference
to ExecuteSaga() are identical, plus update any surrounding references or
signatures to the chosen method name to avoid ambiguity.
- Around line 189-190: The table entry uses an inconsistent Kafka topic pattern
'*.audit.events'; update it to match the example convention (use
'audit.events.<service>' or the wildcard form 'audit.events.*') so the pattern
is consistent with the later examples (e.g., the example topic format
'audit.events.<service>'). Edit the table row (the "All Services |
`*.audit.events` | EventRouter | Platform billing and saga triggering" entry) to
use 'audit.events.*' or 'audit.events.<service>' as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09ef6f44-70dd-4609-a36b-2c9837f4810c
📒 Files selected for processing (2)
README.mdservices/README.md
- testcontainers-usage.md: replace stale PostgreSQL code examples with
CockroachDB testdb.SetupCockroachDB pattern
- README.md: use shell variable for account ID instead of {id} placeholder
- services/README.md: use ExecuteSaga consistently (not TriggerSaga in diagram)
- services/README.md: fix Kafka topic pattern to audit.events.<service>
Replace all remaining SetupTestContainer/PostgresRepository references with testdb.SetupCockroachDB pattern throughout examples, best practices, troubleshooting, and migration sections.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/testcontainers-usage.md`:
- Around line 11-16: Update the overview to match the actual flow: state that
the helper sets up isolated CockroachDB containers, loads the service schema,
and returns a configured GORM DB handle (testdb.SetupCockroachDB / testhelpers),
but remove the claim that repositories are pre-configured — instead document
that callers must instantiate repo instances explicitly (e.g., call
repository.NewRepository(db)) after testdb.SetupCockroachDB(...)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f014763c-7094-4097-bd3b-0780ee8a07bf
📒 Files selected for processing (3)
README.mddocs/guides/testcontainers-usage.mdservices/README.md
Claude Code ReviewCommit: b0ae443 | CI: running (Markdown Lint passed) Summary: Clean, well-organized documentation housekeeping. The PR archives 5 stale docs, fixes broken cross-references, rewrites the event-router README from scratch, adds missing service READMEs for financial-gateway and identity, and updates the testcontainers guide from PostgreSQL to CockroachDB. The new service READMEs follow established conventions (YAML frontmatter). The event-router rewrite accurately reflects the current CEL-filtered saga dispatcher architecture. Risk: Low across all dimensions. Documentation only, no runtime behavior change, trivially revertable. Findings: No actionable findings. Bot Review Notes: Evaluated 7 unresolved CodeRabbit threads. All invalid or non-blocking. Topics: CockroachDB messaging (disagree), shell variable placeholder (already addressed), TriggerSaga vs ExecuteSaga (disagree, both use ExecuteSaga), Kafka topic pattern (disagree, consistent), testhelpers reference (disagree, uses testdb.SetupCockroachDB), additional analysis (consistent), topic config vars (minor, not a blocker). Previously Flagged: All 3 previous suggestions resolved (broken repo-test link - file exists; saga RPC name mismatch - consistent; stale SSE transport ref - now says streamable HTTP). |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Replace 'testhelpers' with 'testdb' in overview, remove stale 'pre-configured repository' bullet - Fix broken See Also link to actual test file path - Align TriggerSaga → ExecuteSaga in event-router Mermaid diagram
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/testcontainers-usage.md`:
- Around line 177-180: The Raw SQL example currently calls
db.Raw(...).Scan(&status) without checking the error; change it to capture the
error returned by Scan (err := db.Raw(..., log.LogID).Scan(&status).Error) and
assert success with require.NoError(t, err) before asserting the status value,
using the same require.NoError pattern as other tests and referencing db.Raw,
log.LogID and status.
In `@services/event-router/README.md`:
- Around line 50-53: The README currently documents consumption of both audit
and transaction events but only lists AUDIT_TOPICS; update the topic
configuration docs to be consistent by either adding a separate
TRANSACTION_TOPICS (or TRANSACTION_EVENT_TOPICS) variable and documenting its
expected format and usage, or rename AUDIT_TOPICS to a generic name like
EVENT_TOPICS/TOPICS and update references to AUDIT_TOPICS in the doc so the
description of the EventHandler and router behavior matches the actual config;
ensure the README entries around AUDIT_TOPICS (and any examples) are changed
accordingly and note which topics feed the saga/SagaExecutionService and billing
transformations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab652879-1e88-4ca5-8bdd-6c3f54dd0a0b
📒 Files selected for processing (2)
docs/guides/testcontainers-usage.mdservices/event-router/README.md
- Add require.NoError check on db.Raw().Scan().Error in testcontainers direct SQL example - Correct event-router purpose: consumes audit topics (not transaction)
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
services/event-router/README.md (1)
50-53:⚠️ Potential issue | 🟡 MinorTopic scope is still inconsistent between behavior and configuration.
Line 50 describes audit-topic consumption, while the architecture/diagram describes multi-topic routing (including transaction events), but config only exposes
AUDIT_TOPICS. Please align these sections with one model (EVENT_TOPICSgeneric, or explicitAUDIT_TOPICS+TRANSACTION_TOPICS).Suggested doc alignment
-| `AUDIT_TOPICS` | Yes | - | Comma-separated list of topics to consume | +| `EVENT_TOPICS` | Yes | - | Comma-separated list of domain-event topics to consume (audit + transaction) |Also applies to: 130-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/event-router/README.md` around lines 50 - 53, Documentation and configuration disagree on topic scope: README mentions audit-topic consumption while diagram and behavior imply multi-topic routing; update to a single consistent model. Change wording in services/event-router/README.md (sections around lines mentioning "Consuming domain events..." and later 130-131) to reference a unified config key (either rename/explain AUDIT_TOPICS to EVENT_TOPICS or explicitly document both AUDIT_TOPICS and TRANSACTION_TOPICS), update examples and config references to match the chosen model, and ensure the architecture diagram/description and any mentions of EventHandler, SagaExecutionService, and tenant-zero billing mapping reference the same topic model so code/config (AUDIT_TOPICS vs EVENT_TOPICS) and docs are aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@services/event-router/README.md`:
- Around line 50-53: Documentation and configuration disagree on topic scope:
README mentions audit-topic consumption while diagram and behavior imply
multi-topic routing; update to a single consistent model. Change wording in
services/event-router/README.md (sections around lines mentioning "Consuming
domain events..." and later 130-131) to reference a unified config key (either
rename/explain AUDIT_TOPICS to EVENT_TOPICS or explicitly document both
AUDIT_TOPICS and TRANSACTION_TOPICS), update examples and config references to
match the chosen model, and ensure the architecture diagram/description and any
mentions of EventHandler, SagaExecutionService, and tenant-zero billing mapping
reference the same topic model so code/config (AUDIT_TOPICS vs EVENT_TOPICS) and
docs are aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b9320841-08fb-41d7-8fa0-c63a0ce75c43
📒 Files selected for processing (2)
docs/guides/testcontainers-usage.mdservices/event-router/README.md
SSE transport was dropped; MCP server supports stdio and streamable HTTP only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/archive/(audit reports, demo guides, old auth integration)shared/migrations/README.md(abandoned shared-schema architecture)services/event-router/README.md— was still describing oldutilization-metering-consumerutilization-metering-consumer→event-routerin production deployment runbookstarlark-patterns.mdlinks →manifest-design-patterns.md(2 occurrences)SetupCockroachDB)financial-gatewayandidentityservicesTest plan
grep -r "utilization-metering-consumer" services/ docs/runbooks/ docs/guides/returns zero hitsgrep -r "starlark-patterns.md" docs/returns zero hitsdocs/archive/financial-gatewayandidentity