feat(analysis-engine): add capo and tuning detection heuristics - #103
Merged
Conversation
Implements initial heuristics to detect Capo and alternative tunings (like Drop D) from the chord list, and integrates this into the acoustic guitar role setup notes. Adds robust test coverage to maintain the 100% threshold.
Collaborator
Author
|
@coderabbitai review |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough코드의 코드 분석 기능에 카포와 튜닝 감지 로직을 추가하고, 역할 추출기에 어쿠스틱 기타 역할을 통합하며 설정 노트 기능을 구현했습니다. 관련 테스트 커버리지도 추가되었습니다. Changes
Sequence DiagramsequenceDiagram
participant RoleExtractor
participant get_setup_note
participant detect_capo_and_tuning
participant ChordDatabase
RoleExtractor->>RoleExtractor: create acoustic_guitar_role<br/>with setupNote field
RoleExtractor->>get_setup_note: call get_setup_note(role_name, chords)
get_setup_note->>get_setup_note: check if guitar role<br/>(contains "guitar", not "bass")
get_setup_note->>detect_capo_and_tuning: call detect_capo_and_tuning(chords)
detect_capo_and_tuning->>detect_capo_and_tuning: analyze chord patterns<br/>for capo and tuning hints
detect_capo_and_tuning-->>get_setup_note: return {capo, tuning}
get_setup_note->>get_setup_note: format setup string<br/>e.g., "Setup: Standard tuning, Capo 1"
get_setup_note-->>RoleExtractor: return setupNote string or None
RoleExtractor->>RoleExtractor: assign setupNote to role
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분 Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
✅ Actions performedReview triggered.
|
seonghobae
enabled auto-merge (squash)
March 28, 2026 11:09
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/analysis-engine/src/bandscope_analysis/chords/capo.py`:
- Line 1: 이 파일(src/bandscope_analysis/chords/capo.py)의 Ruff 포맷팅 오류를 고치려면 ruff
포맷터를 실행하여 코드 스타일을 자동으로 정리하세요: 터미널에서 ruff format
src/bandscope_analysis/chords/capo.py를 실행한 뒤 변경사항을 검토하고 커밋합니다; 모듈명 capo.py와 관련된
함수/클래스(예: capo 관련 함수들)가 있다면 포맷팅 후 임포트 정렬, 공백, 줄바꿈 규칙이 올바른지 확인하여 린트가 통과하도록 합니다.
- Around line 20-24: Remove the dead if-block that checks for drop D indicators:
the conditional "if 'D5' in chords_set or 'Eb' in chords_set and 'D' in
chords_set" is a no-op (contains only pass), has an operator-precedence bug (and
vs or), and duplicates the simpler drop-D logic implemented later; delete this
entire if block referencing chords_set so only the later, intended logic
remains.
In `@services/analysis-engine/src/bandscope_analysis/roles/extractor.py`:
- Around line 236-237: The two lines that mutate part_graph using hardcoded
indices (part_graph[0]["handoff_to"].append("lead-vocal") and
part_graph[4]["handoff_from"].append("bass-guitar")) are fragile; instead locate
nodes by their role_id (e.g., "lead-vocal", "bass-guitar") and update their
handoff lists. Implement or use a helper like find_node_by_role(graph, role_id)
that returns the node (or None) and then append to node["handoff_to"] or
node["handoff_from"] accordingly, and guard against missing nodes before
appending.
In `@services/analysis-engine/src/bandscope_analysis/roles/tuning.py`:
- Around line 22-23: The assignment capo = result["tuning"] if result["capo"] ==
0 else f"Capo {result['capo']} is dead and confusing because later code only
uses capo when result["capo"] > 0; remove the unnecessary branch and only assign
capo when needed (e.g., set capo = f"Capo {result['capo']}" inside the block
guarded by result["capo"] > 0), keeping tuning = result["tuning"] unchanged so
there is no redundant or unused assignment.
In `@services/analysis-engine/tests/test_chords.py`:
- Around line 1-28: The file ends without a trailing newline which causes ruff
format --check to fail; open the test file containing the tests that call
detect_capo_and_tuning (e.g., functions test_detect_capo_standard,
test_detect_capo_fret1, test_detect_capo_empty, test_detect_drop_d) and add a
single newline character at EOF so the file ends with a blank line.
In `@services/analysis-engine/tests/test_tuning.py`:
- Around line 1-30: The ruff formatting check failed because the test file
tests/test_tuning.py is missing a trailing newline at EOF; open the file
(contains functions like test_get_setup_note_acoustic_guitar,
test_get_setup_note_drop_d) and add a single blank line/newline character at the
end of the file so the file ends with a newline, then re-run ruff format/check.
🪄 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: d135a3e9-116b-406b-88d4-cd16ef6ced42
📒 Files selected for processing (9)
services/analysis-engine/src/bandscope_analysis/chords/__init__.pyservices/analysis-engine/src/bandscope_analysis/chords/capo.pyservices/analysis-engine/src/bandscope_analysis/roles/__init__.pyservices/analysis-engine/src/bandscope_analysis/roles/extractor.pyservices/analysis-engine/src/bandscope_analysis/roles/tuning.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_roles.pyservices/analysis-engine/tests/test_tuning.py
- Remove dead if block in chords/capo.py - Refactor fragile hardcoded index graph mutation in roles/extractor.py - Remove unused assignment in roles/tuning.py - Remove accidentally tracked agent JSON files and update .gitignore
Collaborator
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved and changes approved. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Implements initial Capo and alternative tuning (like Drop D) detection heuristics from song chord lists. It enhances the
acoustic-guitarrole setup note with the recommended configuration.Changes
chords/capo.py: Basic chord-set-based detection heuristics for Capo and Tunings (Standard, Drop D, Capo 1).roles/tuning.py: Determines appropriatesetupNotestrings.roles/extractor.py: Addsacoustic-guitarrole dynamically to the graph and uses the new setup note logic.lenassertions intest_roles.pyandtest_api.py.services/analysis-engine.