prac: valid parentheses mapping solution reasoning

[WazedKhan]

Summary by CodeRabbit

• Style

  • Applied code formatting and whitespace standardization across existing solutions.

• New Features

  • Added a new solution for parentheses validation using a stack-based approach.

• Tests

  • Reorganized test suite with updated import paths and added test coverage for the new validation solution.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request reorganizes existing solution files, adds a new parentheses validation method using stack-based approach, performs minor formatting adjustments to a stock trading solution, and consolidates test cases from a general test suite into a dedicated easy problems test file.

Changes

Cohort / File(s)	Summary
Formatting & File Reorganization LeetCode/dump/best_time_buy_sell_stock_121.py	Minor whitespace and quote normalization: float('inf') → float("inf"), list spacing adjustments, and newline standardization with no logic changes.
New Solution Implementation LeetCode/easy/valid_parentheses_20.py	New Solution class added with isValidByMapping method that validates parentheses strings using a stack and closing-to-opening bracket mapping. Handles {}, (), [] with O(n) time and space complexity.
Test Suite Reorganization tests/test_leetcode.py, tests/test_leetcode_easy.py	Removed test_is_valid_parentheses from general test file; added it to easy test file with parameterized test cases validating two solution variants. Updated import path for stock trading solution (easy → dump directory).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A bracket balanced, a stack so neat,
Old tests shuffle to a new beat,
Quotes and spacing, now they align,
Solutions reorganized by design!
*hippity*

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'prac: valid parentheses mapping solution reasoning' accurately describes the main change: adding a new valid parentheses solution using a mapping-based approach.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch valid-parentheses-20-intview

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

LeetCode/easy/valid_parentheses_20.py (1)

1-12: isValidByMapping is algorithmically identical to the existing isValid

Comparing with LeetCode/valid_parentheses_20.py (isValid):

Aspect	isValid	isValidByMapping
Map	hash_map	mapping
Loop var	parentheses	p
Guard condition	hash_map[p] == stack[-1]	stack[-1] == mapping[p]
Return	not stack	len(stack) == 0

