Skip to content

Fix: exclude alloc-only fanin from star markers#1374

Merged
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
Little-oil:codex/deps-viewer-star-non-alloc
Jul 16, 2026
Merged

Fix: exclude alloc-only fanin from star markers#1374
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
Little-oil:codex/deps-viewer-star-non-alloc

Conversation

@Little-oil

Copy link
Copy Markdown
Contributor

Summary

  • require at least one non-alloc predecessor before adding a ⭐ marker
  • preserve alloc tasks as 🔥 sources when mixed with early-dispatch tasks
  • cover alloc-only, mixed, and missing-🔥 predecessor cases

Testing

  • .venv/bin/python -m pytest tests/ut -q (594 passed, 2 skipped)
  • .venv/bin/python -m pytest tests/ut/py/test_deps_viewer.py -q (28 passed after rebase)
  • ruff check and ruff format --check on changed files

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refines the logic for applying the star (⭐) marker in the dependency viewer, ensuring that a task is only marked if all of its predecessors are fire-marked (🔥) and at least one predecessor is not an allocation task. Unit tests have been updated to verify these changes. The review feedback suggests optimizing this logic by leveraging Python's short-circuiting to avoid unnecessary predecessor checks when not all predecessors are fire-marked.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread simpler_setup/tools/deps_viewer.py Outdated
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 32d26892-f977-4dce-a977-1265145aa72e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The dependency viewer now emits “⭐” only when all predecessors are fire-marked and at least one is non-alloc. Tests cover alloc-only, mixed early-dispatch/alloc, and non-fire-ready predecessor cases.

Changes

Dependency star marker rules

Layer / File(s) Summary
Star marker computation and coverage
simpler_setup/tools/deps_viewer.py, tests/ut/py/test_deps_viewer.py
The star condition now requires a non-alloc predecessor alongside all fire-marked predecessors. Tests verify alloc-only fan-in is unmarked, mixed predecessors are marked, and non-fire-ready predecessors remain unmarked.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Poem

A bunny hops through dependency light,
No alloc-only stars tonight.
Mixed fires make the marker gleam,
While sleepy predecessors spoil the dream.
Tests guard each branching stream.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: blocking star markers for alloc-only fan-in.
Description check ✅ Passed The description accurately summarizes the code and test changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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.

@Little-oil
Little-oil force-pushed the codex/deps-viewer-star-non-alloc branch from 5d71962 to c0042f8 Compare July 16, 2026 09:11
Require at least one non-alloc predecessor before adding the star
marker, while still treating alloc tasks as fire-marked sources. Cover
alloc-only, mixed early-dispatch, and missing-fire fanin cases.
@ChaoZheng109
ChaoZheng109 merged commit 172aec1 into hw-native-sys:main Jul 16, 2026
16 checks passed
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