Skip to content

curriculum: add actionable assertion messages to advanced OOP checks (batch 2 of 2) - #139

Open
TrueFurina wants to merge 6 commits into
abhiksark:mainfrom
TrueFurina:fix/assertion-messages-oop-advanced-batch2
Open

curriculum: add actionable assertion messages to advanced OOP checks (batch 2 of 2)#139
TrueFurina wants to merge 6 commits into
abhiksark:mainfrom
TrueFurina:fix/assertion-messages-oop-advanced-batch2

Conversation

@TrueFurina

@TrueFurina TrueFurina commented Aug 27, 2026

Copy link
Copy Markdown

Adds beginner-facing messages to 11 bare assertions across 6 files (oop_advanced7-12). AST probe confirms 0 remaining bare assertions; pytest integration test passes.

Summary by CodeRabbit

  • Tests
    • Added clearer failure messages to object-oriented programming checks.
    • Improved diagnostics for inheritance, calculations, equality, coordinate parsing, validation boundaries, serialization, and vector operations.
    • Existing test behavior and expected outcomes remain unchanged.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: eefa7969-6a75-46b8-8215-4a378acf10eb

📥 Commits

Reviewing files that changed from the base of the PR and between b4d40bf and 8cb427d.

📒 Files selected for processing (6)
  • checks/oop_advanced/oop_advanced10.py
  • checks/oop_advanced/oop_advanced11.py
  • checks/oop_advanced/oop_advanced12.py
  • checks/oop_advanced/oop_advanced7.py
  • checks/oop_advanced/oop_advanced8.py
  • checks/oop_advanced/oop_advanced9.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
Maintain Python 3.9 compatibility. Guard standard-library APIs introduced in

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • checks/oop_advanced/oop_advanced12.py
  • checks/oop_advanced/oop_advanced9.py
  • checks/oop_advanced/oop_advanced8.py
  • checks/oop_advanced/oop_advanced10.py
  • checks/oop_advanced/oop_advanced7.py
  • checks/oop_advanced/oop_advanced11.py
Preserve learner-edited exercises during workspace updates. Reset snapshots

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • checks/oop_advanced/oop_advanced12.py
  • checks/oop_advanced/oop_advanced9.py
  • checks/oop_advanced/oop_advanced8.py
  • checks/oop_advanced/oop_advanced10.py
  • checks/oop_advanced/oop_advanced7.py
  • checks/oop_advanced/oop_advanced11.py
Checks must use bare assertions with actionable, beginner-facing messages. Keep

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • checks/oop_advanced/oop_advanced12.py
  • checks/oop_advanced/oop_advanced9.py
  • checks/oop_advanced/oop_advanced8.py
  • checks/oop_advanced/oop_advanced10.py
  • checks/oop_advanced/oop_advanced7.py
  • checks/oop_advanced/oop_advanced11.py
🪛 Ruff (0.16.2)
checks/oop_advanced/oop_advanced12.py

[error] 1-1: Undefined name Vector

(F821)


[error] 2-2: Undefined name Vector

(F821)

checks/oop_advanced/oop_advanced9.py

[error] 1-1: Undefined name Grade

(F821)


[error] 2-2: Undefined name Grade

(F821)


[error] 3-3: Undefined name Grade

(F821)

checks/oop_advanced/oop_advanced8.py

[error] 1-1: Undefined name Coordinate

(F821)

checks/oop_advanced/oop_advanced10.py

[error] 1-1: Undefined name Square

(F821)


[error] 1-1: Undefined name Shape

(F821)


[error] 2-2: Undefined name Square

(F821)

checks/oop_advanced/oop_advanced7.py

[error] 1-1: Undefined name Book

(F821)


[error] 2-2: Undefined name Book

(F821)

checks/oop_advanced/oop_advanced11.py

[error] 1-1: Undefined name Product

(F821)

🔇 Additional comments (6)
checks/oop_advanced/oop_advanced7.py (1)

1-2: LGTM!

checks/oop_advanced/oop_advanced8.py (1)

1-2: LGTM!

checks/oop_advanced/oop_advanced9.py (1)

1-3: LGTM!

checks/oop_advanced/oop_advanced10.py (1)

1-2: LGTM!

checks/oop_advanced/oop_advanced11.py (1)

1-2: LGTM!

checks/oop_advanced/oop_advanced12.py (1)

1-3: LGTM!


📝 Walkthrough

Walkthrough

The OOP advanced check scripts now include descriptive messages in their existing assertions. The tested conditions, validation behavior, and success output remain unchanged.

Changes

OOP assertion diagnostics

Layer / File(s) Summary
Descriptive assertion messages
checks/oop_advanced/oop_advanced7.py, checks/oop_advanced/oop_advanced8.py, checks/oop_advanced/oop_advanced9.py, checks/oop_advanced/oop_advanced10.py, checks/oop_advanced/oop_advanced11.py, checks/oop_advanced/oop_advanced12.py
Existing assertions now include messages for equality, coordinate parsing, score validation, inheritance, area calculation, dictionary conversion, and vector addition checks.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 8cb42

This change only adds beginner-facing messages to existing assertions, with no actionable merge-blocking risk remaining after normal checks and review.

Poem

A rabbit checks each test with care
Clear messages now wait there
Shapes, vectors, books, and scores
Explain failures at the doors
The checks still hop as before

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides a concise summary and test result, but it omits the required Summary, Tests, Screenshots, and Checklist headings and does not explicitly confirm python -m pytest -q. Update the description to use the repository template. Add the Summary, Tests, Screenshots, and Checklist sections. Record the python -m pytest -q result and complete the checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding actionable assertion messages to the advanced OOP checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 6…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 6 files.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@abhiksark

Copy link
Copy Markdown
Owner

Thanks for adding beginner-facing messages to these advanced OOP checks. I validated the implementation itself: all 11 assertions now have non-empty messages, the reference-solution test passes, the full suite on this branch passes, and the passing curriculum verifies successfully.

I cannot approve this PR in its current state for the following reasons:

  1. The PR targets main. Contributor feature and fix PRs must target dev; main is reserved for verified release promotion from dev.
  2. The branch is currently 19 commits behind dev and does not report the project CI matrix.
  3. All six edited files were converted from LF to CRLF. git diff --check reports trailing whitespace on every changed line. Please preserve LF line endings and make git diff --check pass.
  4. The PR description does not follow the repository template or record all required validation.
  5. The exact six-file scope for curriculum: add actionable assertion messages to advanced OOP checks (batch 2 of 2) #108 is also covered by fix(curriculum): add actionable advanced OOP assertion messages #142, which targets dev and currently has green CI. Please coordinate there to avoid merging duplicate implementations. Given that overlap, closing this PR as superseded by fix(curriculum): add actionable advanced OOP assertion messages #142 is likely the cleanest path.

Validation performed here:

  • python -m pytest tests/integration/test_solution_verify.py -q1 passed
  • python -m pytest -q190 passed on this older branch
  • python -m pythonlings --root tests/fixtures/passing_curriculum verify — passed
  • AST audit — all 11 assertions have non-empty messages
  • git diff --check — failed because of the CRLF conversion

@abhiksark abhiksark left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion changes are functionally sound, but this cannot merge as submitted: it targets main instead of dev, is 19 commits behind dev, has no project CI matrix, and converts all six files to CRLF so git diff --check fails. It also duplicates #142 for #108. Please close as superseded, or retarget/update the branch, restore LF endings, complete the PR template, and coordinate the duplicate scope before requesting another review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants