Dev#86
Conversation
|
@PavelEgorov-ru is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
📝 WalkthroughWalkthroughEstablishes project collaboration rules between Claude and user, configures Windows/WSL/Cursor development setup, applies Ruff linting/formatting via VS Code and pyproject.toml, and delivers complete Russian-language Phase 00 curriculum (six lessons) covering dev environments, Git, GPU access, APIs, Jupyter notebooks, and Python virtual environments with practical code examples and exercises. ChangesProject Collaboration Infrastructure
Phase 00 Course Content
Poem
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.claude/CLAUDE.md:
- Around line 80-82: Two fenced code blocks containing shell/PowerShell examples
(the block with git -C "$env:USERPROFILE\Desktop\ai-engineering-from-scratch"
pull and the block with the wsl commands) are missing language identifiers; add
"powershell" (or "bash" if you prefer) after the opening triple backticks for
each fenced block so markdownlint MD040 is satisfied, e.g., change ``` to
```powershell for both the git example and the wsl/ls/whoami example.
In @.claude/setup.md:
- Around line 172-175: The fenced code block containing the two WSL commands
(the block starting with ``` and the lines `wsl -l -q` and `wsl bash -c
"whoami"`) is missing a language identifier which triggers MD040; add an
appropriate language tag (e.g., ```powershell or ```bash) immediately after the
opening backticks so the block becomes ```powershell (or ```bash) to satisfy
markdown linting.
🪄 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
Run ID: 4324640b-fc7e-4870-8506-465df2851532
📒 Files selected for processing (2)
.claude/CLAUDE.md.claude/setup.md
| ``` | ||
| git -C "$env:USERPROFILE\Desktop\ai-engineering-from-scratch" pull | ||
| ``` |
There was a problem hiding this comment.
Specify fence languages for command examples
Line [80] and Line [99]: both fenced blocks are missing language identifiers (MD040). Please tag them (e.g., powershell) to keep markdownlint clean.
Suggested fix
-```
+```powershell
git -C "$env:USERPROFILE\Desktop\ai-engineering-from-scratch" pull
```diff
-```
+```powershell
wsl -l -q # имя дистрибутива (первая строка = default)
wsl bash -c "whoami" # имя WSL-пользователя
wsl bash -c "ls -d ~/ai-engineering-from-scratch" # склонирован ли репо
</details>
Also applies to: 99-103
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/CLAUDE.md around lines 80 - 82, Two fenced code blocks containing
shell/PowerShell examples (the block with git -C
"$env:USERPROFILE\Desktop\ai-engineering-from-scratch" pull and the block with
the wsl commands) are missing language identifiers; add "powershell" (or "bash"
if you prefer) after the opening triple backticks for each fenced block so
markdownlint MD040 is satisfied, e.g., change topowershell for both the
git example and the wsl/ls/whoami example.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ``` | ||
| wsl -l -q | ||
| wsl bash -c "whoami" | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced command block
Line [172]: the fenced block is missing a language identifier (MD040), which can fail markdown lint checks in CI.
Suggested fix
- - имя дистрибутива и пользователя получает через
- ```
+ - имя дистрибутива и пользователя получает через
+ ```powershell
wsl -l -q
wsl bash -c "whoami"
```📝 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.
| ``` | |
| wsl -l -q | |
| wsl bash -c "whoami" | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 172-172: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 @.claude/setup.md around lines 172 - 175, The fenced code block containing
the two WSL commands (the block starting with ``` and the lines `wsl -l -q` and
`wsl bash -c "whoami"`) is missing a language identifier which triggers MD040;
add an appropriate language tag (e.g., ```powershell or ```bash) immediately
after the opening backticks so the block becomes ```powershell (or ```bash) to
satisfy markdown linting.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
phases/00-setup-and-tooling/04-apis-and-keys/outputs/first_api_call_openrouter.py (2)
43-61: ⚖️ Poor tradeoffAdd error handling for HTTP requests and JSON parsing.
Similar to
call_with_sdk(), this function lacks error handling:
- Line 45: Direct dictionary access will raise
KeyErrorifOPENROUTER_API_KEYis not set.- Line 60:
urlopen()can raiseURLErrorfor network issues orHTTPErrorfor HTTP errors (4xx, 5xx responses).- Line 61:
json.loads()can raiseJSONDecodeErrorif the response is malformed.For educational code demonstrating raw HTTP calls, proper error handling is essential to teach students resilient API integration patterns.
🛡️ Example error handling approach
+from urllib.error import HTTPError, URLError + def call_raw_http() -> None: load_dotenv() - api_key = os.environ["OPENROUTER_API_KEY"] + api_key = os.environ.get("OPENROUTER_API_KEY") + if not api_key: + raise ValueError("OPENROUTER_API_KEY environment variable not set") headers = { "Content-Type": "application/json", "Authorization": f"Bearer {api_key}", } body = json.dumps( { "model": MODEL, "max_tokens": 256, "messages": [{"role": "user", "content": PROMPT}], } ).encode() req = urllib.request.Request(URL, data=body, headers=headers, method="POST") - with urllib.request.urlopen(req) as resp: - result = json.loads(resp.read()) + try: + with urllib.request.urlopen(req) as resp: + result = json.loads(resp.read()) + except (HTTPError, URLError) as e: + print(f"HTTP request failed: {e}") + raise + except json.JSONDecodeError as e: + print(f"Failed to parse JSON response: {e}") + raise🤖 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 `@phases/00-setup-and-tooling/04-apis-and-keys/outputs/first_api_call_openrouter.py` around lines 43 - 61, call_raw_http currently assumes OPENROUTER_API_KEY exists and that network/JSON always succeed; change it to first read the key with os.environ.get("OPENROUTER_API_KEY") and raise or log a clear error if missing (instead of direct os.environ[...] access), then wrap the urllib.request.urlopen(req) call in a try/except catching urllib.error.HTTPError and urllib.error.URLError to handle HTTP/network failures, and finally wrap json.loads(resp.read()) in a try/except catching json.JSONDecodeError to surface a helpful parsing error; reference the function name call_raw_http and the symbols URL, MODEL, PROMPT, api_key, req, resp, and result when implementing these checks and logs/raises.
19-31: ⚖️ Poor tradeoffAdd error handling for environment variables and API calls.
As educational code, this function should demonstrate proper error handling:
- Line 24: Direct access to
os.environ["OPENROUTER_API_KEY"]will raiseKeyErrorif the variable is not set.- Lines 27-31: The API call lacks error handling for network failures, authentication errors, or API rate limits.
Students learning from this code will benefit from seeing how to handle these common failure modes gracefully.
🛡️ Example error handling approach
def call_with_sdk() -> None: load_dotenv() + + api_key = os.environ.get("OPENROUTER_API_KEY") + if not api_key: + raise ValueError("OPENROUTER_API_KEY environment variable not set") client = OpenAI( base_url="https://openrouter.ai/api/v1", - api_key=os.environ["OPENROUTER_API_KEY"], + api_key=api_key, ) - response = client.chat.completions.create( - model=MODEL, - max_tokens=256, - messages=[{"role": "user", "content": PROMPT}], - ) + try: + response = client.chat.completions.create( + model=MODEL, + max_tokens=256, + messages=[{"role": "user", "content": PROMPT}], + ) + except Exception as e: + print(f"API call failed: {e}") + raise🤖 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 `@phases/00-setup-and-tooling/04-apis-and-keys/outputs/first_api_call_openrouter.py` around lines 19 - 31, In call_with_sdk(), avoid directly indexing os.environ["OPENROUTER_API_KEY"]; instead validate the OPENROUTER_API_KEY with os.getenv or an explicit check after load_dotenv and bail with a clear error/log message if missing; then wrap the OpenAI client creation and the client.chat.completions.create(...) call (which uses MODEL and PROMPT) in a try/except that captures KeyError (env), network/auth errors and API errors (e.g., request exceptions, authentication failures, rate limits), log or raise a user-friendly message including the underlying exception details, and optionally implement a simple retry/backoff for transient rate-limit/network errors.
🤖 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.
Nitpick comments:
In
`@phases/00-setup-and-tooling/04-apis-and-keys/outputs/first_api_call_openrouter.py`:
- Around line 43-61: call_raw_http currently assumes OPENROUTER_API_KEY exists
and that network/JSON always succeed; change it to first read the key with
os.environ.get("OPENROUTER_API_KEY") and raise or log a clear error if missing
(instead of direct os.environ[...] access), then wrap the
urllib.request.urlopen(req) call in a try/except catching urllib.error.HTTPError
and urllib.error.URLError to handle HTTP/network failures, and finally wrap
json.loads(resp.read()) in a try/except catching json.JSONDecodeError to surface
a helpful parsing error; reference the function name call_raw_http and the
symbols URL, MODEL, PROMPT, api_key, req, resp, and result when implementing
these checks and logs/raises.
- Around line 19-31: In call_with_sdk(), avoid directly indexing
os.environ["OPENROUTER_API_KEY"]; instead validate the OPENROUTER_API_KEY with
os.getenv or an explicit check after load_dotenv and bail with a clear error/log
message if missing; then wrap the OpenAI client creation and the
client.chat.completions.create(...) call (which uses MODEL and PROMPT) in a
try/except that captures KeyError (env), network/auth errors and API errors
(e.g., request exceptions, authentication failures, rate limits), log or raise a
user-friendly message including the underlying exception details, and optionally
implement a simple retry/backoff for transient rate-limit/network errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc6e77eb-35ea-4aa8-a053-e782b423235d
📒 Files selected for processing (4)
.gitignore.vscode/settings.jsonphases/00-setup-and-tooling/04-apis-and-keys/outputs/first_api_call_openrouter.pypyproject.toml
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
scratch/phase-00-lesson-05/exercise-2.ipynb (1)
28-164: ⚡ Quick winStrip bulky rich outputs from the committed notebook JSON.
Large embedded HTML/DataFrame dumps and base64 plot payloads make diffs hard to review and increase merge friction. Prefer clearing outputs before commit and keeping results reproducible via execution.
Also applies to: 177-280, 346-356
🤖 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 `@scratch/phase-00-lesson-05/exercise-2.ipynb` around lines 28 - 164, The notebook contains bulky cell outputs (the "outputs" array with "text/html" and large "text/plain" dumps and non-empty "execution_count") which should be removed before committing; clear all cell outputs and reset execution_count to null, e.g., remove the "outputs" entries and set "execution_count" to null for affected cells (references: the "outputs" JSON key, "text/html"/"text/plain" payloads, and "execution_count"). Fix by running a notebook output cleaner (nbstripout, pre-commit hook, or jupyter nbconvert --ClearOutputPreprocessor) or manually deleting the outputs in the notebook UI, then commit the cleaned notebook so diffs no longer contain embedded HTML/plot payloads. Ensure you apply the same cleanup to the other ranges mentioned (cells containing large outputs around lines 177-280 and 346-356).phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.py (1)
73-76: ⚡ Quick winAvoid executing two billable API calls by default.
Running both paths in
__main__is useful for demoing, but it can unintentionally double token usage/cost on every run. Prefer selecting mode via CLI arg or env flag.🤖 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 `@phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.py` around lines 73 - 76, The script currently calls both call_with_sdk() and call_raw_http() unconditionally in the if __name__ == "__main__" block which doubles billable API usage; change the entrypoint to accept a CLI argument or environment flag (e.g., via argparse or os.environ) to choose which path to run ("sdk", "raw", or "both") and only invoke call_with_sdk() or call_raw_http() according to that selection, defaulting to a safe option (e.g., "sdk" or none) to avoid accidental dual calls; update the __main__ block to parse the flag and conditionally call the functions accordingly.
🤖 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
`@phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.py`:
- Around line 24-25: Replace direct os.environ["OPENROUTER_API_KEY"] lookups
with an explicit validation that fails fast with a clear message: read the value
via os.getenv("OPENROUTER_API_KEY") (or check "OPENROUTER_API_KEY" in
os.environ), and if missing raise a RuntimeError or ValueError with a
descriptive message like "OPENROUTER_API_KEY environment variable is required".
Apply this change to both places where api_key=os.environ["OPENROUTER_API_KEY"]
appears (the api_key= parameter assignments shown in the diff around lines 24-25
and 45-46) so client creation uses the validated value instead of causing a bare
KeyError.
- Around line 59-61: The raw HTTP call using urllib.request.urlopen(req) lacks a
timeout and network error handling; update the call to pass a timeout (e.g.,
urllib.request.urlopen(req, timeout=10)) and wrap it in a try/except that
catches urllib.error.HTTPError, urllib.error.URLError, socket.timeout and a
generic Exception, converting them into a clear error return/raise (or logging)
so call_with_sdk() parity is maintained; locate the URL, req and
urllib.request.urlopen usage in this file and add the timeout plus the exception
handlers around the with urllib.request.urlopen(...) as resp: block.
In `@scratch/phase-00-lesson-05/exercise-1.ipynb`:
- Around line 128-130: Update the conclusion markdown string that currently
reads "Результат - в 8 раз быстрее работает способ с помощью numpy..." to
reflect the measured ~9.1x speedup reported earlier; locate the notebook cell
containing that exact Russian sentence (the markdown cell string) and change "8
раз" to "≈9.1 раза" (or "9.1 раз" for exact match) so the conclusion matches the
computed result.
- Around line 8-18: The notebook contains machine-specific executed output
(data.text/plain showing '/home/pavel/...') that should be removed; open
scratch/phase-00-lesson-05/exercise-1.ipynb and clear the cell outputs (remove
the "outputs" array and reset "execution_count" for that cell) or run a
notebook-clean step (e.g., "jupyter nbconvert
--ClearOutputPreprocessor.enabled=True" or use "git add --renormalize" after
clearing) so no local filesystem paths remain in the committed file; ensure you
save the notebook with outputs cleared before updating the PR.
In `@scratch/phase-00-lesson-05/exercise-2.ipynb`:
- Line 388: Replace the vague concluding string "продажи растут" with a precise
summary that matches the plotted series — for example, state that sales rise
until a peak and then decline (e.g., "продажи растут до пика, затем наблюдается
спад"), by editing the cell/string used for the final caption or conclusion in
the notebook where the literal "продажи растут" appears.
---
Nitpick comments:
In
`@phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.py`:
- Around line 73-76: The script currently calls both call_with_sdk() and
call_raw_http() unconditionally in the if __name__ == "__main__" block which
doubles billable API usage; change the entrypoint to accept a CLI argument or
environment flag (e.g., via argparse or os.environ) to choose which path to run
("sdk", "raw", or "both") and only invoke call_with_sdk() or call_raw_http()
according to that selection, defaulting to a safe option (e.g., "sdk" or none)
to avoid accidental dual calls; update the __main__ block to parse the flag and
conditionally call the functions accordingly.
In `@scratch/phase-00-lesson-05/exercise-2.ipynb`:
- Around line 28-164: The notebook contains bulky cell outputs (the "outputs"
array with "text/html" and large "text/plain" dumps and non-empty
"execution_count") which should be removed before committing; clear all cell
outputs and reset execution_count to null, e.g., remove the "outputs" entries
and set "execution_count" to null for affected cells (references: the "outputs"
JSON key, "text/html"/"text/plain" payloads, and "execution_count"). Fix by
running a notebook output cleaner (nbstripout, pre-commit hook, or jupyter
nbconvert --ClearOutputPreprocessor) or manually deleting the outputs in the
notebook UI, then commit the cleaned notebook so diffs no longer contain
embedded HTML/plot payloads. Ensure you apply the same cleanup to the other
ranges mentioned (cells containing large outputs around lines 177-280 and
346-356).
🪄 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
Run ID: 2b89b862-ac10-41db-91fc-a2006f7b6697
⛔ Files ignored due to path filters (1)
scratch/phase-00-lesson-05/sales.csvis excluded by!**/*.csv
📒 Files selected for processing (3)
phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.pyscratch/phase-00-lesson-05/exercise-1.ipynbscratch/phase-00-lesson-05/exercise-2.ipynb
| api_key=os.environ["OPENROUTER_API_KEY"], | ||
| ) |
There was a problem hiding this comment.
Fail fast with a clear message when OPENROUTER_API_KEY is missing.
Using os.environ["OPENROUTER_API_KEY"] raises a bare KeyError. A clear validation error improves debuggability for first-time setup.
Suggested patch
+def _get_api_key() -> str:
+ api_key = os.getenv("OPENROUTER_API_KEY")
+ if not api_key:
+ raise RuntimeError("OPENROUTER_API_KEY is not set. Add it to your environment or .env file.")
+ return api_key
+
def call_with_sdk() -> None:
load_dotenv()
@@
- api_key=os.environ["OPENROUTER_API_KEY"],
+ api_key=_get_api_key(),
@@
def call_raw_http() -> None:
load_dotenv()
- api_key = os.environ["OPENROUTER_API_KEY"]
+ api_key = _get_api_key()Also applies to: 45-46
🤖 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
`@phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.py`
around lines 24 - 25, Replace direct os.environ["OPENROUTER_API_KEY"] lookups
with an explicit validation that fails fast with a clear message: read the value
via os.getenv("OPENROUTER_API_KEY") (or check "OPENROUTER_API_KEY" in
os.environ), and if missing raise a RuntimeError or ValueError with a
descriptive message like "OPENROUTER_API_KEY environment variable is required".
Apply this change to both places where api_key=os.environ["OPENROUTER_API_KEY"]
appears (the api_key= parameter assignments shown in the diff around lines 24-25
and 45-46) so client creation uses the validated value instead of causing a bare
KeyError.
| req = urllib.request.Request(URL, data=body, headers=headers, method="POST") | ||
| with urllib.request.urlopen(req) as resp: | ||
| result = json.loads(resp.read()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "first_api_call_openrouter.py" -type fRepository: rohitg00/ai-engineering-from-scratch
Length of output: 161
🏁 Script executed:
cat -n phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.pyRepository: rohitg00/ai-engineering-from-scratch
Length of output: 2598
Add timeout and explicit network error handling for the raw HTTP call.
At line 60, urlopen has no timeout and no exception handling, so transient network/API issues can hang indefinitely or crash with an opaque traceback. This is particularly important since the call_with_sdk() implementation (which uses the OpenAI client) has built-in timeout and error handling, creating an asymmetry.
Suggested patch
+import urllib.error
import urllib.request
from dotenv import load_dotenv
@@
req = urllib.request.Request(URL, data=body, headers=headers, method="POST")
- with urllib.request.urlopen(req) as resp:
- result = json.loads(resp.read())
+ try:
+ with urllib.request.urlopen(req, timeout=30) as resp:
+ result = json.loads(resp.read())
+ except urllib.error.HTTPError as e:
+ raise RuntimeError(f"OpenRouter HTTP error: {e.code} {e.reason}") from e
+ except urllib.error.URLError as e:
+ raise RuntimeError(f"OpenRouter network error: {e.reason}") from e🧰 Tools
🪛 Ruff (0.15.12)
[error] 59-59: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 60-60: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
🤖 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
`@phases/00-setup-and-tooling/04-apis-and-keys/code/first_api_call_openrouter.py`
around lines 59 - 61, The raw HTTP call using urllib.request.urlopen(req) lacks
a timeout and network error handling; update the call to pass a timeout (e.g.,
urllib.request.urlopen(req, timeout=10)) and wrap it in a try/except that
catches urllib.error.HTTPError, urllib.error.URLError, socket.timeout and a
generic Exception, converting them into a clear error return/raise (or logging)
so call_with_sdk() parity is maintained; locate the URL, req and
urllib.request.urlopen usage in this file and add the timeout plus the exception
handlers around the with urllib.request.urlopen(...) as resp: block.
| "outputs": [ | ||
| { | ||
| "data": { | ||
| "text/plain": [ | ||
| "'/home/pavel/ai-engineering-from-scratch/.venv/bin/python3'" | ||
| ] | ||
| }, | ||
| "execution_count": 1, | ||
| "metadata": {}, | ||
| "output_type": "execute_result" | ||
| } |
There was a problem hiding this comment.
Remove machine-specific notebook output before commit.
This committed output exposes a local filesystem path (/home/pavel/...) and adds non-reproducible noise to diffs. Please clear this output (or notebook outputs globally) before merge.
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 9-9: Found useless expression. Either assign it to a variable or remove it.
(B018)
[warning] 13-13: Found useless expression. Either assign it to a variable or remove it.
(B018)
[warning] 17-17: Found useless expression. Either assign it to a variable or remove it.
(B018)
🤖 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 `@scratch/phase-00-lesson-05/exercise-1.ipynb` around lines 8 - 18, The
notebook contains machine-specific executed output (data.text/plain showing
'/home/pavel/...') that should be removed; open
scratch/phase-00-lesson-05/exercise-1.ipynb and clear the cell outputs (remove
the "outputs" array and reset "execution_count" for that cell) or run a
notebook-clean step (e.g., "jupyter nbconvert
--ClearOutputPreprocessor.enabled=True" or use "git add --renormalize" after
clearing) so no local filesystem paths remain in the committed file; ensure you
save the notebook with outputs cleared before updating the PR.
| "Результат - в 8 раз быстрее работает способ с помощью numpy. Если я правильно понимаю, потому, что бибилотека написана на C.\n", | ||
| "Первый способ использует интерпретатор python" | ||
| ] |
There was a problem hiding this comment.
Align the conclusion text with the measured speedup.
The notebook reports ~9.1x speedup (Line 110), but the markdown says 8x. Please update the statement to match the computed result.
🤖 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 `@scratch/phase-00-lesson-05/exercise-1.ipynb` around lines 128 - 130, Update
the conclusion markdown string that currently reads "Результат - в 8 раз быстрее
работает способ с помощью numpy..." to reflect the measured ~9.1x speedup
reported earlier; locate the notebook cell containing that exact Russian
sentence (the markdown cell string) and change "8 раз" to "≈9.1 раза" (or "9.1
раз" for exact match) so the conclusion matches the computed result.
| "id": "5528f3da-f9ba-4445-a853-646761d57e60", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "продажи растут" |
There was a problem hiding this comment.
Make the concluding statement more precise.
“продажи растут” is not fully accurate for the shown series (there is a decline after the peak). Please adjust the text to reflect the actual trend.
🤖 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 `@scratch/phase-00-lesson-05/exercise-2.ipynb` at line 388, Replace the vague
concluding string "продажи растут" with a precise summary that matches the
plotted series — for example, state that sales rise until a peak and then
decline (e.g., "продажи растут до пика, затем наблюдается спад"), by editing the
cell/string used for the final caption or conclusion in the notebook where the
literal "продажи растут" appears.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
scratch/lesson-06/playground-project/pyproject.toml (1)
5-10: ⚡ Quick winConsider version upper bounds or more specific constraints.
All dependencies use only lower-bound constraints (
>=), which may lead to breaking changes when major versions are released. For a playground project, consider either:
- Adding upper bounds for stability:
numpy >=1.26,<2.0- Using exact versions for reproducibility:
numpy ==1.26.4🤖 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 `@scratch/lesson-06/playground-project/pyproject.toml` around lines 5 - 10, Update the loose >= version constraints to safer ranges or pin exact versions: replace "numpy >=1.26" with either a bounded spec like "numpy >=1.26,<2.0" or an exact pin like "numpy ==1.26.4", and similarly tighten the optional-dependencies lists for torch/torchvision ("torch>=2.3" -> e.g. "torch>=2.3,<3.0" or "torch==2.3.x") and llm packages anthropic/openai ("anthropic>=0.39" and "openai>=1.50" -> add upper bounds like "<1.0" or pin exact versions) so that numpy, torch, torchvision, anthropic, and openai have either upper bounds or exact pins for stability and reproducibility.
🤖 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 `@phases/00-setup-and-tooling/01-dev-environment/docs/ru.md`:
- Line 123: Update the PyTorch pip index URL to reference CUDA 12.6: replace the
current command string containing "https://download.pytorch.org/whl/cu124" in
the docs line (the existing pip install invocation "uv pip install torch
torchvision torchaudio --index-url https://download.pytorch.org/whl/cu124") so
it instead uses "cu126" (i.e., "https://download.pytorch.org/whl/cu126") to
align the installation instructions with the current PyTorch CUDA
recommendation.
In `@phases/00-setup-and-tooling/03-gpu-setup-and-cloud/docs/ru.md`:
- Around line 23-39: The fenced code block that begins with "1. Локальная NVIDIA
GPU" in ru.md lacks a language tag which triggers markdownlint MD040; update the
opening fence from ``` to a language-hint such as ```text (or ```plain) so the
block becomes ```text and keep the rest unchanged to satisfy the linter.
- Around line 72-80: The snippet uses an invalid attribute name on the device
properties: update the access of torch.cuda.get_device_properties(0).total_mem
to the correct attribute torch.cuda.get_device_properties(0).total_memory (and
ensure any other occurrences such as the similar use elsewhere are updated to
total_memory) so the GPU memory print uses the valid property name.
In `@phases/00-setup-and-tooling/06-python-environments/docs/ru.md`:
- Line 371: The Russian docs contain a typo in a subheading: replace the
incorrect "Gott'cha" string with the correct "Gotcha" in
phases/00-setup-and-tooling/06-python-environments/docs/ru.md; locate the
subheading text (the line containing "Gott'cha") and change it to "Gotcha" so
the heading reads correctly.
In `@scratch/lesson-06/playground-project/pyproject.toml`:
- Line 2: The project name in pyproject.toml is set to "pyproject", which
conflicts with the config filename and causes confusion; update the name field
in pyproject.toml (the "name" key) to a descriptive, non-conflicting package
name such as "lesson-06-playground" or "ai-engineering-playground" so the
project metadata no longer matches the filename.
---
Nitpick comments:
In `@scratch/lesson-06/playground-project/pyproject.toml`:
- Around line 5-10: Update the loose >= version constraints to safer ranges or
pin exact versions: replace "numpy >=1.26" with either a bounded spec like
"numpy >=1.26,<2.0" or an exact pin like "numpy ==1.26.4", and similarly tighten
the optional-dependencies lists for torch/torchvision ("torch>=2.3" -> e.g.
"torch>=2.3,<3.0" or "torch==2.3.x") and llm packages anthropic/openai
("anthropic>=0.39" and "openai>=1.50" -> add upper bounds like "<1.0" or pin
exact versions) so that numpy, torch, torchvision, anthropic, and openai have
either upper bounds or exact pins for stability and reproducibility.
🪄 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
Run ID: 0d34ea5c-3d2f-41c9-b94e-c0121c11bc20
⛔ Files ignored due to path filters (1)
scratch/lesson-06/playground-project/requirements.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
phases/00-setup-and-tooling/01-dev-environment/docs/ru.mdphases/00-setup-and-tooling/02-git-and-collaboration/docs/ru.mdphases/00-setup-and-tooling/03-gpu-setup-and-cloud/docs/ru.mdphases/00-setup-and-tooling/04-apis-and-keys/docs/ru.mdphases/00-setup-and-tooling/05-jupyter-notebooks/docs/ru.mdphases/00-setup-and-tooling/06-python-environments/docs/ru.mdscratch/lesson-06/playground-project/pyproject.toml
| Дальше — PyTorch с CUDA: | ||
|
|
||
| ```bash | ||
| uv pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cu124 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the PyTorch CUDA 12.4 wheel index exists and is the recommended version
curl -s https://download.pytorch.org/whl/cu124/ | head -20Repository: rohitg00/ai-engineering-from-scratch
Length of output: 946
🌐 Web query:
What is the current recommended CUDA version for PyTorch installation as of May 2025?
💡 Result:
As of May 2025, the “current recommended” CUDA version for installing PyTorch (via the official PyTorch “Get Started” selector for CUDA-enabled binaries) is CUDA 12.6.[1][2] PyTorch’s official install selector lists supported CUDA binary builds (including 12.6 and 12.8); it also notes that choosing the latest CUDA version is often better, and points users to the specific CUDA version shown in the selector when installing.[1][2]
Citations:
Update CUDA version to 12.6 for PyTorch installation.
The --index-url references CUDA 12.4, but PyTorch's official installer recommends CUDA 12.6 as of May 2025. Update line 123 to use cu126 instead of cu124 to align with current PyTorch best practices.
🤖 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 `@phases/00-setup-and-tooling/01-dev-environment/docs/ru.md` at line 123,
Update the PyTorch pip index URL to reference CUDA 12.6: replace the current
command string containing "https://download.pytorch.org/whl/cu124" in the docs
line (the existing pip install invocation "uv pip install torch torchvision
torchaudio --index-url https://download.pytorch.org/whl/cu124") so it instead
uses "cu126" (i.e., "https://download.pytorch.org/whl/cu126") to align the
installation instructions with the current PyTorch CUDA recommendation.
| ``` | ||
| 1. Локальная NVIDIA GPU | ||
| Стоимость: $0 (если уже есть) | ||
| Настройка: CUDA + cuDNN + PyTorch с CUDA | ||
| Когда брать: Регулярная работа, большие датасеты, низкая латентность | ||
|
|
||
| 2. Google Colab (free tier) | ||
| Стоимость: $0 | ||
| Настройка: отсутствует — браузер и Google-аккаунт | ||
| Когда брать: Эксперименты, обучение, когда нет своей карты | ||
| Ограничения: 90 минут idle → отключение; сессии сбрасываются | ||
|
|
||
| 3. Cloud GPU (Lambda, RunPod, Vast.ai) | ||
| Стоимость: $0.20–2.00/час за GPU | ||
| Настройка: SSH + установка библиотек | ||
| Когда брать: Серьёзная тренировка, большие модели, надолго | ||
| ``` |
There was a problem hiding this comment.
Add a language hint to the fenced block to satisfy markdownlint MD040.
At Line 23, the fenced block has no language tag; this is already flagged by static analysis.
Proposed fix
-```
+```text
1. Локальная NVIDIA GPU
Стоимость: $0 (если уже есть)
Настройка: CUDA + cuDNN + PyTorch с CUDA
@@
3. Cloud GPU (Lambda, RunPod, Vast.ai)
Стоимость: $0.20–2.00/час за GPU
Настройка: SSH + установка библиотек
Когда брать: Серьёзная тренировка, большие модели, надолго</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@phases/00-setup-and-tooling/03-gpu-setup-and-cloud/docs/ru.md` around lines
23 - 39, The fenced code block that begins with "1. Локальная NVIDIA GPU" in
ru.md lacks a language tag which triggers markdownlint MD040; update the opening
fence from ``` to a language-hint such as ```text (or ```plain) so the block
becomes ```text and keep the rest unchanged to satisfy the linter.
| ```python | ||
| import torch | ||
|
|
||
| print(f"CUDA available: {torch.cuda.is_available()}") | ||
| print(f"CUDA version: {torch.version.cuda}") | ||
| if torch.cuda.is_available(): | ||
| print(f"GPU: {torch.cuda.get_device_name(0)}") | ||
| print(f"Memory: {torch.cuda.get_device_properties(0).total_mem / 1e9:.1f} GB") | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In official PyTorch documentation for torch.cuda.get_device_properties, what is the correct attribute for total GPU memory: total_memory or total_mem?
💡 Result:
In the official PyTorch documentation and API, the correct attribute for accessing the total GPU memory via torch.cuda.get_device_properties is total_memory [1][2][3]. The attribute total_mem is not a valid property of the object returned by this function. When you call torch.cuda.get_device_properties(device), it returns a _CudaDeviceProperties object [4], which includes the total_memory attribute (typically expressed in bytes) [1][3].
Citations:
- 1: https://discuss.pytorch.org/t/get-total-memory-of-gpu/57391
- 2: https://stackoverflow.com/questions/58216000/get-total-amount-of-free-gpu-memory-and-available-using-pytorch
- 3: https://github.com/ECNU-Cross-Innovation-Lab/Mamba-Spike/blob/main/check_cuda.py
- 4: https://docs.pytorch.org/docs/2.12/generated/torch.cuda.get_device_properties.html
Fix invalid PyTorch device property attribute name.
Line 79 uses total_mem, which is not a valid attribute. The correct attribute is total_memory (also see Line 231). This will cause an AttributeError when learners run the snippet.
Proposed fix
- print(f"Memory: {torch.cuda.get_device_properties(0).total_mem / 1e9:.1f} GB")
+ print(f"Memory: {torch.cuda.get_device_properties(0).total_memory / 1e9:.1f} GB")🤖 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 `@phases/00-setup-and-tooling/03-gpu-setup-and-cloud/docs/ru.md` around lines
72 - 80, The snippet uses an invalid attribute name on the device properties:
update the access of torch.cuda.get_device_properties(0).total_mem to the
correct attribute torch.cuda.get_device_properties(0).total_memory (and ensure
any other occurrences such as the similar use elsewhere are updated to
total_memory) so the GPU memory print uses the valid property name.
| без переустановки — стандартный режим разработки. | ||
|
|
||
| ### Гott'cha: `--extra` — это имя группы, не пакета | ||
|
|
There was a problem hiding this comment.
Fix typo in subheading.
"Gott'cha" should be "Gotcha".
✏️ Proposed fix
-### Гott'cha: `--extra` — это имя группы, не пакета
+### Gotcha: `--extra` — это имя группы, не пакета📝 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.
| ### Gotcha: `--extra` — это имя группы, не пакета |
🤖 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 `@phases/00-setup-and-tooling/06-python-environments/docs/ru.md` at line 371,
The Russian docs contain a typo in a subheading: replace the incorrect
"Gott'cha" string with the correct "Gotcha" in
phases/00-setup-and-tooling/06-python-environments/docs/ru.md; locate the
subheading text (the line containing "Gott'cha") and change it to "Gotcha" so
the heading reads correctly.
| @@ -0,0 +1,10 @@ | |||
| [project] | |||
| name = "pyproject" | |||
There was a problem hiding this comment.
Rename the project to avoid confusion with the config file.
The project name "pyproject" conflicts with the standard configuration filename pyproject.toml, creating unnecessary confusion. Package names should be descriptive of the project's purpose, such as lesson-06-playground or ai-engineering-playground.
📦 Proposed fix
-name = "pyproject"
+name = "lesson-06-playground"📝 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.
| name = "pyproject" | |
| name = "lesson-06-playground" |
🤖 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 `@scratch/lesson-06/playground-project/pyproject.toml` at line 2, The project
name in pyproject.toml is set to "pyproject", which conflicts with the config
filename and causes confusion; update the name field in pyproject.toml (the
"name" key) to a descriptive, non-conflicting package name such as
"lesson-06-playground" or "ai-engineering-playground" so the project metadata no
longer matches the filename.
1 participant
What this PR does
Kind of change
Checklist
LESSON_TEMPLATE.mdstructure[Name](phases/...)), not bare textdocs/en.mdclaimsPhase / lesson
Notes for reviewer
Summary by CodeRabbit
Release Notes
Documentation
New Features
Exercises
Configuration