There is no semantic difference. Consider either having isValidByMapping delegate to isValid, or document the intended distinction if one exists.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@LeetCode/easy/valid_parentheses_20.py` around lines 1 - 12, isValidByMapping
is functionally identical to isValid; either remove the duplication or make
isValidByMapping delegate to isValid. Modify isValidByMapping to simply return
self.isValid(s) (or, if you intend a different implementation, change its logic
and add a docstring explaining the distinction), and remove any redundant local
variables like mapping/hash_map to avoid confusion; ensure references to isValid
and isValidByMapping are updated accordingly.

tests/test_leetcode_easy.py (1)

202-212: Test conflates two solutions in one function; separate them

test_is_valid_parentheses asserts both isValid (old module) and isValidByMapping (new module) in sequence. If isValid fails, the failure message won't indicate whether isValidByMapping also failed, obscuring coverage.

Consider splitting into two focused test functions, or at minimum only test isValidByMapping here (the old test coverage can live in its own function):

 def test_is_valid_parentheses(s: str, expected: bool):
-    from LeetCode.easy.valid_parentheses_20 import Solution as Sol
-    from LeetCode.valid_parentheses_20 import Solution
-
-    solution = Solution()
-    result = solution.isValid(s)
-    assert result == expected
-
-    sol = Sol()
-    res = sol.isValidByMapping(s)
-    assert res == expected
+    from LeetCode.easy.valid_parentheses_20 import Solution
+
+    sol = Solution()
+    assert sol.isValidByMapping(s) == expected

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_leetcode_easy.py` around lines 202 - 212, The current test
function test_is_valid_parentheses mixes two implementations and should be
split; update tests so one function exercises the old implementation
(instantiate LeetCode.valid_parentheses_20.Solution and call isValid) and a
separate function tests the new implementation (instantiate
LeetCode.easy.valid_parentheses_20.Solution and call isValidByMapping), each
asserting their expected result independently, or alternatively remove the old
assertion and only test isValidByMapping in this file; ensure you reference the
two classes/methods (Solution.isValid and Solution.isValidByMapping) so failures
clearly indicate which implementation broke.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@LeetCode/dump/best_time_buy_sell_stock_121.py`:
- Line 32: The profit update assigns the inverted expression; in the loop that
compares price, min_price, and best_profit (variables price, min_price,
best_profit inside the solution function, e.g., maxProfit), change the
assignment so best_profit is set to price - min_price (not min_price - price)
when if price - min_price > best_profit; ensure the conditional and assignment
are properly separated/terminated so the check updates best_profit correctly.
- Around line 79-81: The module currently runs Solution().maxProfit(price_3) and
prints the result at import time, causing side effects when tests import
Solution; move those example invocation lines (the price_3 assignment, the call
to Solution().maxProfit, and the print) into a guard block so they only run when
executed as a script: wrap them in if __name__ == "__main__": or remove them
entirely so importing the Solution class and its maxProfit method has no stdout
or computation side effects.

---

Nitpick comments:
In `@LeetCode/easy/valid_parentheses_20.py`:
- Around line 1-12: isValidByMapping is functionally identical to isValid;
either remove the duplication or make isValidByMapping delegate to isValid.
Modify isValidByMapping to simply return self.isValid(s) (or, if you intend a
different implementation, change its logic and add a docstring explaining the
distinction), and remove any redundant local variables like mapping/hash_map to
avoid confusion; ensure references to isValid and isValidByMapping are updated
accordingly.

In `@tests/test_leetcode_easy.py`:
- Around line 202-212: The current test function test_is_valid_parentheses mixes
two implementations and should be split; update tests so one function exercises
the old implementation (instantiate LeetCode.valid_parentheses_20.Solution and
call isValid) and a separate function tests the new implementation (instantiate
LeetCode.easy.valid_parentheses_20.Solution and call isValidByMapping), each
asserting their expected result independently, or alternatively remove the old
assertion and only test isValidByMapping in this file; ensure you reference the
two classes/methods (Solution.isValid and Solution.isValidByMapping) so failures
clearly indicate which implementation broke.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Docstring typo: profit update formula is inverted

The thought process says best_profit = min_price - price, but the correct formula is price - min_price (as correctly implemented at Line 74).

📝 Fix

-        if price - min_price > best_profit best_profit = min_price - price
+        if price - min_price > best_profit: best_profit = price - min_price

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@LeetCode/dump/best_time_buy_sell_stock_121.py` at line 32, The profit update
assigns the inverted expression; in the loop that compares price, min_price, and
best_profit (variables price, min_price, best_profit inside the solution
function, e.g., maxProfit), change the assignment so best_profit is set to price
- min_price (not min_price - price) when if price - min_price > best_profit;
ensure the conditional and assignment are properly separated/terminated so the
check updates best_profit correctly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Module-level side effects execute on import, polluting test output

Solution().maxProfit(price_3) and print(...) run unconditionally at module scope. Now that test_leetcode_easy.py (Line 88) imports from LeetCode.dump.best_time_buy_sell_stock_121 import Solution, every test run will execute this computation and emit stdout, dirtying pytest output.

Guard behind __main__:

-price_3 = [7, 6, 4, 3, 1]
-sol = Solution().maxProfit(price_3)
-print(f"Best profit: {sol}")
+if __name__ == "__main__":
+    price_3 = [7, 6, 4, 3, 1]
+    sol = Solution().maxProfit(price_3)
+    print(f"Best profit: {sol}")

📝 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

	price_3 = [7, 6, 4, 3, 1]
	sol = Solution().maxProfit(price_3)
	print(f"Best profit: {sol}")
	\ No newline at end of file
	print(f"Best profit: {sol}")
	if __name__ == "__main__":
	price_3 = [7, 6, 4, 3, 1]
	sol = Solution().maxProfit(price_3)
	print(f"Best profit: {sol}")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@LeetCode/dump/best_time_buy_sell_stock_121.py` around lines 79 - 81, The
module currently runs Solution().maxProfit(price_3) and prints the result at
import time, causing side effects when tests import Solution; move those example
invocation lines (the price_3 assignment, the call to Solution().maxProfit, and
the print) into a guard block so they only run when executed as a script: wrap
them in if __name__ == "__main__": or remove them entirely so importing the
Solution class and its maxProfit method has no stdout or computation side
effects.