refactor: reorganize repository documentation into docs directory - #688
refactor: reorganize repository documentation into docs directory#688shivanimishra2006 wants to merge 1 commit into
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe PR adds comprehensive product and engineering documentation, updates the README repository tree with ChangesProject documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
docs/development-phases.md (2)
27-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the staging approval gate explicit.
“Final deploy to staging/production” is ambiguous and does not reflect the documented requirement that stakeholders sign off in staging before production release. Split this into ordered staging deployment, validation/sign-off, and production deployment steps.
🤖 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 `@docs/development-phases.md` around lines 27 - 32, Update the Phase 4 deployment bullet in the documentation to explicitly sequence deployment to staging, stakeholder validation and approval in staging, and deployment to production only after sign-off. Replace the ambiguous “Final deploy to staging/production” wording while preserving the surrounding phase scope.
23-25: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDocument how Celery Beat is deployed.
Phase 3 requires Celery Beat, but
docs/environment-and-devops.mdonly defines a worker tier. Specify whether Beat runs as a separate singleton process or alongside workers, including its scaling/leader-election behavior; otherwise scheduled campaign execution may never run or may run multiple times.🤖 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 `@docs/development-phases.md` around lines 23 - 25, Update the Phase 3 documentation near the Celery Beat requirement to describe its deployment model: state whether Beat runs as a separate singleton process or with workers, and document the scaling or leader-election behavior that prevents missed or duplicate scheduled campaign execution. Align the description with the worker deployment model in environment-and-devops documentation.
🤖 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/api-contracts.md`:
- Around line 42-44: The enroll endpoint contract uses enrolled_count in the
documentation while the implementation returns total_enrolled. Align the
documented response with the key returned by the enroll view, or update the view
to return the documented key, and add a regression test asserting the exact JSON
response key for POST /api/v1/campaigns/{id}/enroll/.
In `@docs/architecture.md`:
- Around line 17-36: Update the architecture flow diagram so Firebase is shown
as a side-channel from the Django/API or worker layer for realtime
notifications, dashboards, and UI synchronization rather than in the primary
PostgreSQL-to-worker sequence. Reorder the downstream flow so Celery workers
obtain content from the Gemini AI Service before dispatching messages through
the Messaging Providers.
In `@docs/CAMPAIGN_COUNTERS_CACHE.md`:
- Around line 74-80: Update the Notes section of CAMPAIGN_COUNTERS_CACHE.md to
remove the unconditional claim that signals prevent race conditions, unless the
signal implementation is changed to provide transaction.atomic() and appropriate
locking. Prefer documenting that concurrent CampaignLead updates may temporarily
leave cached counters stale.
- Around line 23-24: Update the API Changes section and the related statements
at the referenced locations to scope cached counters specifically to list/detail
responses. Clarify that time-series and aggregate analytics remain query-backed
and may continue using SUM() or other aggregate calculations, rather than
claiming all serializers and views use cached counters.
In `@docs/database-schema.md`:
- Around line 72-77: Update the MessageEvent event_type definition to include
SENT and UNSUBSCRIBED, or explicitly document that these states are represented
by Message.status and Lead.global_unsubscribe rather than analytics events.
Ensure the schema clearly satisfies the required tracking behavior without
leaving either event type ambiguous.
In `@docs/engineering-scope.md`:
- Around line 15-18: Reconcile capability-status labeling across the three
documentation sections: in docs/engineering-scope.md lines 15-18, explicitly
identify whether the EMAIL/WAIT-only Campaign Engine scope is historical, MVP
target, or current; in docs/information-architecture.md lines 36-39, label SMS,
LinkedIn, and OAuth mailbox flows as deferred or planned when outside MVP; and
in docs/mvp-tech.md lines 16-20, assign each messaging integration a clear MVP,
current, or deferred status.
In `@docs/environment-and-devops.md`:
- Around line 27-32: Update the “Workflow Steps on Merge to main” sequence so
Django migrations run through an explicit migration job before triggering the
Web and Worker rolling restart. Preserve the existing CI, image build, and
registry push steps, and document health checks or rollback handling if required
by the deployment process.
- Around line 3-16: Extend the “Environments” section of the deployment plan
with a per-environment secret-management policy covering storage, runtime
injection, access control, and rotation for JWT, Gemini, SMTP, Firebase,
PostgreSQL, and Redis credentials. Distinguish local development handling from
staging and production, and specify that production secrets use a managed secret
store rather than repository files or plaintext configuration.
In `@docs/monorepo-structure.md`:
- Around line 3-47: Mark the layout in docs/monorepo-structure.md as
future-state, or revise it to match the actual repository tree documented in
README.md; do not present the apps/worker/infra structure as current. In
docs/architecture.md lines 48-61, use the same authoritative package paths and
explicitly distinguish the target architecture from the deployed structure.
---
Nitpick comments:
In `@docs/development-phases.md`:
- Around line 27-32: Update the Phase 4 deployment bullet in the documentation
to explicitly sequence deployment to staging, stakeholder validation and
approval in staging, and deployment to production only after sign-off. Replace
the ambiguous “Final deploy to staging/production” wording while preserving the
surrounding phase scope.
- Around line 23-25: Update the Phase 3 documentation near the Celery Beat
requirement to describe its deployment model: state whether Beat runs as a
separate singleton process or with workers, and document the scaling or
leader-election behavior that prevents missed or duplicate scheduled campaign
execution. Align the description with the worker deployment model in
environment-and-devops documentation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: def1531a-000f-42ea-848b-1ba264b6445d
📒 Files selected for processing (13)
README.mddocs/CAMPAIGN_COUNTERS_CACHE.mddocs/api-contracts.mddocs/architecture.mddocs/database-schema.mddocs/development-phases.mddocs/engineering-scope.mddocs/environment-and-devops.mddocs/information-architecture.mddocs/monorepo-structure.mddocs/mvp-tech.mddocs/prd.mddocs/product-requirements.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
🧹 Nitpick comments (2)
docs/development-phases.md (2)
27-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the staging approval gate explicit.
“Final deploy to staging/production” is ambiguous and does not reflect the documented requirement that stakeholders sign off in staging before production release. Split this into ordered staging deployment, validation/sign-off, and production deployment steps.
🤖 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 `@docs/development-phases.md` around lines 27 - 32, Update the Phase 4 deployment bullet in the documentation to explicitly sequence deployment to staging, stakeholder validation and approval in staging, and deployment to production only after sign-off. Replace the ambiguous “Final deploy to staging/production” wording while preserving the surrounding phase scope.
23-25: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDocument how Celery Beat is deployed.
Phase 3 requires Celery Beat, but
docs/environment-and-devops.mdonly defines a worker tier. Specify whether Beat runs as a separate singleton process or alongside workers, including its scaling/leader-election behavior; otherwise scheduled campaign execution may never run or may run multiple times.🤖 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 `@docs/development-phases.md` around lines 23 - 25, Update the Phase 3 documentation near the Celery Beat requirement to describe its deployment model: state whether Beat runs as a separate singleton process or with workers, and document the scaling or leader-election behavior that prevents missed or duplicate scheduled campaign execution. Align the description with the worker deployment model in environment-and-devops documentation.
🤖 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/api-contracts.md`:
- Around line 42-44: The enroll endpoint contract uses enrolled_count in the
documentation while the implementation returns total_enrolled. Align the
documented response with the key returned by the enroll view, or update the view
to return the documented key, and add a regression test asserting the exact JSON
response key for POST /api/v1/campaigns/{id}/enroll/.
In `@docs/architecture.md`:
- Around line 17-36: Update the architecture flow diagram so Firebase is shown
as a side-channel from the Django/API or worker layer for realtime
notifications, dashboards, and UI synchronization rather than in the primary
PostgreSQL-to-worker sequence. Reorder the downstream flow so Celery workers
obtain content from the Gemini AI Service before dispatching messages through
the Messaging Providers.
In `@docs/CAMPAIGN_COUNTERS_CACHE.md`:
- Around line 74-80: Update the Notes section of CAMPAIGN_COUNTERS_CACHE.md to
remove the unconditional claim that signals prevent race conditions, unless the
signal implementation is changed to provide transaction.atomic() and appropriate
locking. Prefer documenting that concurrent CampaignLead updates may temporarily
leave cached counters stale.
- Around line 23-24: Update the API Changes section and the related statements
at the referenced locations to scope cached counters specifically to list/detail
responses. Clarify that time-series and aggregate analytics remain query-backed
and may continue using SUM() or other aggregate calculations, rather than
claiming all serializers and views use cached counters.
In `@docs/database-schema.md`:
- Around line 72-77: Update the MessageEvent event_type definition to include
SENT and UNSUBSCRIBED, or explicitly document that these states are represented
by Message.status and Lead.global_unsubscribe rather than analytics events.
Ensure the schema clearly satisfies the required tracking behavior without
leaving either event type ambiguous.
In `@docs/engineering-scope.md`:
- Around line 15-18: Reconcile capability-status labeling across the three
documentation sections: in docs/engineering-scope.md lines 15-18, explicitly
identify whether the EMAIL/WAIT-only Campaign Engine scope is historical, MVP
target, or current; in docs/information-architecture.md lines 36-39, label SMS,
LinkedIn, and OAuth mailbox flows as deferred or planned when outside MVP; and
in docs/mvp-tech.md lines 16-20, assign each messaging integration a clear MVP,
current, or deferred status.
In `@docs/environment-and-devops.md`:
- Around line 27-32: Update the “Workflow Steps on Merge to main” sequence so
Django migrations run through an explicit migration job before triggering the
Web and Worker rolling restart. Preserve the existing CI, image build, and
registry push steps, and document health checks or rollback handling if required
by the deployment process.
- Around line 3-16: Extend the “Environments” section of the deployment plan
with a per-environment secret-management policy covering storage, runtime
injection, access control, and rotation for JWT, Gemini, SMTP, Firebase,
PostgreSQL, and Redis credentials. Distinguish local development handling from
staging and production, and specify that production secrets use a managed secret
store rather than repository files or plaintext configuration.
In `@docs/monorepo-structure.md`:
- Around line 3-47: Mark the layout in docs/monorepo-structure.md as
future-state, or revise it to match the actual repository tree documented in
README.md; do not present the apps/worker/infra structure as current. In
docs/architecture.md lines 48-61, use the same authoritative package paths and
explicitly distinguish the target architecture from the deployed structure.
---
Nitpick comments:
In `@docs/development-phases.md`:
- Around line 27-32: Update the Phase 4 deployment bullet in the documentation
to explicitly sequence deployment to staging, stakeholder validation and
approval in staging, and deployment to production only after sign-off. Replace
the ambiguous “Final deploy to staging/production” wording while preserving the
surrounding phase scope.
- Around line 23-25: Update the Phase 3 documentation near the Celery Beat
requirement to describe its deployment model: state whether Beat runs as a
separate singleton process or with workers, and document the scaling or
leader-election behavior that prevents missed or duplicate scheduled campaign
execution. Align the description with the worker deployment model in
environment-and-devops documentation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: def1531a-000f-42ea-848b-1ba264b6445d
📒 Files selected for processing (13)
README.mddocs/CAMPAIGN_COUNTERS_CACHE.mddocs/api-contracts.mddocs/architecture.mddocs/database-schema.mddocs/development-phases.mddocs/engineering-scope.mddocs/environment-and-devops.mddocs/information-architecture.mddocs/monorepo-structure.mddocs/mvp-tech.mddocs/prd.mddocs/product-requirements.md
🛑 Comments failed to post (9)
docs/api-contracts.md (1)
42-44: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the implementation’s response key or change the endpoint.
The documented enroll response uses
enrolled_count, butbackend/campaigns/views.pyreturnstotal_enrolled. This is a client-visible contract mismatch; update the documentation or implementation and add a regression test for the exact JSON key.🤖 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 `@docs/api-contracts.md` around lines 42 - 44, The enroll endpoint contract uses enrolled_count in the documentation while the implementation returns total_enrolled. Align the documented response with the key returned by the enroll view, or update the view to return the documented key, and add a regression test asserting the exact JSON response key for POST /api/v1/campaigns/{id}/enroll/.docs/architecture.md (1)
17-36: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the system-flow diagram.
The diagram places Firebase in the primary PostgreSQL-to-worker path and places Gemini after messaging providers. Firebase should be a side-channel for notifications/UI sync, while workers should obtain AI content before dispatching messages through providers.
🤖 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 `@docs/architecture.md` around lines 17 - 36, Update the architecture flow diagram so Firebase is shown as a side-channel from the Django/API or worker layer for realtime notifications, dashboards, and UI synchronization rather than in the primary PostgreSQL-to-worker sequence. Reorder the downstream flow so Celery workers obtain content from the Gemini AI Service before dispatching messages through the Messaging Providers.docs/CAMPAIGN_COUNTERS_CACHE.md (2)
23-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the cached-counter claim to the endpoints that actually use it.
This says serializers and views no longer compute aggregates, but the same document states that analytics still uses
SUM()andviews.pyuses aggregate calculations. Clarify that list/detail responses use cached fields while time-series or aggregate analytics remain query-backed.Also applies to: 60-61, 69-70
🤖 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 `@docs/CAMPAIGN_COUNTERS_CACHE.md` around lines 23 - 24, Update the API Changes section and the related statements at the referenced locations to scope cached counters specifically to list/detail responses. Clarify that time-series and aggregate analytics remain query-backed and may continue using SUM() or other aggregate calculations, rather than claiming all serializers and views use cached counters.
74-80: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not claim atomic race prevention unless the signal implementation provides it.
The supplied
backend/campaigns/signals.pyrecalculates counters and callssave(update_fields=...), but does not usetransaction.atomic()or locking. Under concurrentCampaignLeadchanges, cached values can still become stale. Either implement the stated synchronization guarantee or document the remaining consistency limitations.🤖 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 `@docs/CAMPAIGN_COUNTERS_CACHE.md` around lines 74 - 80, Update the Notes section of CAMPAIGN_COUNTERS_CACHE.md to remove the unconditional claim that signals prevent race conditions, unless the signal implementation is changed to provide transaction.atomic() and appropriate locking. Prefer documenting that concurrent CampaignLead updates may temporarily leave cached counters stale.docs/database-schema.md (1)
72-77: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add or explicitly model the required SENT and UNSUBSCRIBED events.
The requirements call for tracking Sent and Unsubscribed events, but
MessageEvent.event_typeomits both. Either add those event types or document that sent state and unsubscribe state are intentionally stored inMessage.statusandLead.global_unsubscriberather than analytics events.🤖 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 `@docs/database-schema.md` around lines 72 - 77, Update the MessageEvent event_type definition to include SENT and UNSUBSCRIBED, or explicitly document that these states are represented by Message.status and Lead.global_unsubscribe rather than analytics events. Ensure the schema clearly satisfies the required tracking behavior without leaving either event type ambiguous.docs/engineering-scope.md (1)
15-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reconcile MVP scope and channel availability across the documentation.
The documents mix current capabilities, the MVP target, and deferred roadmap items without labeling which state each list represents.
docs/engineering-scope.md#L15-L18: define whether the Email/WAIT-only scope is historical, target-state, or current.docs/information-architecture.md#L36-L39: mark SMS, LinkedIn, and OAuth mailbox flows as deferred/planned if they are outside MVP.docs/mvp-tech.md#L16-L20: label each messaging integration by its MVP/current/deferred status.📍 Affects 3 files
docs/engineering-scope.md#L15-L18(this comment)docs/information-architecture.md#L36-L39docs/mvp-tech.md#L16-L20🤖 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 `@docs/engineering-scope.md` around lines 15 - 18, Reconcile capability-status labeling across the three documentation sections: in docs/engineering-scope.md lines 15-18, explicitly identify whether the EMAIL/WAIT-only Campaign Engine scope is historical, MVP target, or current; in docs/information-architecture.md lines 36-39, label SMS, LinkedIn, and OAuth mailbox flows as deferred or planned when outside MVP; and in docs/mvp-tech.md lines 16-20, assign each messaging integration a clear MVP, current, or deferred status.docs/environment-and-devops.md (2)
3-16: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Document production secret management.
The planned system requires sensitive credentials for JWT signing, Gemini, SMTP, Firebase, PostgreSQL, and Redis, but the environment strategy does not specify secret storage, injection, access control, or rotation. Define the per-environment secret-management policy before treating the deployment plan as complete.
🤖 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 `@docs/environment-and-devops.md` around lines 3 - 16, Extend the “Environments” section of the deployment plan with a per-environment secret-management policy covering storage, runtime injection, access control, and rotation for JWT, Gemini, SMTP, Firebase, PostgreSQL, and Redis credentials. Distinguish local development handling from staging and production, and specify that production secrets use a managed secret store rather than repository files or plaintext configuration.
27-32: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Run migrations before the rolling application restart.
The documented order deploys/restarts Web and Worker tiers, then runs migrations. New code can therefore start against the old schema, causing rollout failures or partial outages. Use an explicit migration job before rollout, or document an expand/contract migration strategy with health checks and rollback handling.
🤖 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 `@docs/environment-and-devops.md` around lines 27 - 32, Update the “Workflow Steps on Merge to main” sequence so Django migrations run through an explicit migration job before triggering the Web and Worker rolling restart. Preserve the existing CI, image build, and registry push steps, and document health checks or rollback handling if required by the deployment process.docs/monorepo-structure.md (1)
3-47: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use one authoritative repository layout.
The monorepo and architecture documents describe a future
apps/worker/infrastructure that does not match the current layout documented in README.md.
docs/monorepo-structure.md#L3-L47: label this explicitly as future-state or update it to the actual tree.docs/architecture.md#L48-L61: use the same authoritative package paths and distinguish target architecture from deployed structure.🧰 Tools
🪛 LanguageTool
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...repo structure separating the Stitch MCP generated frontend from the Django backe...(QB_NEW_EN_HYPHEN)
📍 Affects 2 files
docs/monorepo-structure.md#L3-L47(this comment)docs/architecture.md#L48-L61🤖 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 `@docs/monorepo-structure.md` around lines 3 - 47, Mark the layout in docs/monorepo-structure.md as future-state, or revise it to match the actual repository tree documented in README.md; do not present the apps/worker/infra structure as current. In docs/architecture.md lines 48-61, use the same authoritative package paths and explicitly distinguish the target architecture from the deployed structure.
1 participant
Pull Request
🔗 Related Issue
Closes #
📝 Summary of Changes
🏷️ Type of Change
🧪 Testing
Steps to test:
1.
2.
3.
📸 Screenshots (if applicable)
✅ Checklist
Summary by CodeRabbit
docs/directory.