docs: add spec 03 (Databases) tasks file - #927
Open
HexSleeves wants to merge 1 commit into
Open
Conversation
Break down Chapter 11.4 Databases & Data Persistence spec into demoable units of work (SQL fundamentals, SQLAlchemy ORM, Repository pattern, query optimization, NoSQL, integration exercise) plus quiz and sidebar/front-matter integration tasks. Resolves #919
Contributor
📝 WalkthroughWalkthroughAdds the Chapter 11.4 database task specification with six demoable units, required files and proof artifacts, quiz instructions, documentation navigation updates, and concrete validation commands. ChangesDatabases task specification
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
![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: 5
🤖 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/specs/03-spec-databases/03-tasks-databases.md`:
- Line 7: Update the Markdown headings in this document, including Relevant
Files, Tasks, each unit section, Notes, and Testing Commands, from H2 to H3.
Also revise the instruction for the target chapter page to require H3 content
headings; reserve H2 exclusively for the navigation table of contents.
- Around line 297-301: Update task 6.8 to require a base repository and concrete
repositories with entity-specific query methods for every entity in the
integration schema, not only User, Post, and Comment. Keep task 6.9’s
requirement that all entity CRUD routes use those repositories, and align the
wording with the all-entities coverage expected by task 6.12.
- Around line 11-18: Update the database documentation specification so every
referenced or newly created image uses the repository root img/ directory
instead of docs/11-application-development/img11/. Revise the image inventory,
creation tasks, embed paths, and related notes consistently across the
referenced sections, while retaining the requirement to use HTML <img> tags.
- Around line 23-24: Update the Unit 1 dependency description in the database
tasks specification to use terminology consistent with the manifests: reserve
“pinned” for exact versions, otherwise describe dependencies as
version-constrained. Align any shared Flask/SQLAlchemy constraint wording across
the affected unit descriptions, while preserving the note that Unit 1 requires
no external dependencies.
- Around line 366-368: Update the verification commands in the database task
documentation: replace the literal unit-N seed path with a shell loop that
executes every matching unit-*/solution/seed.py script, and replace the single
command containing “vs” with two explicit SQLALCHEMY_ECHO=True app.py commands
for the starter and solution applications.
🪄 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: c982b0fb-7fc4-403e-a5b0-2b3d42be6bfc
📒 Files selected for processing (1)
docs/specs/03-spec-databases/03-tasks-databases.md
|
|
||
| Students are first-time learners partway through a six-month bootcamp, so exercise time estimates in front-matter are intentionally generous (they include reading, experimentation, and debugging time, not just "typing the solution" time). | ||
|
|
||
| ## Relevant Files |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use H3 for documentation content headings.
This file uses H2 for content sections such as Relevant Files, Tasks, every unit, Notes, and Testing Commands; it also instructs the target chapter page to create H2 content sections. Use H3 by default and reserve H2 for a navigation table of contents.
As per coding guidelines: docs/**/*.md should use H3 headers by default, with H2 reserved for navigation table of contents.
Also applies to: 113-115, 149-151, 180-182, 213-215, 245-247, 277-279, 306-308, 329-331, 351-351, 364-364
🤖 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/specs/03-spec-databases/03-tasks-databases.md` at line 7, Update the
Markdown headings in this document, including Relevant Files, Tasks, each unit
section, Notes, and Testing Commands, from H2 to H3. Also revise the instruction
for the target chapter page to require H3 content headings; reserve H2
exclusively for the navigation table of contents.
Source: Coding guidelines
Comment on lines
+11
to
+18
| - `docs/11-application-development/11.4-databases.md` - Main content page for Chapter 11.4 including all teaching content, exercises, and front-matter metadata | ||
| - `docs/11-application-development/img11/` - Directory for ER diagrams and architecture diagrams referenced by 11.4 | ||
|
|
||
| ### Images and Diagrams | ||
|
|
||
| - `docs/11-application-development/img11/library-schema-erd.png` - Rendered ER diagram for the Unit 1 library schema design exercise | ||
| - `docs/11-application-development/img11/repository-pattern-layers.png` - Rendered architecture diagram showing Repository pattern layers (11.2.2 abstract pattern next to concrete SQLAlchemy implementation) | ||
| - `docs/11-application-development/img11/n-plus-one-before-after.png` - Optional diagram/screenshot comparing query counts before/after fixing an N+1 problem |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Place referenced images in the root img/ folder.
The task repeatedly requires images under docs/11-application-development/img11/, which conflicts with the repository rule and can produce incorrect asset paths. Update the inventory, creation tasks, embeds, and notes consistently; the HTML <img> requirement itself is correct.
As per coding guidelines: use HTML <img> tags for images and place all images in the root img/ folder.
Also applies to: 122-145, 187-199, 241-241, 358-358
🤖 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/specs/03-spec-databases/03-tasks-databases.md` around lines 11 - 18,
Update the database documentation specification so every referenced or newly
created image uses the repository root img/ directory instead of
docs/11-application-development/img11/. Revise the image inventory, creation
tasks, embed paths, and related notes consistently across the referenced
sections, while retaining the requirement to use HTML <img> tags.
Source: Coding guidelines
Comment on lines
+23
to
+24
| - `examples/ch11/databases/unit-1-sql/requirements.txt` and `pyproject.toml` - Pinned Python 3.11+ dependencies (stdlib `sqlite3`, no external deps required for this unit) | ||
| - `examples/ch11/databases/unit-1-sql/starter/init_db.py` - Starting point script that creates an empty SQLite file and a connection helper |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'pinned|>=|<3\.0|requirements\.txt|pyproject\.toml' \
docs/specs/03-spec-databases/03-tasks-databases.mdRepository: liatrio/engineering-bootcamp
Length of output: 3061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate relevant database example manifests:"
fd -H '^(requirements.txt|pyproject.toml)$' examples/ch11/databases 2>/dev/null | sort | sed -n '1,80p'
echo
echo "Inspect found manifest files with content type indicators:"
while IFS= read -r f; do
echo "--- $f ---"
sed -n '1,120p' "$f" | sed -n '1,120p'
done < <(fd -H '^(requirements.txt|pyproject.toml)$' examples/ch11/databases 2>/dev/null | sort)
echo
echo "Search for exact pins vs ranges in these manifests:"
python3 - <<'PY'
import re, pathlib
for p in sorted(pathlib.Path('examples/ch11/databases').rglob('*')):
if p.name not in {'requirements.txt','pyproject.toml'}:
continue
text = p.read_text(encoding='utf-8', errors='replace')
if not text.strip():
continue
print(f'--- {p} ---')
# Print package declarations involving version operators, keeping dependency blocks concise.
lines = [line for line in text.splitlines() if re.search(r'(==|!=|>=|<=|>|<|~=)', line)]
for line in lines:
print(line.strip())
PYRepository: liatrio/engineering-bootcamp
Length of output: 210
Clarify whether “pinned” means exact versions or version constraints.
Unit 2 and 6 explicitly list SQLAlchemy>=2.0,<3.0, while Unit 1 says no external dependencies are required. Use the same terminology throughout, and only use “pinned” if the manifests are actually exact; otherwise call them version-constrained and ensure any shared Flask/SQLAlchemy constraints are documented consistently across units.
🤖 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/specs/03-spec-databases/03-tasks-databases.md` around lines 23 - 24,
Update the Unit 1 dependency description in the database tasks specification to
use terminology consistent with the manifests: reserve “pinned” for exact
versions, otherwise describe dependencies as version-constrained. Align any
shared Flask/SQLAlchemy constraint wording across the affected unit
descriptions, while preserving the note that Unit 1 requires no external
dependencies.
Comment on lines
+297
to
+301
| - [ ] 6.8 Create `examples/ch11/databases/unit-6-integration/solution/repositories/` with a `base_repository.py` and concrete repositories for at least User, Post, and Comment, each with at least one entity-specific query method | ||
| - [ ] 6.9 Create `examples/ch11/databases/unit-6-integration/solution/app.py` implementing full CRUD routes for all entities via repositories, using `selectinload`/`joinedload` on list views to prevent N+1 queries | ||
| - [ ] 6.10 Create `examples/ch11/databases/unit-6-integration/solution/seed.py` seeding realistic fictional sample data (no real names/emails) across all entities | ||
| - [ ] 6.11 Optionally add Redis caching to one frequently-accessed route (e.g., popular posts) in `solution/app.py`, with `examples/ch11/databases/unit-6-integration/solution/docker-compose.yml` providing the Redis service (reusing the Unit 5 pattern), clearly marked as an optional extension | ||
| - [ ] 6.12 Create `examples/ch11/templates/database-self-assessment.md` with a checklist organized by category: Schema Design (3NF? appropriate relationships? constraints?), ORM Usage (models correctly mapped? relationships defined?), Repository Pattern (base + concrete repositories implemented for all entities? routes use repositories, not direct session access?), Optimization (indexes added? N+1 queries prevented? eager loading used on list views?), and Overall (references 11.2.2 concepts? application runs end-to-end?) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align repository coverage with the integration acceptance criteria.
Line 297 requires repositories for only User, Post, and Comment, while Lines 298 and 301 require repository-backed CRUD and concrete repositories for all entities. Make the requirement consistent; the current wording allows a solution to satisfy one task while failing the proof checklist.
🤖 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/specs/03-spec-databases/03-tasks-databases.md` around lines 297 - 301,
Update task 6.8 to require a base repository and concrete repositories with
entity-specific query methods for every entity in the integration schema, not
only User, Post, and Comment. Keep task 6.9’s requirement that all entity CRUD
routes use those repositories, and align the wording with the all-entities
coverage expected by task 6.12.
Comment on lines
+366
to
+368
| - `python examples/ch11/databases/unit-N-*/solution/seed.py` - Seed each unit's database with sample data | ||
| - `docker compose up -d` (in `unit-5-nosql/redis-caching/` and `unit-5-nosql/document-store/`) - Verify NoSQL dependencies start | ||
| - `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/starter/app.py` vs `solution/app.py` - Compare query logs |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the verification commands executable.
unit-N-* is a literal placeholder that matches no directory, and vs is passed as an argument to the first Python process rather than running a second command. Replace these with a loop over unit-*/solution/seed.py and two explicit app commands.
Suggested replacement
-- `python examples/ch11/databases/unit-N-*/solution/seed.py`
+- `for script in examples/ch11/databases/unit-*/solution/seed.py; do python "$script"; done`
-- `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/starter/app.py` vs `solution/app.py`
+- `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/starter/app.py`
+- `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/solution/app.py`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `python examples/ch11/databases/unit-N-*/solution/seed.py` - Seed each unit's database with sample data | |
| - `docker compose up -d` (in `unit-5-nosql/redis-caching/` and `unit-5-nosql/document-store/`) - Verify NoSQL dependencies start | |
| - `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/starter/app.py` vs `solution/app.py` - Compare query logs | |
| - `for script in examples/ch11/databases/unit-*/solution/seed.py; do python "$script"; done` - Seed each unit's database with sample data | |
| - `docker compose up -d` (in `unit-5-nosql/redis-caching/` and `unit-5-nosql/document-store/`) - Verify NoSQL dependencies start | |
| - `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/starter/app.py` | |
| - `SQLALCHEMY_ECHO=True python examples/ch11/databases/unit-4-optimization/solution/app.py` |
🤖 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/specs/03-spec-databases/03-tasks-databases.md` around lines 366 - 368,
Update the verification commands in the database task documentation: replace the
literal unit-N seed path with a shell loop that executes every matching
unit-*/solution/seed.py script, and replace the single command containing “vs”
with two explicit SQLALCHEMY_ECHO=True app.py commands for the starter and
solution applications.
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/specs/03-spec-databases/03-tasks-databases.md, breaking Chapter 11.4 (Databases & Data Persistence) into 8 demoable task groups: SQL fundamentals/schema design, SQLAlchemy ORM, Repository pattern (building on 11.2.2), indexes/N+1 query optimization, NoSQL (Redis + document store), the integration exercise, quiz, and sidebar/front-matter integration.01-tasks-design-patterns-section.mdand02-tasks-system-thinking.md(Relevant Files, task groups with Purpose + Proof Artifact(s) + numbered subtasks).03-questions-1-databases.md(intermediate SQL depth, multiple NoSQL types, SQLAlchemy, progressive exercises, Repository pattern focus, basic query optimization).Closes #919
Related to #915
Test plan
npx markdownlint-cli2on the new file passes with 0 issuesnpm run lint(via pre-commit hook) passes with 0 issues across 173 filesSummary by CodeRabbit