feat(ui): redesign workspace with shadcn and tailwind v4#164
Merged
Conversation
This refactors the frontend to use Tailwind CSS v4 and Shadcn UI components. It improves the layout and typography of the App, Workspace, RoleSwitcher, and SectionRoadmap.
Owner
Author
|
@coderabbitai resolve |
|
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이 PR은 데스크톱 앱의 Shadcn UI 컴포넌트 통합, Python 기반의 음악 분석 엔진(코드 인식, 음정 추적, 음역대 분석, 소스 분리, 템포 분석), 공유 타입 확장(overlapWarnings, partGraph 필드 추가), GitHub Actions 워크플로우 보안 강화, ESLint JSDoc 요구사항 추가, 그리고 광범위한 테스트 커버리지를 도입합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as Desktop App
participant Tauri as Tauri Backend
participant Engine as Analysis Engine
participant Models as ML Models
User->>App: Import YouTube URL or select local audio
App->>App: Validate URL (protocol, structure)
App->>Tauri: invoke('import_youtube_url' or 'select_local_audio')
Tauri->>Engine: Pass audio path to analysis job
Engine->>Engine: TemporalAnalyzer: Extract tempo, beats
Engine->>Models: PitchTracker: Track vocal/bass pitch ranges
Engine->>Models: ChordRecognizer: Identify chords from stems
Engine->>Models: RangeAnalyzer: Compute overlaps
Engine->>Engine: RoleExtractor: Synthesize roles with detected features
Engine-->>Tauri: Return RehearsalSong with sections/roles/metadata
Tauri-->>App: Return analysis result
App->>App: Render song data in workspace features
App->>App: Display roles, ranges, chords, overlaps
sequenceDiagram
participant Dev as Developer
participant GH as GitHub
participant Bandit as Bandit
participant Trivy as Trivy
participant Scorecard as OSSF Scorecard
Dev->>GH: Push to main/develop
GH->>Bandit: Run bandit security scan (new workflow)
Bandit-->>GH: Report Python security issues
GH->>Trivy: Scan container/filesystem
Trivy-->>GH: Report vulnerability findings
GH->>Scorecard: Run OSSF scorecard (conditional publish for develop)
Scorecard-->>GH: Publish SARIF (if develop branch)
GH->>GH: Enforce job-scoped permissions (contents:read, security-events:write)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 54
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/bandit.yml:
- Around line 17-20: Add a job-level timeout to the Bandit job to avoid blocking
the PR pipeline: in the job stanza for "bandit-scan" add a timeout-minutes key
(e.g., timeout-minutes: 10 or another project-appropriate value) so the
"bandit-scan" job will be forcibly stopped after that duration; update the
"bandit-scan" job definition to include this key alongside existing keys (name,
runs-on, steps) to ensure the workflow unblocks on runaway scans.
In @.github/workflows/ossf-scorecard.yml:
- Line 28: Replace the hardcoded branch check for publish_results with a dynamic
check against the repository default branch: change the existing
publish_results: ${{ github.ref == 'refs/heads/develop' }} to use the repository
default branch via format, e.g. publish_results: ${{ github.ref ==
format('refs/heads/{0}', github.event.repository.default_branch) }}, so the
Scorecard publishing follows the repo's configured default branch (update the
publish_results line in the workflow).
In `@apps/desktop/package.json`:
- Line 23: Move the "shadcn" dependency out of runtime dependencies and into
devDependencies in package.json: remove the "shadcn": "^4.5.0" entry from the
top-level "dependencies" and add the same entry under "devDependencies" (ensure
no runtime imports of shadcn remain), then update the lockfile by running your
package manager (npm/yarn/pnpm install) so the change is reflected.
In `@apps/desktop/src-tauri/.cargo/audit.toml`:
- Around line 2-20: The audit config currently ignores advisory
RUSTSEC-2024-0429 but the project uses glib 0.18.5 (within the vulnerable
range); remove "RUSTSEC-2024-0429" from the ignore array in .cargo/audit.toml to
restore CI alerts, or alternatively upgrade the glib dependency to >=0.20.0 in
your Cargo.toml (update any references to glib 0.18.5) so the advisory is no
longer applicable; target either removing the entry from the ignore list or
bumping glib to 0.20.0+ and run audits to confirm the warning is gone.
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 790-805: The code trusts metadata.get("filepath") and uses
Path::new(filepath) directly (variables filepath, path, metadata_fs), allowing
path traversal; fix by canonicalizing the resolved path (std::fs::canonicalize)
and canonicalizing your cache_root, then check that the canonicalized path
starts_with the canonicalized cache_root before calling std::fs::metadata or
using the file; if the check fails return an error (e.g., "downloaded audio file
outside cache root"), and keep the existing extension extraction but operate on
the validated canonical path. Ensure the same canonicalize+starts_with
validation is applied to the other similar block referenced (the code around the
second occurrence).
In `@apps/desktop/src/App.test.tsx`:
- Around line 755-758: The test is an anti-pattern because it manipulates the
DOM by calling saveButton.removeAttribute("disabled") to force a click; either
delete this DOM-level test or convert it to a unit test that calls the handler
directly: remove the lines interacting with saveButton and instead write a unit
test that imports/creates the handleSaveProject function (or the component
instance method) and invokes handleSaveProject with jobResult = null, asserting
mockSaveProject is not called; reference saveButton, handleSaveProject,
mockSaveProject, and jobResult to locate the relevant code to change.
In `@apps/desktop/src/App.tsx`:
- Around line 185-188: The handler handleSaveProject currently calls
saveProject(jobResult!) using a non-null assertion which can cause a runtime
crash; replace the non-null assertion with a defensive null check inside
handleSaveProject: validate that jobResult is defined (e.g., if (!jobResult)
return or show an error) before calling saveProject, and propagate or log a
clear error when missing so callers and future code paths are safe without
relying on UI disablement.
In `@apps/desktop/src/components/ui/progress.tsx`:
- Around line 7-24: The Progress component currently injects its own
ProgressTrack and ProgressIndicator while also exporting those subcomponents,
causing duplicate bars when consumers compose
(<Progress><ProgressTrack>...</ProgressTrack></Progress>); remove the internal
default track/indicator so Progress becomes composition-only: in the Progress
function (the component wrapping ProgressPrimitive.Root) stop rendering
<ProgressTrack> and <ProgressIndicator> and render only children (preserve
value, data-slot, className and {...props}), and update any docs/tests to show
consumers must include <ProgressTrack> and <ProgressIndicator> themselves;
alternatively, if you want a closed component instead, make that choice
explicitly by removing exported ProgressTrack/ProgressIndicator and keeping the
internal ones or by adding a prop (e.g., defaultTrack) to toggle insertion—pick
one approach and apply it consistently to Progress, ProgressTrack, and
ProgressIndicator.
In `@apps/desktop/src/components/ui/scroll-area.tsx`:
- Around line 35-43: The Tailwind attribute selectors in the
ScrollAreaPrimitive.Scrollbar class are incorrect (using
data-horizontal:/data-vertical:) so orientation-specific styles don't apply;
update the className in ScrollAreaPrimitive.Scrollbar (the JSX where orientation
and data-orientation are set) to use Tailwind v4 attribute syntax
data-[orientation=horizontal]: and data-[orientation=vertical]: for all
instances (e.g., replace data-horizontal:... with
data-[orientation=horizontal]:... and data-vertical:... with
data-[orientation=vertical]:...) while preserving the existing utility classes
and passing through className and {...props}.
In `@apps/desktop/src/components/ui/tabs.tsx`:
- Around line 12-19: The TabsPrimitive.Root is missing the orientation prop
which breaks direction-sensitive behavior and CSS selectors; update the Tabs
component to pass orientation={orientation} into TabsPrimitive.Root (where
TabsPrimitive.Root, orientation, cn, and className are used) so the base
primitive receives the orientation and enables correct ARIA/keyboard handling
and data-* selectors to activate.
In `@apps/desktop/src/features/chords/index.tsx`:
- Around line 7-13: The empty-state JSX in the chords component uses inline
style props—replace this with the shared UI primitives and Tailwind classes used
elsewhere: wrap the content in the common Card (or CardFallback) component
instead of a bare <section>, use the shared heading/typography components for
{title}, and convert the paragraph to a Badge/MutedText styled with
Tailwind/dark-mode tokens; update the conditional that checks song (the early
return block in apps/desktop/src/features/chords/index.tsx) and apply the same
replacement to the other similar blocks between the Song-check return and the
larger chords UI (the areas noted around lines 31–77) so the component uses the
project’s Card/Badge/typography primitives and Tailwind classes rather than
inline style props.
In `@apps/desktop/src/features/home/index.tsx`:
- Around line 8-43: This component still uses inline style objects (e.g., the
root <section>, <h2>, the paragraph and the three info cards, and the
empty-state <div>) instead of Tailwind classes; replace those style={{...}}
usages in the JSX (look for identifiers song, title, song.exportSummary, and the
three info card containers) with equivalent Tailwind utility class names
(padding, text sizes/colors, flex/gap/flex-wrap, bg-*, rounded, min-w-*,
italics, centered layout, etc.), preserving the existing structure and dynamic
expressions (like {song.sections.length} and the Set calculation) and ensure the
empty-state and conditional headline rendering use Tailwind classes consistent
with Workspace.tsx and SectionRoadmap.tsx patterns.
In `@apps/desktop/src/features/player/index.tsx`:
- Around line 7-55: The Player component currently uses inline style props for
the root section, the metadata/card container, the title/meta block, the
sections container, each section "pill", and the footer note (see usage around
song, song.sections.map and the surrounding divs); extract those inline styles
into shared CSS classes or a shadcn/Card + tokens-based components (e.g.,
replace the outer <section> padding, the inner card
padding/background/borderRadius/border, the title/meta spacing, the sections
wrapper flex/gap, and the pill styling) and apply theme-aware token values
(spacing, colors, border, radius, font sizes) so dark mode and global design
tokens are honored; update JSX to use the new classNames or shadcn components
and remove the inline style props from the elements that render song metadata
and section pills.
In `@apps/desktop/src/features/ranges/index.tsx`:
- Around line 11-55: Replace hardcoded user-facing strings with i18n keys using
useTranslation(); import and call const { t } = useTranslation() in this
component and replace the paragraph "No song loaded. Start an analysis to see
range data." with t('ranges.noSongLoaded'), wrap the range display as
t('ranges.range', { low: role.range.lowestNote, high: role.range.highestNote })
(keep emoji outside translation if desired), replace inline warning text
rendering to t(...) instead of raw warning strings (or ensure
role.overlapWarnings contains translation keys and call t(warning)), and ensure
section.label, role.name and title are provided as localized strings or
converted to translation keys (refer to useTranslation, role.overlapWarnings,
section.label, role.name, title in this component).
- Around line 42-45: Replace the index-based key used in the
role.overlapWarnings map to a stable, meaning-based key to avoid UI state
mismatches; update the mapping that renders {role.overlapWarnings.map((warning,
wIndex) => (... key={wIndex} ...))} to compute a unique key per item (for
example by joining the warning string with the role.id or another stable
identifier: e.g., `${role.id}-${warning}`) and use that expression as the key
prop instead of wIndex.
In `@apps/desktop/src/features/settings/index.tsx`:
- Around line 12-45: Replace the hardcoded UI strings in this settings component
with i18n keys: swap "Supported Audio Formats", each list item under "Analysis
Pipeline" ("Decode audio source", "Draft section and role extraction", "Separate
stems by category", "Persist analysis results"), "About" and the paragraph
"BandScope is a local-first rehearsal prep tool. All analysis runs on your
device." for translation lookups (e.g., using the t function from
useTranslation). Update the JSX where those literals appear (the headings, the
li elements, and the paragraph) to call the translation keys (e.g.,
t('settings.supportedAudioFormats'), t('settings.analysisPipeline.decodeAudio'),
etc.), and ensure you import/use the translation hook (useTranslation) at the
top of the component so t is available; keep SUPPORTED_AUDIO_FORMATS as-is but
render its label via i18n.
- Around line 8-48: The SettingsFeature component currently hardcodes UI strings
("Supported Audio Formats", "Analysis Pipeline", list items, "About" paragraph)
and inline style objects; replace hardcoded text with the app i18n pattern used
elsewhere (use detectPreferredLocale() and createTranslator(...) to create a
translator instance and call it for keys like settings.supportedAudioFormats,
settings.analysisPipeline, settings.analysis.steps.decode,
settings.analysis.steps.extractRoles, settings.analysis.steps.separateStems,
settings.analysis.steps.persist, settings.about.description) and add
corresponding entries to the locale resources; also move the inline
style={{...}} objects for the section, headings, lists and badges into CSS/SCSS
classes (or a shared styled component) and apply className to the containing
elements so styling is consistent with WorkspaceStates/SectionRoadmap patterns.
In `@apps/desktop/src/features/workspace/ConfidenceBadge.tsx`:
- Around line 10-11: ConfidenceBadge 현재 컴포넌트가 매 렌더링마다
createTranslator(detectPreferredLocale())를 호출해 불필요한 재생성이 발생하니,
createTranslator(detectPreferredLocale()) 호출을 useMemo로 메모이제이션하거나 상위에서
translator를 props로 받아 사용하도록 변경하세요; 구체적으로 ConfidenceBadge 컴포넌트 내부에서
createTranslator/ detectPreferredLocale을 직접 호출하는 대신 React.useMemo(() =>
createTranslator(detectPreferredLocale()), [/* optional deps: locale */])로 래핑하거나
상위 컴포넌트에서 생성한 translator를 props로 전달받도록 수정해 재렌더링 비용을 줄이세요.
In `@apps/desktop/src/features/workspace/RoleSwitcher.tsx`:
- Line 26: The TabsList has conflicting height utilities ("h-10" and "h-auto")
which breaks the responsive height; update the TabsList className in
RoleSwitcher.tsx to remove the base "h-10" and keep a single default height
(e.g., "h-auto") plus the breakpoint rule "sm:h-10" so it reads like "h-auto
sm:h-10 ..." to ensure correct responsive behavior.
- Around line 22-23: The component uses the string sentinel "all" in the
value/onValueChange logic which can collide with a real role id; replace that
ad-hoc sentinel with a dedicated constant (e.g., ROLE_ALL or ROLE_SENTINEL) or
another collision-free representation and update the value prop and
onValueChange handler accordingly (references: activeRole, onRoleChange, the
value={...} and onValueChange={(val) => onRoleChange(...)} calls) so that real
role ids named "all" are handled correctly by translating between the internal
sentinel and null for external consumers.
In `@apps/desktop/src/features/workspace/SectionRoadmap.tsx`:
- Around line 170-175: The map over role.overlapWarnings in SectionRoadmap.tsx
uses the iteration index (key={wIdx}), which can cause unstable keys when
warnings are added/removed; update the key to a stable unique identifier instead
(e.g., use warning.id if each warning object has an id, or fallback to a
deterministic value like the warning message string or a composite key such as
`${role.id}-${warning}`), and ensure the data structure for overlapWarnings
includes that unique id if necessary so React can correctly preserve list item
identity.
In `@apps/desktop/src/features/workspace/Workspace.tsx`:
- Line 15: Replace the placeholder JSDoc comments ("/* Documented. */") in
Workspace.tsx with meaningful documentation or remove them: update the JSDoc
above the Workspace React component and any functions referenced at those
positions (e.g., lifecycle handlers or helpers like handleExportCueSheet) to
describe purpose, parameters, return values, and side effects, or delete the
placeholder if no doc is needed; ensure each JSDoc block references the exact
function/component name (Workspace, handleExportCueSheet, etc.) so future
readers can understand intent.
In `@apps/desktop/src/features/workspace/WorkspaceStates.tsx`:
- Around line 14-15: 제목이 하드코딩되어 있어 로케일 혼합이 발생하므로 Ready to Analyze(및 파일 내 다른 하드코딩
제목, 예: 해당 파일의 28-29행에 있는 로딩 제목)를 i18n 키로 추출하고 JSX에서 t(...)로 사용하도록 수정하세요; 예를 들어
컴포넌트의 <h3> 텍스트를 t("workspaceReadyHeading")로 바꾸고 로딩 제목은
t("workspaceLoadingHeading") 같은 키를 추가해 locales 파일에 문자열을 추가한 뒤 <h3
className="...">{t("workspaceReadyHeading")}</h3> 방식으로 일관되게 처리하세요.
In `@apps/desktop/src/i18n/index.ts`:
- Around line 4-7: Replace the low-value placeholder comments (/** Documented.
*/) with meaningful one-line doc comments that explain the intent and usage of
the exported types and values: for example, document that Locale is the set of
supported locale codes ("en" | "ko"), that TranslationKey represents keys of the
enCommon translation object (keyof typeof enCommon), and similarly replace the
other placeholder comments for any other exported types/functions in this file
with concise descriptions of their purpose and usage so maintainers can
understand them at a glance.
In `@apps/desktop/src/index.css`:
- Around line 6-8: biome의 CSS 파서가 Tailwind v4 문법(`@custom-variant`, `@theme` inline,
`@apply` 등)을 인식하지 못해 린트 오류가 발생하므로 biome.json의 css.parser 설정에 tailwindDirectives:
true를 추가해 Tailwind 지시문을 활성화하고 필요하면 cssModules 설정을 유지하세요; 이 변경은 해당 파일의 css.parser
블록(현재 없음 또는 기존 설정)에 적용해야 합니다.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 35-36: Replace the placeholder JSDoc comments with meaningful
documentation: for the exported type LocalAudioSelectionResult, add a short
description of what the type represents, list and explain each property (their
types and when they are present), and describe expected usage/return semantics;
do the same for the other exported type referenced around lines 40-41 (replace
its /** Documented. */ with a description of purpose, fields and any
nullable/optional behavior). Locate these by the type names
LocalAudioSelectionResult and the adjacent exported type and update their JSDoc
to clearly describe intent, properties, and consumer expectations.
In `@apps/desktop/src/lib/export.ts`:
- Around line 16-25: The CSV formula-injection check only looks at the very
first character (using /^[=+\-@]/) and misses cases where leading whitespace or
control characters precede an operator; update the detection to test the first
non-control/non-whitespace character instead (e.g. use a regex like
/^[\s\x00-\x1F]*[=+\-@]/) and, when it matches, prefix the original escapedValue
with a single quote (keep all original leading whitespace/control chars intact),
i.e. modify the block that references escapedValue and the /^[=+\-@]/ test so it
checks the new regex and still returns the value with the leading single-quote
added.
In `@apps/desktop/vite.config.ts`:
- Around line 4-10: The config uses Node's __dirname in the Vite config
(path.resolve(__dirname, "./src")) which fails under ESM; replace the __dirname
usage with an import.meta.url-derived directory: compute the file dir from
import.meta.url (via fileURLToPath + path.dirname or by resolving new
URL('./src', import.meta.url)) and update the alias value (the "@" alias) to use
that computed path; update any references in vite.config.ts around defineConfig,
resolve.alias, and the path.resolve call accordingly.
In `@CHANGELOG.md`:
- Around line 1-13: The top-level headings violate Markdown lint rules: replace
the initial H2 "## [Unreleased]" or move it below a proper H1 "Changelog" so the
file begins with a single H1, and ensure every heading (e.g., "## [Unreleased]",
"## [0.1.1] - 2026-04-28" and the "### Added"/"### Fixed" sections) is followed
by a blank line to satisfy MD022; update the heading order and insert single
blank lines immediately after each heading shown in the diff so MD041 and MD022
are resolved.
In `@docs/plans/2026-03-28-ml-engine-integration.md`:
- Line 3: 파일의 모든 Markdown 섹션 헤더(예: "## Overview" 및 코멘트에 언급된 다른 헤더들) 앞뒤에 반드시 한 줄의
빈 줄을 추가해 markdownlint MD022 경고를 제거하세요; 구체적으로 각 헤더 줄 바로 위에 빈 줄을 삽입하고 헤더 다음 줄도 빈
줄로 분리하여 문단 또는 다음 헤더와 붙지 않도록 수정하세요.
In `@docs/plans/2026-04-28-pr-159-rollout.md`:
- Around line 153-160: Update the "Trust Boundary" and "Mitigations" sections to
not treat GitHub Releases as inherently trusted: explicitly mark downloaded
artifacts (DMG/EXE) as untrusted until signature/checksum validation succeeds,
add verification steps (code-signature and checksum validation) and abort
installation on validation failure with safe-failure behavior; expand
"Mitigations" to include verification procedures and logging of verification
outcomes and privacy/impact notes; add a new "Security Notes" subsection
documenting untrusted inputs, trust boundaries, allowlists/validation rules,
safe-failure behavior, logging/privacy impact, and concrete test points to
validate signature/checksum failures and successful validation flows (reference
the "Trust Boundary", "Mitigations", and new "Security Notes" headings to locate
where to add these).
In `@docs/security/dependency-policy.md`:
- Line 105: The current blanket exception sentence grouping multiple advisories
(e.g. RUSTSEC-2024-0413, RUSTSEC-2024-0429, RUSTSEC-2025-0057) must be replaced
with per-advisory entries: for each advisory listed, create a separate bullet or
subsection that documents the advisory ID, scope (how/where the vulnerable crate
is inherited via Tauri v2 wry/webkit2gtk), exposure (impact to our product),
compensating controls (mitigations or runtime protections), and the
removal/reevaluation condition (what must change to revoke the exception);
remove the phrase “ignored by default” and instead state the explicit exception
status per advisory so each RUSTSEC entry is auditable and self-contained.
In `@packages/shared-types/src/index.ts`:
- Around line 921-929: 현재 parseRehearsalSong 검사에서 partGraph의 타입만 확인하고 있어
sections에 정의되지 않은 role을 참조해도 통과하므로, value.sections에서 모든 roles[].id를 모아 Set으로 만들고
중복된 role id가 있으면 invalidField(`${path}.sections`)을 반환하도록 추가하세요; 그 후 기존 루프(참조:
partGraph, validatePartGraphNode)에서 각 node에 대해 node.role_id가 id 집합에 존재하는지, 그리고
node.handoff_to/node.handoff_from가 있는 경우에도 해당 id들이 집합에 속하는지 확인해 유효하지 않으면 각각
invalidField(`${path}.partGraph[${index}].role_id`) 또는 적절한 하위 경로로 오류를 반환하도록
수정하세요.
In `@scripts/release/package_desktop_artifact.py`:
- Around line 111-125: artifact_identity currently produces identical
archive/manifest names for installers that share extensions, and the
replace(ext, f"{ext}") is a no-op; to fix, incorporate a unique identifier from
each installer (e.g., installer_path.stem or installer_path.parent.name) into
archive_name and identity["manifest_name"] when len(installers) > 1 so names do
not collide: after calling artifact_identity(installer_path.name) modify
archive_name (and identity["manifest_name"] accordingly) to append or prepend
the installer stem or bundle subdir, then use that adjusted archive_name for
shutil.copy2, checksum generation (sha256_file(archive_path)), and writing the
manifest path (manifest_path) so each installer in the same run gets a distinct
archive and manifest.
- Around line 110-120: The current loop copies installer_path entries directly
which can follow symlinks and export files outside the intended bundle; before
copying, ensure each installer is a regular file inside the expected bundle by
resolving paths and validating containment: compute bundle_dir_resolved =
bundle_dir.resolve() and installer_resolved = installer_path.resolve(); check
installer_resolved.is_file() and that installer_resolved is located within
bundle_dir_resolved (e.g., by confirming bundle_dir_resolved in
installer_resolved.parents or by comparing string prefixes), and only then
perform the copy (use installer_resolved as the source if you must avoid
following symlinks); reference variables/functions: find_installer_packages
(producer of installers), installers, installer_path, artifact_identity,
bundle_dir, output_dir, and shutil.copy2 to locate where to add these checks.
In `@SECURITY.md`:
- Line 10: Replace the placeholder reporting email 'seonghobae@example.com' in
the SECURITY.md line with a real, monitored security contact address (e.g.,
security@yourdomain.com) or remove the email entirely and rely solely on the
existing GitHub private vulnerability report link; update the line that
currently contains the email string to use the chosen real/contact method and
ensure the message text still clearly invites private vulnerability reports.
- Line 11: Replace the phrase "90 days expectation" with the grammatically
correct hyphenated compound adjective "90-day expectation" in the sentence shown
in the diff; locate the exact sentence in SECURITY.md (the line containing "We
expect vulnerability disclosure timelines to follow coordinated practices,
generally providing a 90 days expectation to fix before public disclosure.") and
update it to use "90-day expectation".
- Around line 10-11: The SECURITY.md change is out-of-scope for the UI redesign
PR; remove or revert the SECURITY.md edits from this branch/commit and create a
separate branch+PR containing only the security policy update (the lines
updating the vulnerability reporting email and disclosure timeline). Ensure the
workspace redesign PR (the Tailwind v4 / shadcn UI changes) contains only
UI-related files and reference the new security-PR number in its description if
needed.
In `@services/analysis-engine/src/bandscope_analysis/chords/analyzer.py`:
- Around line 67-78: The current dedupe using seen_chords drops later entries so
a 'user' source arriving after a 'model' one never upgrades the stored entry;
change the loop that processes section_roles/harmony/chord_name so that if
chord_name already exists you locate the existing entry in chords (by matching
"chord") and merge/upgrade it instead of skipping: if the incoming
harmony.get("source") == "user" and the existing entry's "source" != "user", set
existing["source"]="user" (also update existing["functionLabel"] to the incoming
non-empty functionLabel if present); otherwise leave the earlier entry intact;
keep using seen_chords to avoid duplicates but perform this merge/upgrade when
duplicates are found.
In `@services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py`:
- Around line 85-86: Wrap the call to librosa.decompose.nn_filter that assigns
chromagram in a try/except around the nn_filter call (referencing chromagram and
librosa.decompose.nn_filter) to catch any exceptions, log or handle the error,
and fall back to the original chromagram (or a safe alternative) so downstream
code (e.g., chord recognizer) still receives a valid chromagram; ensure the
except block is narrow (catch Exception) and includes context in the log message
to aid debugging.
- Around line 118-129: The hardcoded thresholds (0.3, 0.01, 0.02) used when
deciding unvoiced/noise in chord_recognizer.py should be extracted to
class-level constants on ChordRecognizer (e.g., SIMILARITY_THRESHOLD,
RMS_THRESHOLD, CHROMA_VARIANCE_THRESHOLD) and referenced instead of literals;
update the block that computes max_sim, rms_val, and chroma_var to compare
against these constants (use the class name or self constants depending on
context) and remove the magic numbers so tuning and readability improve.
In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 86-92: Do not log raw local paths or full exception text from
TemporalAnalyzer.analyze; instead sanitize and replace audio_path and exception
details with a safe identifier (e.g., job_id) and minimal context. Update the
try/except around TemporalAnalyzer() and analyze() so logging.info and
logging.warning do not include audio_path or str(e); log a short message like
"Temporal analysis failed for job_id=<job_id>, using fallback" and include any
non-sensitive error category (e.g., "ValueError" or a custom error code) if
needed. Ensure references to TemporalAnalyzer, analyze, audio_path and the
logging calls are changed consistently and avoid emitting user filesystem or
internal stack/exception strings. Ensure job_id is passed into this scope or
generated earlier and used in the sanitized logs.
- Around line 79-85: The branch that handles requests with "sourceKind" ==
"local_audio" accesses request["localSource"].get("sourcePath") without
verifying localSource is a dict, causing AttributeError on malformed inputs;
update the CLI code in services/analysis-engine/src/bandscope_analysis/cli.py to
first validate that request["localSource"] is a dict (e.g.,
isinstance(request.get("localSource"), dict)) before calling .get, and also
optionally validate the resulting audio_path is a string; if the type checks
fail, return/raise the same structured invalid-request response used elsewhere
instead of letting the exception bubble (references: the request dict,
"localSource", the audio_path variable and the local_audio handling branch).
In `@services/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.py`:
- Around line 69-78: avg_prob is being computed over all voiced_probs including
unvoiced/NaN frames which can lower the mean if silence surrounds voiced
segments; update the calculation in pitch_tracker.py to apply the same
voiced_flag mask used for voiced_f0 (i.e., compute avg_prob =
mean(voiced_probs[voiced_flag] or filtered non-NaN entries) and only use those
values for the confidence ("high" if >0.6 else "low") and the low-confidence
early return (avg_prob < 0.2). Ensure you reference and use the existing
voiced_flag, voiced_probs and voiced_f0 variables so the avg_prob and subsequent
return logic align with the actual voiced frames.
In `@services/analysis-engine/src/bandscope_analysis/roles/extractor.py`:
- Around line 67-79: The current logic overwrites the default vocal_range and
bass_range with empty-string bounds when PitchTracker.track() returns a truthy
p_res that lacks one of lowest_note or highest_note; change the update so you
only replace vocal_range and bass_range when both p_res["lowest_note"] and
p_res["highest_note"] are present (non-empty), otherwise leave the existing
default range intact — update the blocks that call
pitch_tracker.track(stems["vocal"], sr=sr) and
pitch_tracker.track(stems["bass"], sr=sr) to check both bounds on p_res before
assigning to vocal_range and bass_range.
In `@services/analysis-engine/src/bandscope_analysis/separation/model.py`:
- Line 24: The field `category` in the model currently typed as `str` must be
changed to use the `StemCategory` enum to preserve contract and model integrity;
update the declaration of `category` in
services/analysis-engine/src/bandscope_analysis/separation/model.py to type
`StemCategory`, add/import the `StemCategory` enum at the top of the file, and
adjust any serialization/deserialization or validation code that assumes a
string (e.g., constructors, pydantic/schema conversions, and tests) so they
accept/produce the enum values rather than plain strings.
In `@services/analysis-engine/src/bandscope_analysis/separation/separator.py`:
- Around line 87-99: Normalize role id and type before deduping and
categorization: call strip() on role.get("id", ...) and if the result is empty
use the fallback f"role-{i}" when constructing role_id and adding to seen_ids;
for roleType call strip().lower() and pass that normalized value into
_categorize_role (and use it when computing confidence), and ensure role_name is
also trimmed via strip() before use. Update the code paths referencing
role.get("id"), role.get("name"), role.get("roleType"), the local variables
role_id, role_name, role_type, the call to _categorize_role, and the confidence
assignment so all use the normalized values.
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py`:
- Around line 83-85: The current except Exception block in the temporal analysis
code logs and wraps every error as ValueError; change it so FileNotFoundError
and PermissionError are re-raised (preserve original exceptions) while only
other exceptions are logged and wrapped into ValueError; update the except
clause(s) around the analysis call in analyzer.py (the block that currently does
logger.error(f"Failed to analyze audio {path_str}: {e}") and raise
ValueError(...)) to explicitly catch and re-raise FileNotFoundError and
PermissionError, and have a separate broad except to log and raise ValueError
for all other exceptions.
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Around line 48-49: Replace repetitive placeholder docstrings that just restate
the test function names with descriptive sentences explaining each test's
intent; for example update test functions like
test_chord_recognizer_hpss_exception, test_chord_recognizer_harmonic_exception,
test_chord_recognizer_onset_exception (and the other tests listed) so each
docstring describes the behavior being validated (e.g., "Verify recognizer
gracefully handles HPSS extraction failures.") instead of repeating the function
name—locate the test functions by their names and modify their triple-quoted
docstrings accordingly.
- Line 24: Remove the leftover debug print statement print("RESULT:", result)
from the test (tests/test_chord_recognizer.py); delete that line (or replace
with a proper logger.debug if persistent output is needed) so CI logs aren't
polluted, then run the test suite to confirm no behavior changed.
In `@services/analysis-engine/tests/test_pitch_tracker.py`:
- Around line 19-27: The test test_pitch_tracker_unvoiced_audio is flaky because
it uses uncontrolled np.random.randn() noise and asserts None for
lowest_note/highest_note; either set a fixed seed before generating noise (e.g.,
call np.random.seed(...) in the test) to make the noise deterministic, or better
mock the pitch extractor used inside PitchTracker.track (e.g., patch
librosa.pyin or the internal method that returns f0/voiced_flag) to return an f0
array of all NaNs (or voiced flags all False) so the tracker deterministically
produces lowest_note/highest_note = None and confidence = "low". Ensure you
reference PitchTracker.track when applying the mock/seed so the test behaves
reliably.
In `@services/analysis-engine/tests/test_priority.py`:
- Around line 11-16: Multiple tests repeat the same "role" dict literal causing
drift; extract a helper factory like a function (e.g., make_role or
role_factory) that returns the common base dict with "confidence",
"overlapWarnings", "manualOverrides", "setupNote" and allow per-test overrides
(kwargs or merge) so tests only supply changed fields; replace the repeated role
assignments in test cases (the variables named role) with calls to this helper
to reduce duplication and make maintenance easier.
- Line 17: Remove the unsafe cast(Any, role) from the assertion and replace the
repeated role dict literals with a reusable pytest fixture (e.g.,
rehearsal_role_fixture) that returns a concrete RehearsalRole TypedDict instance
containing all required keys (id, name, roleType, harmony, cue, range,
rehearsalPriority); update tests to import/use that fixture and pass its value
into calculate_rehearsal_priority instead of the ad-hoc dicts so type checking
catches schema changes and eliminates the cast bypass.
In `@services/analysis-engine/tests/test_release_packaging.py`:
- Around line 83-84: The test asserts a strict list order for installers which
can be flaky; change the assertion in test_release_packaging.py to compare as
sets (or use unordered comparison) so that installers returned by
packaging.find_installer_packages is compared to {dmg_path} (or use
set(installers) == {dmg_path}); update the assertion referencing installers,
packaging.find_installer_packages, and dmg_path accordingly to make the test
order-independent.
🪄 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: cb891b22-f331-404d-b126-a5893ca7179f
⛔ Files ignored due to path filters (3)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (94)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.yml.github/workflows/security-audit.yml.github/workflows/trivy.yml.gitignoreCHANGELOG.mdREADME.mdSECURITY.mdVERSIONapps/desktop/components.jsonapps/desktop/package.jsonapps/desktop/src-tauri/.cargo/audit.tomlapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/components/ui/badge.tsxapps/desktop/src/components/ui/button.tsxapps/desktop/src/components/ui/card.tsxapps/desktop/src/components/ui/input.tsxapps/desktop/src/components/ui/progress.tsxapps/desktop/src/components/ui/scroll-area.tsxapps/desktop/src/components/ui/separator.tsxapps/desktop/src/components/ui/tabs.tsxapps/desktop/src/features/chords/index.tsxapps/desktop/src/features/home/index.tsxapps/desktop/src/features/player/index.tsxapps/desktop/src/features/ranges/index.tsxapps/desktop/src/features/settings/index.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/i18n/index.tsapps/desktop/src/index.cssapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/export.test.tsapps/desktop/src/lib/export.tsapps/desktop/src/lib/utils.tsapps/desktop/src/main.tsxapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsdocs/plans/2026-03-28-ml-engine-integration.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/security/dependency-policy.mdeslint.config.jspackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/chords/__init__.pyservices/analysis-engine/src/bandscope_analysis/chords/analyzer.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/chords/model.pyservices/analysis-engine/src/bandscope_analysis/cli.pyservices/analysis-engine/src/bandscope_analysis/ranges/__init__.pyservices/analysis-engine/src/bandscope_analysis/ranges/analyzer.pyservices/analysis-engine/src/bandscope_analysis/ranges/model.pyservices/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.pyservices/analysis-engine/src/bandscope_analysis/roles/extractor.pyservices/analysis-engine/src/bandscope_analysis/separation/__init__.pyservices/analysis-engine/src/bandscope_analysis/separation/model.pyservices/analysis-engine/src/bandscope_analysis/separation/separator.pyservices/analysis-engine/src/bandscope_analysis/temporal/__init__.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/src/bandscope_analysis/temporal/model.pyservices/analysis-engine/src/bandscope_analysis/youtube.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_pitch_tracker.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_ranges.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_separation.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.pyservices/analysis-engine/tests/test_youtube.py
💤 Files with no reviewable changes (3)
- scripts/checks/verify_security_notes.py
- scripts/checks/verify_github_bootstrap_policy.py
- scripts/checks/verify_docs.py
✅ Actions performedComments resolved and changes approved. |
This was referenced Apr 29, 2026
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.
Refactors the raw inline styles to use Tailwind CSS v4 and Shadcn UI components for a more modern and readable desktop application UI.