fix: use subordinate_id/supervisor_id in HierarchyResolver#751
fix: use subordinate_id/supervisor_id in HierarchyResolver#751
Conversation
HierarchyResolver used bare role names as dict keys when processing explicit reporting lines, collapsing agents that share a role name into a single entry. Add subordinate_key/supervisor_key computed properties to ReportingLine and use them as dict keys so agents with different IDs are properly distinguished. Closes #746 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Update HierarchyResolver docstring to reflect subordinate_key/supervisor_key usage. Strengthen override test with distinct member name vs ID. Add chain-walking test with ID keys and cycle detection test with ID keys. Pre-reviewed by 9 agents, 4 findings addressed Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🧰 Additional context used📓 Path-based instructions (4)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/!(observability)/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (9)📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
📚 Learning: 2026-03-15T18:42:17.990ZApplied to files:
📚 Learning: 2026-03-17T11:41:02.964ZApplied to files:
📚 Learning: 2026-03-22T19:37:56.694ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
📚 Learning: 2026-03-15T18:38:44.202ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
🧬 Code graph analysis (3)tests/unit/core/test_company_reporting.py (1)
src/synthorg/communication/delegation/hierarchy.py (1)
tests/unit/communication/delegation/test_hierarchy.py (2)
🔇 Additional comments (6)
WalkthroughHierarchy resolution now keys reporting relationships by identity rather than by name. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the hierarchy resolution logic by incorporating subordinate and supervisor IDs in addition to role names. This change ensures that agents with the same role name but different IDs are correctly distinguished in the reporting hierarchy. The update includes new computed properties and corresponding tests to validate the new functionality and maintain backward compatibility. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of distinguishing agents with identical role names by leveraging subordinate_id and supervisor_id. The introduction of subordinate_key and supervisor_key as computed properties on the ReportingLine model is a clean and appropriate solution. The HierarchyResolver is correctly updated to use these new keys, and the accompanying tests are comprehensive, covering various scenarios including disambiguation, overrides, and cycle detection. I have one minor suggestion to improve the clarity and efficiency of the code in HierarchyResolver.
| old_reports = reports_of.get(old_sup, []) | ||
| reports_of[old_sup] = [ | ||
| r for r in old_reports if r != line.subordinate | ||
| ] | ||
| supervisor_of[line.subordinate] = line.supervisor | ||
| reports_of.setdefault(line.supervisor, []).append( | ||
| line.subordinate, | ||
| ) | ||
| reports_of[old_sup] = [r for r in old_reports if r != sub_key] |
There was a problem hiding this comment.
The list comprehension used to remove the subordinate from their old supervisor's list of reports is functional but can be made more direct and readable. Since the logic ensures that sub_key will be present in reports_of[old_sup], you can use list.remove() to modify the list in-place. This more clearly expresses the intent of removing an element and avoids creating a new list.
| old_reports = reports_of.get(old_sup, []) | |
| reports_of[old_sup] = [ | |
| r for r in old_reports if r != line.subordinate | |
| ] | |
| supervisor_of[line.subordinate] = line.supervisor | |
| reports_of.setdefault(line.supervisor, []).append( | |
| line.subordinate, | |
| ) | |
| reports_of[old_sup] = [r for r in old_reports if r != sub_key] | |
| reports_of[old_sup].remove(sub_key) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #751 +/- ##
==========================================
+ Coverage 92.28% 92.29% +0.01%
==========================================
Files 573 573
Lines 29690 29704 +14
Branches 2877 2879 +2
==========================================
+ Hits 27398 27414 +16
+ Misses 1811 1810 -1
+ Partials 481 480 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Document normalization asymmetry between _identity_key and computed keys in subordinate_key/supervisor_key docstrings - Fix HierarchyResolver class docstring: "resolve to" instead of "prefer" - Update all public method Args docs to say "name or ID" - Fix misleading comment in override test (alice is not "distinct") - Add assertions for old supervisor reports removal in override test - Add negative assertion for raw role name when IDs are used - Add get_lowest_common_manager test with ID-based keys - Add head_id-not-used-by-resolver documentation test - Add serialization round-trip test for computed fields Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [0.4.8](v0.4.7...v0.4.8) (2026-03-22) ### Features * add auto_cleanup config and improve update UX ([#741](#741)) ([289638f](289638f)) * add reporting lines, escalation paths, and workflow handoffs to templates ([#745](#745)) ([c374cc9](c374cc9)) * differentiate template operational configs ([#742](#742)) ([9b48345](9b48345)) * diversify personality preset assignments across templates ([#743](#743)) ([15487a5](15487a5)) * improve template metadata -- skill taxonomy, descriptions, tags, and display names ([#752](#752)) ([f333f24](f333f24)) ### Bug Fixes * resolve log analysis findings (Ollama prefix, logging, init) ([#748](#748)) ([8f871a4](8f871a4)) * use git tag for dev release container image tags ([#749](#749)) ([f30d071](f30d071)) * use subordinate_id/supervisor_id in HierarchyResolver ([#751](#751)) ([118235b](118235b)) ### Performance * add long-lived cache headers for content-hashed static assets ([#747](#747)) ([4d350b5](4d350b5)) * use worksteal distribution for pytest-xdist ([#750](#750)) ([b7dd7de](b7dd7de)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
subordinate_key/supervisor_key@computed_fieldproperties toReportingLinethat returnsubordinate_id/supervisor_idwhen set, falling back to the role nameHierarchyResolver.__init__to use these properties as dict keys in the explicit reporting lines loop, so agents sharing a role name but with different IDs are properly distinguishedTest plan
uv run python -m pytest tests/unit/core/test_company_reporting.py tests/unit/communication/delegation/test_hierarchy.py -n auto -v-- all 91 tests passuv run mypy src/ tests/-- cleanuv run ruff check src/ tests/-- clean🤖 Generated with Claude Code
Closes #746