Skip to content

fix(ci): install workspace packages as editable + fix lint#12

Merged
polaz merged 4 commits intomainfrom
fix/#11-ci-editable-install
Apr 9, 2026
Merged

fix(ci): install workspace packages as editable + fix lint#12
polaz merged 4 commits intomainfrom
fix/#11-ci-editable-install

Conversation

@polaz
Copy link
Copy Markdown
Member

@polaz polaz commented Apr 9, 2026

Summary

  • Fix uv sync in CI test jobs: add --all-packages so workspace members are installed as editable packages
  • Fix ruff.toml: update proto stubs exclude path after package restructure in fix(coordinode): move package into coordinode/ subdirectory for correct wheel build #10
  • Fix tests/unit/test_types.py: I001 import sorting (blank line between same-group imports)
  • Fix Makefile clean target: also removes coordinode/_proto/google
  • Remove coordinode/_proto/__init__.py that was mistakenly committed (generated file)

Root Cause

After #10 moved Python source files into coordinode/coordinode/ subdirectory, uv sync without --all-packages no longer installs the coordinode workspace package as an editable. Previously, tests worked accidentally because coordinode/__init__.py existed at the workspace member root and Python's CWD resolution treated it as the coordinode package. With the new structure there's no __init__.py at the root, so Python sees a namespace package and CoordinodeClient is not found.

uv sync --all-packages creates a _coordinode.pth file pointing to the workspace member directory, which allows Python to correctly resolve coordinodecoordinode/coordinode/__init__.py.

Verification

Locally: lint passes, 18/18 unit tests pass.

Closes #11

- ci.yml: use 'uv sync --all-packages' in test jobs so workspace members
  (coordinode, langchain-coordinode, llama-index-graph-stores-coordinode)
  are installed as proper editable packages via .pth files. Without this,
  uv sync only installs dev dependencies and leaves workspace packages
  uninstalled, causing ModuleNotFoundError in tests.

- ruff.toml: update proto stubs exclude path to coordinode/coordinode/_proto/
  (moved in #10); exclude _version.py (generated by hatch-vcs, unformatted)

- tests/unit/test_types.py: fix I001 — remove blank line between same-group
  imports (pytest + coordinode._types are both third-party)

- Makefile: clean target also removes coordinode/coordinode/_proto/google
  (google.api stubs generated alongside coordinode stubs)

- Remove coordinode/_proto/__init__.py that was mistakenly committed to git
  (generated file, should only exist in gitignored coordinode/_proto/)
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5879af1c-ac80-46aa-8761-860dc5d925aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2a1b542 and a9f5964.

📒 Files selected for processing (1)
  • ruff.toml

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated CI workflow to improve dependency installation process
    • Enhanced build cleanup procedures for generated artifacts
    • Refined code quality and import sorting configuration for development tooling
    • Removed outdated internal documentation comments

Walkthrough

The changes adjust build configuration, CI/CD workflows, and linting rules following a package structure reorganization where the coordinode package was moved into a coordinode/ subdirectory. Updates include modified dependency installation commands, expanded cleanup targets, and adjusted path exclusions in linting configuration.

Changes

Cohort / File(s) Summary
CI/CD Configuration
.github/workflows/ci.yml
Updated dependency installation to use uv sync --all-packages in test and test-integration jobs instead of uv sync.
Build Configuration
Makefile
Broadened the clean target to delete entire $(PROTO_OUT) directory instead of only $(PROTO_OUT)/coordinode.
Linting Configuration
ruff.toml
Updated proto stubs exclusion path from coordinode/_proto/ to coordinode/coordinode/_proto/, added **/_version.py file exclusion, and configured isort settings with known-first-party modules.
Generated Code Documentation
coordinode/_proto/__init__.py
Removed module-level comments documenting generated gRPC stubs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: fixing CI dependency installation and linting issues.
Description check ✅ Passed The description clearly explains the root cause, changes made, and verification steps for all modifications in the PR.
Linked Issues check ✅ Passed The PR directly addresses the package restructure fixes required by issue #11, updating CI configuration and build artifacts accordingly.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing issues introduced by the package restructure in #10, with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/#11-ci-editable-install

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

Copy link
Copy Markdown

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Line 57: The Makefile currently hard-codes subpaths in the cleanup step (the
rm -rf $(PROTO_OUT)/coordinode $(PROTO_OUT)/google line) which can leave stale
generated files; change that target to remove all contents under the
$(PROTO_OUT) generated output directory instead of listing namespaces
explicitly—use a safe deletion that removes every child entry of $(PROTO_OUT)
but not the directory itself (for example, replace the hard-coded rm with a
deletion that operates on all entries under $(PROTO_OUT), or use a find-based
deletion with mindepth 1) so future proto namespaces are automatically cleaned.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 782b5b0f-6e3e-4ce6-97d7-e3a17213ac19

📥 Commits

Reviewing files that changed from the base of the PR and between 2a63dd0 and f5d0b2c.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • Makefile
  • coordinode/_proto/__init__.py
  • ruff.toml
  • tests/unit/test_types.py
💤 Files with no reviewable changes (2)
  • tests/unit/test_types.py
  • coordinode/_proto/init.py

Comment thread Makefile Outdated
@polaz polaz requested a review from Copilot April 9, 2026 14:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates CI + local tooling to correctly work with the post-#10 workspace/package layout, ensuring the coordinode package is installed as an editable workspace member so tests can import it reliably.

Changes:

  • CI: run uv sync --all-packages in test jobs so workspace members are installed as editable packages.
  • Tooling/lint fixes: update Ruff excludes for moved proto stubs; resolve a minor import-formatting issue in unit tests.
  • Cleanup: expand make clean to remove generated google protos; delete a mistakenly committed generated _proto/__init__.py.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/test_types.py Import formatting tweak to satisfy Ruff import rules.
ruff.toml Update excluded generated paths after package restructure.
Makefile Clean target now removes additional generated proto output (google).
coordinode/_proto/init.py Removes mistakenly committed generated stub file.
.github/workflows/ci.yml Installs workspace packages as editable in CI test jobs via --all-packages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ruff.toml Outdated
@polaz polaz requested a review from Copilot April 9, 2026 14:43
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 9, 2026

@coderabbitai re-review

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/unit/test_types.py
@polaz
Copy link
Copy Markdown
Member Author

polaz commented Apr 9, 2026

@coderabbitai re-review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

@polaz: Sure! I'll re-review the changes in this PR now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…rt grouping

Without explicit known-first-party, ruff classifies coordinode as third-party
when the package is not installed as editable (e.g. plain `uv sync` in the
lint job). This caused inconsistent blank-line requirements across test files.
Setting known-first-party ensures consistent import grouping regardless of the
install state.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 9, 2026

@polaz polaz merged commit e6e503f into main Apr 9, 2026
7 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