Walk/nowakfli/ex1 architecture map#129
Closed
nowakfli wants to merge 22 commits into
Closed
Conversation
GHCP — Crawl: Ex0 bootstrap
GHCP — Crawl: Ex1 repo-orientation
…eline GHCP — Crawl: Ex2 build-test-baseline
GHCP — Crawl: Ex3 tiny-refactor
…sync GHCP — Crawl: Ex4 documentation-sync
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds “exercise 1” orientation artifacts (system overview + architecture map) and strengthens the Python book-app sample with safer input handling, optional CLI display toggles, and a growing baseline test/docs set to support the AI Engineering “Crawl” track.
Changes:
- Added helper parsing + docstrings in
utils.pyand introduced new pytest coverage for utils + CLI behaviors. - Improved resilience/validation in the Python sample (
BookCollection.load_books()OSError handling,add_book()validation, structured logging inhandle_add(), and a year-display toggle). - Added track documentation (system overview, build/test notes, toggles, logging, perf baseline, backlog) plus repo hygiene updates (
.gitignore, track README).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/book-app-project/utils.py | Adds docstrings and a parse_year() helper used by get_book_details(). |
| samples/book-app-project/tests/test_utils.py | Adds pytest coverage for parse_year() and get_book_details() using monkeypatched input. |
| samples/book-app-project/tests/test_books.py | Expands tests for validation, CLI add behavior/logging, display toggle, and load error resilience. |
| samples/book-app-project/run_tests.py | Adds a simple python run_tests.py wrapper to run pytest from the sample directory. |
| samples/book-app-project/pyproject.toml | Pins pytest to a bounded major version range (>=8,<9). |
| samples/book-app-project/books.py | Adds read-time OSError fallback and validates inputs in add_book(). |
| samples/book-app-project/book_app.py | Adds structured logging for handle_add() and an env toggle controlling year display. |
| samples/book-app-project/benchmark_list_books.py | Adds a micro-benchmark script for BookCollection.list_books(). |
| ai-track-docs/toggles.md | Documents the BOOK_APP_SHOW_YEAR toggle and how to try it locally. |
| ai-track-docs/SYSTEM-OVERVIEW.md | Provides a repository/system overview and highlights low-risk modules for exercises. |
| ai-track-docs/static-analysis.md | Notes current lack of lint/static-analysis tooling and suggests low-friction checks. |
| ai-track-docs/security.md | Documents secret-hygiene checks and .gitignore improvements. |
| ai-track-docs/resilience.md | Documents the new load_books() OSError fallback behavior and its tests. |
| ai-track-docs/pr-hygiene.md | Defines review focus, evidence expectations, and rollback guidance for the track. |
| ai-track-docs/perf-baseline.md | Captures a baseline benchmark workflow and example results. |
| ai-track-docs/logging.md | Documents the structured logging fields and a local verification path. |
| ai-track-docs/extending-utils.md | Adds guidance on extending utils.py (but currently contains corrupted trailing content). |
| ai-track-docs/dependencies.md | Documents dependency policy and current gaps for the Python sample. |
| ai-track-docs/build-test.md | Documents build/test commands and a local validation script (includes a coverage command that needs dependency alignment). |
| ai-track-docs/BACKLOG-EX12.md | Tracks follow-up backlog items (deps layout, logging parity, data path override, etc.). |
| ai-track-docs/architecture.mmd | Adds a Mermaid architecture diagram linking docs, code, tests, and scripts. |
| .gitignore | Expands ignore rules for common secret/certificate artifacts. |
| .copilot-track/crawl/README.md | Adds Crawl-track process/guardrails documentation and repo structure notes. |
| raise ValueError("Author cannot be empty.") | ||
| if not (0 <= year <= 9999): | ||
| raise ValueError("Year must be between 0 and 9999.") | ||
|
|
Comment on lines
+8
to
+10
| logging.basicConfig(level=logging.INFO, format="%(message)s") | ||
| logger = logging.getLogger("book_app") | ||
|
|
Comment on lines
65
to
76
| try: | ||
| year = int(year_str) if year_str else 0 | ||
| if year < 0 or year > 9999: | ||
| raise ValueError("Year must be between 0 and 9999.") | ||
| collection.add_book(title, author, year) | ||
| log_operation("add_book", "success", started_at, year=year) | ||
| print("\nBook added successfully.\n") | ||
| except ValueError as e: | ||
| log_operation("add_book", "validation_error", started_at, reason="invalid_year") | ||
| print(f"\nError: {e}\n") | ||
|
|
||
|
|
Comment on lines
+19
to
+23
| ```bash | ||
| cd samples/book-app-project | ||
| $env:BOOK_APP_SHOW_YEAR="1" | ||
| python book_app.py list | ||
| ``` |
Comment on lines
+116
to
+117
| - Input functions return validated data to the main application logic</content> | ||
| <parameter name="filePath">c:\crawl-walk-run\copilot-cli-for-beginners-mnf\ai-track-docs\extending-utils.md No newline at end of file |
Comment on lines
+7
to
+27
| def run_benchmark(iterations: int = 10000, book_count: int = 1000) -> dict[str, float]: | ||
| collection = BookCollection() | ||
| collection.books = [ | ||
| Book(title=f"Book {index}", author="Benchmark Author", year=2000 + (index % 20)) | ||
| for index in range(book_count) | ||
| ] | ||
|
|
||
| samples = [] | ||
| for _ in range(iterations): | ||
| start = time.perf_counter() | ||
| collection.list_books() | ||
| samples.append((time.perf_counter() - start) * 1_000_000) | ||
|
|
||
| return { | ||
| "iterations": iterations, | ||
| "book_count": book_count, | ||
| "mean_us": statistics.mean(samples), | ||
| "median_us": statistics.median(samples), | ||
| "min_us": min(samples), | ||
| "max_us": max(samples), | ||
| } |
Comment on lines
+45
to
+48
| ### Run Tests with Coverage Report | ||
| ```bash | ||
| cd samples/book-app-project | ||
| pytest tests/ --cov=. --cov-report=term-missing |
Collaborator
|
Thank you, but this isn’t something we’re looking to as to the course right now so going to close this PR. |
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
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.
No description provided.