feat(algorithms, graphs, topological-sort): possible recipes from supplies #149
Conversation
BrianLusina
added
enhancement
Algorithm
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds three algorithm modules (Alien Dictionary, Find All Possible Recipes, Find All Subsets) with READMEs and parameterized unit tests, updates Changes
Sequence Diagram(s)(omitted — changes do not meet sequence-diagram generation criteria) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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
🤖 Fix all issues with AI agents
In @algorithms/graphs/recipes_supplies/__init__.py:
- Line 25: Rename the misspelled module-level variable possible_recipies to
possible_recipes and update every reference to it in this module (e.g., the
places that currently read possible_recipies on subsequent lines such as where
it's initialized and where it's referenced later). Ensure you update the
variable name consistently (assignment, reads, and any imports/exports within
the module) so there are no NameError or unresolved reference issues.
In @algorithms/graphs/recipes_supplies/test_find_all_possible_recipes.py:
- Around line 57-65: Rename the misspelled parameter `recipies` to `recipes` in
the test function `test_find_recipes` and likewise update the implementation
signature and all usages in the `find_recipes` function to use `recipes` (fix
the variable name where it's referenced in the function body and any
tests/fixtures that pass it); ensure spellings are consistent across the test
parameter, the call to `find_recipes(recipes, ingredients, supplies)`, and the
function definition for `find_recipes` so no references to `recipies` remain.
🧹 Nitpick comments (1)
DIRECTORY.md (1)
109-110: Optional: adjust indentation to match markdownlint conventions.The indentation follows the existing pattern in this file, but markdownlint suggests using 2-space increments for nested list items. This is a minor style preference that could be adjusted for consistency with linting rules.
📝 Suggested indentation adjustment
- * Recipes Supplies - * [Test Find All Possible Recipes](https://github.com/BrianLusina/PythonSnips/blob/master/algorithms/graphs/recipes_supplies/test_find_all_possible_recipes.py) + * Recipes Supplies + * [Test Find All Possible Recipes](https://github.com/BrianLusina/PythonSnips/blob/master/algorithms/graphs/recipes_supplies/test_find_all_possible_recipes.py)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
algorithms/graphs/recipes_supplies/images/examples/all_possible_recipes_from_supplies_example_1.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/examples/all_possible_recipes_from_supplies_example_2.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_1.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_10.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_2.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_3.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_4.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_5.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_6.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_7.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_8.pngis excluded by!**/*.pngalgorithms/graphs/recipes_supplies/images/solutions/all_possible_recipes_from_supplies_solution_9.pngis excluded by!**/*.png
📒 Files selected for processing (4)
DIRECTORY.mdalgorithms/graphs/recipes_supplies/README.mdalgorithms/graphs/recipes_supplies/__init__.pyalgorithms/graphs/recipes_supplies/test_find_all_possible_recipes.py
🧰 Additional context used
🧬 Code graph analysis (1)
algorithms/graphs/recipes_supplies/test_find_all_possible_recipes.py (1)
algorithms/graphs/recipes_supplies/__init__.py (1)
find_recipes(5-40)
🪛 LanguageTool
algorithms/graphs/recipes_supplies/README.md
[style] ~3-~3: Try using a synonym here to strengthen your writing.
Context: ...le Recipes from Given Supplies You are given information about n different recipes. ...
(GIVE_PROVIDE)
[style] ~70-~70: Using many exclamation marks might seem excessive (in this case: 8 exclamation marks for a text that’s 4936 characters long)
Context: ...e_recipes_from_supplies_solution_1.png) 
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
109-109: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
110-110: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🔇 Additional comments (3)
algorithms/graphs/recipes_supplies/README.md (1)
1-89: Excellent documentation!The README provides comprehensive coverage of the problem statement, constraints, solution approach, and complexity analysis. The step-by-step algorithm walkthrough clearly explains how topological sort is applied to solve the recipe dependency problem.
algorithms/graphs/recipes_supplies/__init__.py (1)
5-40: Algorithm implementation is correct!The topological sort approach properly handles all cases including circular dependencies. The graph construction and BFS traversal correctly identify which recipes can be made given the initial supplies.
algorithms/graphs/recipes_supplies/test_find_all_possible_recipes.py (1)
6-52: Comprehensive test coverage!The test cases effectively validate the algorithm across various scenarios including simple cases, dependency chains, and circular dependencies. The parameterized approach keeps the tests clean and maintainable.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @algorithms/graphs/alien_dictionary/README.md:
- Around line 183-186: Fix the duplicated "0" in the README sentence: locate the
sentence in algorithms/graphs/alien_dictionary/README.md that reads "When the
in-degree of the node reaches 0 0" and remove the extra "0" (or the stray
newline) so it reads "When the in-degree of the node reaches 0" to correct the
formatting.
In @algorithms/graphs/alien_dictionary/test_alien_dictionary.py:
- Around line 50-59: The tests test_alien_order and test_alien_order_2 currently
compare sorted(expected) to sorted(actual), which only checks multiset equality
and not topological order; replace these assertions by validating that the
returned order from alien_order and alien_order_2 respects the ordering
constraints implied by the input word list (or, if the implementations are
deterministic and expected is the unique correct order, assert exact equality).
Concretely, add a small helper is_valid_alien_order(words, order) that builds a
rank map from order and verifies each adjacent word pair respects the first
differing character ranks (and that a longer prefix before a shorter word is
invalid), then call self.assertTrue(is_valid_alien_order(words, actual)) (handle
empty/invalid expected cases accordingly) instead of using sorted(...)
comparisons.
In @algorithms/subsets/find_all_subsets/__init__.py:
- Around line 7-8: The function find_all_subsets currently returns [] when n ==
0, but the correct power set for empty input is [[]]; remove the early return
that checks "if n == 0: return []" (or modify it to return [[]]) so that the
existing backtracking logic in find_all_subsets appends the empty current subset
(curr[:]) and produces [[]] for empty input; ensure the change references the
same function name find_all_subsets and variables n and curr so behavior for
non-empty inputs remains unchanged.
In @algorithms/subsets/find_all_subsets/test_find_all_subsets.py:
- Line 7: The test case expecting an empty list for the power set is wrong;
update the test in test_find_all_subsets.py so that the input [] expects [[]]
(i.e., the power set of an empty set contains one empty subset). Locate the
tuple ([], []) in the test file and change the expected value to ([[]]) or the
equivalent list containing an empty list so it matches the output from
find_all_subsets.
- Line 35: The test's expected output for input [3, 6, 9] is in the wrong order;
update the expected list in the test tuple for [3, 6, 9] to match the
backtracking traversal order produced by the implementation ([], [3], [3, 6],
[3, 6, 9], [3, 9], [6], [6, 9], [9]) so the test in test_find_all_subsets.py
aligns with the actual output of the find_all_subsets/backtracking routine.
🧹 Nitpick comments (2)
algorithms/graphs/alien_dictionary/__init__.py (2)
1-2: Shadowed import:Counterfromtypingis unused.
Counteris imported from bothtyping(line 1) andcollections(line 2). The second import shadows the first, making thetyping.Counterimport unused. Since Python 3.9+,collections.Countersupports subscripting directly for type hints.Suggested fix
-from typing import Deque, DefaultDict, List, Counter, Set +from typing import Deque, DefaultDict, List, Set from collections import defaultdict, Counter, deque
18-24: Simplify defaultdict initialization.
- Line 18:
defaultdict(int)is more idiomatic thandefaultdict(lambda: 0).- Lines 21-24: The conditional check is redundant—simply accessing the key is sufficient to register it in the defaultdict.
Suggested simplification
- in_degree: DefaultDict[str, int] = defaultdict(lambda: 0) + in_degree: DefaultDict[str, int] = defaultdict(int) # set every unique character in every word in words to 0 for word in words: for char in word: - if not in_degree[char]: - in_degree[char] = 0 + _ = in_degree[char] # registers char with default value 0Or even simpler, using set comprehension:
in_degree: DefaultDict[str, int] = defaultdict(int) all_chars = {char for word in words for char in word} for char in all_chars: _ = in_degree[char]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (19)
algorithms/graphs/alien_dictionary/images/examples/alien_dictionary_example_1.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/examples/alien_dictionary_example_2.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/examples/alien_dictionary_example_3.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/examples/alien_dictionary_example_4.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/examples/alien_dictionary_example_5.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_1.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_10.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_2.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_3.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_4.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_5.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_6.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_7.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_8.pngis excluded by!**/*.pngalgorithms/graphs/alien_dictionary/images/solutions/alien_dictionary_solution_9.pngis excluded by!**/*.pngalgorithms/subsets/find_all_subsets/images/examples/find_all_subsets_example_1.pngis excluded by!**/*.pngalgorithms/subsets/find_all_subsets/images/examples/find_all_subsets_example_2.pngis excluded by!**/*.pngalgorithms/subsets/find_all_subsets/images/examples/find_all_subsets_example_3.pngis excluded by!**/*.pngalgorithms/subsets/find_all_subsets/images/examples/find_all_subsets_example_4.pngis excluded by!**/*.png
📒 Files selected for processing (12)
DIRECTORY.mdalgorithms/graphs/alien_dictionary/README.mdalgorithms/graphs/alien_dictionary/__init__.pyalgorithms/graphs/alien_dictionary/test_alien_dictionary.pyalgorithms/graphs/recipes_supplies/__init__.pyalgorithms/subsets/__init__.pyalgorithms/subsets/cascading_subsets/README.mdalgorithms/subsets/cascading_subsets/__init__.pyalgorithms/subsets/cascading_subsets/test_cascading_subsets.pyalgorithms/subsets/find_all_subsets/README.mdalgorithms/subsets/find_all_subsets/__init__.pyalgorithms/subsets/find_all_subsets/test_find_all_subsets.py
✅ Files skipped from review due to trivial changes (1)
- algorithms/subsets/find_all_subsets/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- algorithms/graphs/recipes_supplies/init.py
🧰 Additional context used
🧬 Code graph analysis (2)
algorithms/subsets/find_all_subsets/test_find_all_subsets.py (1)
algorithms/subsets/find_all_subsets/__init__.py (1)
find_all_subsets(4-22)
algorithms/subsets/cascading_subsets/test_cascading_subsets.py (1)
algorithms/subsets/cascading_subsets/__init__.py (1)
each_cons(4-9)
🪛 LanguageTool
algorithms/graphs/alien_dictionary/README.md
[style] ~85-~85: Consider using a more formal alternative.
Context: ...in with the rest of the letters. To get more information, we need to look further into our Engli...
(MORE_INFO)
[grammar] ~133-~133: Use a hyphen to join words.
Context: ...etters on this new graph that have no in arrows. We can add these to our output l...
(QB_NEW_EN_HYPHEN)
[style] ~160-~160: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ... lists of all the other nodes and check whether or not they contain a link to that particular ...
(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)
[style] ~263-~263: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ...em and look at the final formula to see whether or not we can identify any relation between th...
(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)
[style] ~290-~290: You have already used this phrasing in nearby sentences. Consider omitting or replacing it to add variety to your writing.
Context: ...ency list takes O(u+min(u^2,n)) space. So, the space complexity for a large number...
(REP_SENT_START_SO)
[style] ~290-~290: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...n)) space. So, the space complexity for a large number of letters is O(u+min(u^2 ,n)). However, f...
(LARGE_NUMBER_OF)
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
86-86: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
87-87: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
111-111: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
112-112: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
210-210: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
211-211: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
212-212: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🔇 Additional comments (9)
algorithms/subsets/cascading_subsets/test_cascading_subsets.py (1)
3-3: LGTM!The import path update correctly aligns with the module relocation from
datastructures.arraystoalgorithms.subsets. Test logic remains unchanged and covers the key scenarios.DIRECTORY.md (1)
86-87: LGTM!The directory documentation updates are well-structured and consistent with existing entries. The new sections for Alien Dictionary, Recipes Supplies, and Subsets correctly reference the corresponding test files.
Also applies to: 111-112, 210-214
algorithms/subsets/find_all_subsets/__init__.py (1)
10-22: Backtracking implementation is correct.The algorithm properly:
- Appends a copy of the current subset to avoid mutation issues
- Iterates from the current index to avoid duplicate subsets
- Recurses with
i + 1to ensure proper subset generation- Backtracks by popping the last element
algorithms/subsets/find_all_subsets/test_find_all_subsets.py (1)
39-47: Test structure is well-organized.Good use of
parameterized.expandfor data-driven testing, and proper type hints on the test method parameters.algorithms/graphs/alien_dictionary/README.md (2)
1-52: Documentation is comprehensive and well-structured.The problem statement is clear with well-defined constraints, and the solution overview effectively introduces the topological sort approach. The notes about edge cases (prefixes, multiple valid orderings, cycles) are helpful.
211-292: Complexity analysis is thorough.The time complexity derivation correctly arrives at O(c) and the space complexity explanation for both the general case O(u + min(u², n)) and the fixed alphabet case O(1) is accurate.
algorithms/graphs/alien_dictionary/__init__.py (1)
76-105: LGTM!Clean implementation using
Counterfor in-degree tracking and a Pythonicfor-elseconstruct for the prefix validity check. The topological sort logic is correct.algorithms/graphs/alien_dictionary/test_alien_dictionary.py (2)
1-47: Test cases look comprehensive.Good coverage including edge cases: prefix violations (lines 30-45), single-word inputs (line 46), and scenarios expected to return empty strings.
62-63: Standard unittest entry point.
| class AlienDictionaryTestCase(unittest.TestCase): | ||
| @parameterized.expand(ALIEN_DICTIONARY_TEST_CASES) | ||
| def test_alien_order(self, words: List[str], expected: str): | ||
| actual = alien_order(words) | ||
| self.assertEqual(sorted(expected), sorted(actual)) | ||
|
|
||
| @parameterized.expand(ALIEN_DICTIONARY_TEST_CASES) | ||
| def test_alien_order_2(self, words: List[str], expected: str): | ||
| actual = alien_order_2(words) | ||
| self.assertEqual(sorted(expected), sorted(actual)) |
There was a problem hiding this comment.
Test assertions don't validate topological ordering correctness.
Using sorted(expected) == sorted(actual) only verifies that both strings contain the same characters, completely ignoring ordering constraints. For example, if expected = "cab" and actual = "bca", the test passes even if "bca" violates the derived ordering rules.
For the empty-string cases this works fine, but for valid orderings, the tests don't actually verify correctness.
Consider one of these approaches:
- Validate the actual output against the input constraints (most robust):
def is_valid_alien_order(words: List[str], order: str) -> bool:
if not order:
return False # or handle expected empty case separately
char_rank = {c: i for i, c in enumerate(order)}
for w1, w2 in zip(words, words[1:]):
for c1, c2 in zip(w1, w2):
if c1 != c2:
if char_rank.get(c1, -1) > char_rank.get(c2, -1):
return False
break
else:
if len(w1) > len(w2):
return False
return True- If the implementation is deterministic, test for exact expected output instead of sorted comparison:
self.assertEqual(expected, actual)🤖 Prompt for AI Agents
In @algorithms/graphs/alien_dictionary/test_alien_dictionary.py around lines 50
- 59, The tests test_alien_order and test_alien_order_2 currently compare
sorted(expected) to sorted(actual), which only checks multiset equality and not
topological order; replace these assertions by validating that the returned
order from alien_order and alien_order_2 respects the ordering constraints
implied by the input word list (or, if the implementations are deterministic and
expected is the unique correct order, assert exact equality). Concretely, add a
small helper is_valid_alien_order(words, order) that builds a rank map from
order and verifies each adjacent word pair respects the first differing
character ranks (and that a longer prefix before a shorter word is invalid),
then call self.assertTrue(is_valid_alien_order(words, actual)) (handle
empty/invalid expected cases accordingly) instead of using sorted(...)
comparisons.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
algorithms/graphs/alien_dictionary/README.md (1)
120-141: Standardize terminology for graph edges.The document uses inconsistent terminology when referring to edges entering a node:
- Line 120: "incoming arrows"
- Lines 133, 141: "in arrows"
- Line 158: "incoming links"
- Line 185: "incoming edges"
For consistency and technical accuracy, use "incoming edges" throughout the document, which aligns with standard graph theory terminology and the formal introduction of "in-degree" on Line 177.
📝 Suggested terminology fixes
-As we can see from the graph, four of the letters have no incoming arrows. This means that there are no letters that +As we can see from the graph, four of the letters have no incoming edges. This means that there are no letters that have to come before any of these four.-There are now three new letters on this new graph that have no in arrows. We can add these to our output list. +There are now three new letters on this new graph that have no incoming edges. We can add these to our output list.-Then, we add the two new letters with no in arrows. +Then, we add the two new letters with no incoming edges.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
algorithms/graphs/alien_dictionary/README.md
🧰 Additional context used
🪛 LanguageTool
algorithms/graphs/alien_dictionary/README.md
[style] ~85-~85: Consider using a more formal alternative.
Context: ...in with the rest of the letters. To get more information, we need to look further into our Engli...
(MORE_INFO)
[grammar] ~133-~133: Use a hyphen to join words.
Context: ...etters on this new graph that have no in arrows. We can add these to our output l...
(QB_NEW_EN_HYPHEN)
[style] ~160-~160: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ... lists of all the other nodes and check whether or not they contain a link to that particular ...
(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)
[style] ~260-~260: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ...em and look at the final formula to see whether or not we can identify any relation between th...
(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)
[style] ~287-~287: You have already used this phrasing in nearby sentences. Consider omitting or replacing it to add variety to your writing.
Context: ...ency list takes O(u+min(u^2,n)) space. So, the space complexity for a large number...
(REP_SENT_START_SO)
[style] ~287-~287: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...n)) space. So, the space complexity for a large number of letters is O(u+min(u^2 ,n)). However, f...
(LARGE_NUMBER_OF)
🔇 Additional comments (2)
algorithms/graphs/alien_dictionary/README.md (2)
1-30: LGTM! Clear problem statement and constraints.The problem description is well-structured, provides clear constraints, and includes helpful notes about lexicographical ordering. The example references follow a logical naming convention.
197-289: Excellent complexity analysis!The time and space complexity derivations are mathematically rigorous and well-explained. The analysis correctly accounts for all algorithm components and provides clear reasoning for the final O(c) time complexity and O(1) space complexity (for fixed alphabet size).
Describe your change:
Algorithm to return a list of all possible recipes from supplies
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.