Skip to content

Conversation

@BrianLusina
Copy link
Owner

@BrianLusina BrianLusina commented Nov 14, 2025

Describe your change:

Adds zig zag level order traversal algorithm

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Summary by CodeRabbit

  • New Features
    • Added zigzag (alternating-direction) level-order traversal for binary trees, allowing consumers to retrieve nodes level-by-level with direction flipping each level.
    • Traversal handles empty trees and documents expected performance characteristics.
@BrianLusina BrianLusina self-assigned this Nov 14, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

A new public method zig_zag_level_order() was added to BinaryTree implementing BFS-based zigzag level-order traversal. The typing imports were updated to include Tuple, and the local stack in longest_zig_zag_stack received a precise List[Tuple[BinaryTreeNode | None, int, str | None]] annotation.

Changes

Cohort / File(s) Summary
Zigzag Traversal + Type Annotations
datastructures/trees/binary/tree/__init__.py
Added zig_zag_level_order(self) -> List[List[BinaryTreeNode]] implementing BFS zigzag level-order traversal (alternating reversal per level); added Tuple to typing imports; annotated stack in longest_zig_zag_stack as List[Tuple[BinaryTreeNode | None, int, str | None]].

Sequence Diagram(s)

sequenceDiagram participant Client participant BinaryTree participant Queue participant Result Client->>BinaryTree: zig_zag_level_order() activate BinaryTree BinaryTree->>Queue: enqueue(root) if root exists loop per level BinaryTree->>Queue: dequeue all nodes at current level BinaryTree->>BinaryTree: collect node list for level alt odd level BinaryTree->>Result: append reversed node list else even level BinaryTree->>Result: append node list as-is end BinaryTree->>Queue: enqueue children of current level end BinaryTree-->>Client: return Result (List[List[BinaryTreeNode]]) deactivate BinaryTree 
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Changes are localized to one file and consist of a standard traversal implementation plus a small typing refinement.
  • Review focus:
    • Correctness of level reversal logic and empty-tree handling in zig_zag_level_order.
    • Type consistency with existing BinaryTreeNode imports and public API signature.

Possibly related PRs

Poem

Hop hop through levels, I bound and I peep,
Left, then right, in a zigzaggy sweep.
Queues carry leaves in a neat, rhythmed beat,
I nibble on nodes — traversal tastes sweet! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a zig zag level order traversal algorithm to the binary tree data structure.
Description check ✅ Passed The pull request description follows the required template with all major sections completed, checklist items marked, and most compliance requirements confirmed as met.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a9a7ab and 773eeba.

📒 Files selected for processing (1)
  • datastructures/trees/binary/tree/__init__.py (3 hunks)

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
datastructures/trees/binary/tree/__init__.py (1)

653-653: Consider tightening the type annotation.

The type annotation allows None for the BinaryTreeNode, but in practice, the first element of tuples in the stack is never None because:

  1. The initial element is self.root, which is guaranteed non-None by the guard on line 649
  2. All subsequent appends (lines 659, 661, 664, 666, 670, 672) check that node.left or node.right is not None before appending

Consider applying this diff to tighten the type:

- stack: List[Tuple[BinaryTreeNode | None, int, str | None]] = [(self.root, 0, None)] + stack: List[Tuple[BinaryTreeNode, int, str | None]] = [(self.root, 0, None)]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a081e7 and 0a9a7ab.

📒 Files selected for processing (1)
  • datastructures/trees/binary/tree/__init__.py (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
datastructures/trees/binary/tree/__init__.py (1)
datastructures/trees/binary/node.py (1)
  • BinaryTreeNode (6-184)
🔇 Additional comments (2)
datastructures/trees/binary/tree/__init__.py (2)

2-2: LGTM!

The Tuple import is correctly added to support the type annotation on line 653.


677-677: Verify the return type choice.

The method returns List[List[BinaryTreeNode]] (lists of node objects), which differs from level_order_traversal (lines 85-107) that returns List[Any] containing node values (node.data).

Most standard zigzag level-order traversal implementations return node values rather than node objects. Please confirm this design choice is intentional.

If values are intended instead of nodes, apply this diff:

- def zig_zag_level_order(self) -> List[List[BinaryTreeNode]]: + def zig_zag_level_order(self) -> List[List[T]]: """ Perform zigzag level order traversal of a binary tree. Returns: - List[List[BinaryTreeNode]] - zigzag level order traversal + List[List[T]] - zigzag level order traversal of node values

And update line 707:

- current_level.append(node) + current_level.append(node.data)
@BrianLusina BrianLusina merged commit a90f0fc into main Nov 15, 2025
4 of 7 checks passed
@BrianLusina BrianLusina deleted the feat/datastructures-binary-tree-zigzag-level-order branch November 15, 2025 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Algorithm Algorithm Problem Binary Tree Datastructures Datastructures Documentation Documentation Updates enhancement Trees

2 participants