Skip to content

feat(workspace): name tonight's first drop plan on the map - #1040

Open
seonghobae wants to merge 27 commits into
developfrom
feat/workspace-first-drop-plan
Open

feat(workspace): name tonight's first drop plan on the map#1040
seonghobae wants to merge 27 commits into
developfrom
feat/workspace-first-drop-plan

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Buyer-visible next action

This PR names tonight's first drop plan on the rehearsal map when an entering part lands a full-band arrival after a thin texture: previous graph 1–2 distinct sources, current graph ≥3 sources, previous sources stay, and a new named entrance arrives. Open scrolls that rendered map section so the landing part can come in together when the texture fills.

A drop is a corroborated density fill that arrives. Breakdown is a staying sparse hold. Dropout is a leaving part. Cutoff is a stop-time. Pickup is rest-then-enter. Heuristic-only topology stays unnamed.

Summary

  • Engine emits Hit this drop with {target}; come in together when the texture fills. (or the solo fill) only from real stem activity that proves the thin-to-full arrival. Shared other stems may corroborate density but never own the landing.
  • Fail-closed own-data / Proxy-safe resolver admits bounded dropPlan copy only from owned data properties and snapshots it once before ranking. Inherited, accessor-backed, or Proxy-substituted runtime metadata remains guidance-only.
  • i18n EN/KO next-action copy is particle-safe. Korean Open is {at} {role} 드롭 열기.
  • Navigation uses the current song-structure renderer (data-section-index) and fails closed on ambiguous or missing targets. Reduced motion uses behavior: "auto".
  • Demo and heuristic topology stay unnamed until real stem activity corroborates the fill.

Exact current identity

Verification

  • Desktop vitest for drop-plan resolver, callout, i18n, and workspace armed-state
  • shared-types dropPlan provenance tests
  • Python tests/test_drop_plan.py plus extractor/roles/activity
  • Rust drop_plan_contract round-trip
  • ESLint + tsc --noEmit for desktop and shared-types
  • ./scripts/harness/quickcheck.sh (CI)

Security Notes

Attack surface

Untrusted rehearsal-song JSON, Proxy/inherited role metadata, and rendered-map DOM selectors used for Open navigation.

Trust boundary

UI resolver reads only own data properties (Object.getOwnPropertyDescriptor) and fails closed. Engine drop copy is emitted only from corroborated stem activity. Native payload admits optional dropPlan / dropPlanSource with deny_unknown_fields.

Mitigations

Bounded copy (180 code points), reject multiline/blank plans, reject provenance without copy, snapshot guidance before ranking, fail closed on ambiguous song-structure-grid mounts, preserve user copy verbatim unless model provenance is explicit.

Test points

Hostile Proxy get, inherited metadata, malformed song root, Korean particle-safe copy, navigation failure, reduced motion, demo unnamed, 2→3 solo fill, two named partners, other-stem unnamed, density-drop-is-not-drop, live stem extract.

Dependency and Supply Chain

  • No new direct dependency was added
  • If a new dependency was added, this PR explains why it is needed
  • runtime / dev / build / test classification is recorded
  • alternatives were considered
  • maintainer trust and update health were checked
  • license fit was checked
  • known security issues were checked
  • transitive footprint impact was considered
  • SBOM or supplemental inventory impact was recorded (unchanged)

Canonical #783 is protected develop shipped truth. This branch inherits that JavaScript baseline and does not duplicate or suppress it. Inherited npm HIGH must not be suppressed elsewhere.

i18n impact

  • No user-visible string changed
  • Korean and English locale impact was updated

Reviewer checklist

  • Gitflow target branch is develop
  • protected-branch rules were not weakened
  • required checks are expected to stay green

Keep unmerged until the unchanged then-current head has every applicable repository and central CI/build/release/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, zero valid unresolved findings, and a qualifying independent non-author last-push approval under live branch protection.

Queued, pending, skipped, cancelled, failed, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.


Devin Review

Summary by CodeRabbit

  • 새로운 기능

    • 리허설 워크스페이스에 “오늘 밤의 첫 드롭 계획” 안내를 추가했습니다.
    • 관련 파트와 진입 시점을 확인하고, 버튼으로 곡 맵의 해당 구간으로 이동할 수 있습니다.
    • 충분히 확인된 스템 활동이 있을 때만 안내를 표시합니다.
    • 사용자 작성 안내를 보존하며 영어와 한국어를 지원합니다.
  • 개선 사항

    • 드롭 계획 정보의 저장 및 불러오기를 지원합니다.
    • 이동 효과를 동작 감지 설정에 맞추고, 이동 실패 시 상태 메시지를 표시합니다.

Name the earliest corroborated full-band arrival after a thin texture
so the entering part can land together when the map fills. Heuristic
demo topology stays unnamed until real stem activity proves the drop.
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 27 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2abd136c-dcda-49bd-b864-7ab884dae13c

📥 Commits

Reviewing files that changed from the base of the PR and between bfd8c02 and 0fe3cf7.

📒 Files selected for processing (3)
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.test.tsx
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.tsx
  • services/analysis-engine/src/bandscope_analysis/roles/extractor.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 685b40fb-d6b8-4331-a30d-b0addcd37733

📥 Commits

Reviewing files that changed from the base of the PR and between fa27cdf and bfd8c02.

📒 Files selected for processing (13)
  • apps/desktop/core/src/lib.rs
  • apps/desktop/core/tests/drop_plan_contract.rs
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.custom-guidance.test.tsx
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.particle.test.tsx
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.provenance.test.tsx
  • apps/desktop/src/features/workspace/firstDropPlan.accompaniment-provenance.test.ts
  • apps/desktop/src/features/workspace/firstDropPlan.model-guidance.test.ts
  • apps/desktop/src/features/workspace/firstDropPlan.source-continuity.test.ts
  • apps/desktop/src/features/workspace/firstDropPlan.ts
  • apps/desktop/src/i18n/index.test.ts
  • apps/desktop/src/locales/ko/common.json
  • services/analysis-engine/src/bandscope_analysis/roles/extractor.py
  • services/analysis-engine/tests/test_drop_plan.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.particle.test.tsx
  • apps/desktop/src/locales/ko/common.json
  • apps/desktop/src/features/workspace/FirstDropPlanCallout.provenance.test.tsx

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

분석 엔진이 검증된 스템 활동에서 첫 드롭 계획을 생성합니다. 데스크톱 워크스페이스는 계획을 현지화해 표시하고, 해당 곡 맵 섹션으로 이동합니다. 데이터 계약과 문서도 갱신되었습니다.

Changes

첫 드롭 계획

Layer / File(s) Summary
드롭 계획 데이터 계약
packages/shared-types/src/index.ts, apps/desktop/core/src/lib.rs, apps/desktop/src/i18n/*, apps/desktop/src/locales/*, packages/shared-types/test/*, apps/desktop/core/tests/*
역할과 프로젝트 payload가 dropPlan 및 provenance 필드를 지원합니다. 섹션 라벨 번역과 계약 검증 테스트가 추가되었습니다.
분석 엔진의 드롭 계획 생성
services/analysis-engine/src/bandscope_analysis/roles/*, services/analysis-engine/tests/test_drop_plan.py
이전 섹션의 source 활동을 사용해 확인된 density fill을 찾습니다. 조건을 충족한 새 역할에만 model 출처의 계획을 부여합니다.
첫 드롭 해석 및 선택
apps/desktop/src/features/workspace/firstDropPlan.ts, apps/desktop/src/features/workspace/firstDropPlan*.test.ts
시간, 역할, 그래프, source, 소유 데이터 속성을 검증합니다. 유효한 후보 중 가장 이른 계획을 안정적으로 선택합니다.
워크스페이스 표시와 섹션 탐색
apps/desktop/src/features/workspace/FirstDropPlanCallout*, apps/desktop/src/features/workspace/Workspace*
callout이 provenance별 문구를 표시합니다. 일반 및 reduced-motion 스크롤, 탐색 실패, 상태 초기화와 다중 마운트 식별자를 처리합니다.
기능 규칙 문서화
AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md
첫 드롭 계획의 생성 조건, 표시 동작 및 다른 first-* 계획과의 구분을 문서화합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🔵 Low · up to bfd8c

The PR adds buyer-visible drop guidance and navigation, but excluding drum activity from density calculations can cause some valid drop plans to be omitted. The change is mergeable with explicit owner awareness or follow-up on that bounded detection gap.

Sequence Diagram(s)

sequenceDiagram
  participant StemActivity
  participant AnalysisEngine
  participant Workspace
  participant SongMap
  StemActivity->>AnalysisEngine: 이전 및 현재 섹션 활동 제공
  AnalysisEngine->>AnalysisEngine: 확인된 density fill에서 첫 드롭 계획 선택
  AnalysisEngine-->>Workspace: dropPlan과 dropPlanSource 전달
  Workspace->>Workspace: 계획 문구 현지화
  Workspace->>SongMap: 일치하는 섹션으로 스크롤
  SongMap-->>Workspace: 열린 계획 상태 표시
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 워크스페이스 지도에 오늘 밤의 첫 드롭 계획을 명명하는 핵심 변경 사항을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed Docstring coverage is 80.82% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 73 functions across 28 files. (1 skipped: 1…
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 80.82% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 73 functions across 28 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/workspace-first-drop-plan

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.

github-code-quality[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

Strix exact-head handoff — no BandScope-local source repair is valid here.

  • PR head: ed677cbd09ffacf8c751eeb7f42cff329f84db8c
  • Strix run/job: 33071209195 / 98518643278
  • First causal boundary is provider availability, before an authoritative Strix report can be produced: primary NVIDIA NIM exhausted bounded retries on HTTP 429; NVIDIA fallback returned provider/model 404; openrouter/free exhausted the existing bounded HTTP 502 retry path; direct OpenAI then failed with HTTP 429 insufficient_quota / credit balance exhausted.
  • A transient attempt printed Vulnerabilities 0, but no authoritative report artifact was produced, so that line is not success evidence and the gate correctly failed closed.
  • Current central .github/main@17052a7ca3c16db90932a4d6036b43165ddee418 already classifies bounded LiteLLM/OpenRouter APIError 502 + metadata.provider_name as transient and retries it. Therefore weakening/retrying around Strix in BandScope or adding a leaf fallback would duplicate foreign authority rather than fix the cause.

Acceptance for this lane: rerun the unchanged then-current BandScope head only when at least one contracted provider can complete the assessment and emit the authoritative report; do not transfer predecessor, zero-vulnerability log text, neutral, or partial-provider evidence. Provider credit/capacity ownership is central/operational (see ContextualWisdomLab/.github#624), not this product branch.

@devin-ai-integration devin-ai-integration 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.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 new potential issues.

Devin Review

Comment on lines 200 to +204
overlap_warnings: Vec<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
drop_plan: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
drop_plan_source: Option<DropPlanSourcePayload>,

@devin-ai-integration devin-ai-integration Bot Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Native role payload omits optional shared-types fields

RehearsalRolePayload uses deny_unknown_fields and adds drop_plan/drop_plan_source, but still lacks harmonicExplanation, transpositionPlan, transcription, and practiceProgress from the shared-types role. A persisted project carrying any of those is rejected by project_payload_from_content. Pre-existing, but this PR extends the same struct.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread apps/desktop/src/features/workspace/FirstDropPlanCallout.tsx

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Repair the exact current-head Ruff formatter failure on the existing feat/workspace-first-drop-plan branch only. Current head c0431582cbf38b2f81e54714ac3ca98dda96f91a; CI run 33089519544, job 98578395957, actual merge checkout 5bf9aa15364b8c265bd94a3fb639c112fd83940d proves every quickcheck stage through ruff check passes and uv run ruff format --check src tests fails only because services/analysis-engine/src/bandscope_analysis/roles/extractor.py would be reformatted. Two manual formatting hypotheses have already been falsified by hosted Ruff 0.15.5, so do not guess or alter semantics. Run the repository-pinned formatter itself (uv run ruff format src/bandscope_analysis/roles/extractor.py) on this exact branch, inspect the formatter delta, require that it is formatting-only and preserves the drop-plan source/provenance behavior and tests, then run focused uv run ruff check src/bandscope_analysis/roles/extractor.py plus uv run ruff format --check src/bandscope_analysis/roles/extractor.py and the canonical quickcheck. Commit only the exact formatter output to this same branch; do not touch gates, dependencies, or foreign repositories. Report the resulting exact head and evidence.

@devin-ai-integration devin-ai-integration 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.

Devin Review found 2 new potential issues.

Devin Review

Comment on lines +385 to +405
const landingRole = pickLandingRole(
rankedActiveRoles(section as RehearsalSection).flatMap((metadata) => {
if (
previousActiveIds.has(metadata.id) ||
ACCOMPANIMENT_SOURCE_ROLE_IDS.has(metadata.id)
) {
return [];
}
const dropPlan = ownedDropPlan(metadata.role);
return dropPlan === null
? []
: [
{
...metadata,
dropPlan: dropPlan.text,
dropPlanSource: dropPlan.source,
dropPlanGuidance: dropPlan.guidance
}
];
})
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: User-authored drop plans surface only inside a detected density fill

resolveSafeFirstDropPlan gates every drop plan (including user-source and source-less ones) behind the full density-fill detection. A user who authors a dropPlan on a role in a section that is not a corroborated drop never sees it rendered. This is consistent with the fail-closed design, but means user guidance can silently disappear.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +1595 to +1606
if (value.dropPlan !== undefined && typeof value.dropPlan !== "string") {
return invalidField(`${path}.dropPlan`);
}
if (
value.dropPlanSource !== undefined &&
!isOneOf(PROVENANCE_SOURCES, value.dropPlanSource)
) {
return invalidField(`${path}.dropPlanSource`);
}
if (value.dropPlanSource !== undefined && value.dropPlan === undefined) {
return invalidField(`${path}.dropPlanSource`);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Empty dropPlan with a source persists but never renders

Both the shared-types validateRehearsalRole and the Rust validate_drop_plan_provenance accept an empty-string dropPlan paired with a source, since they only reject the source-without-plan case. ownedDropPlan trims and drops zero-length text, so the record round-trips but never displays.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Work only on the existing feat/workspace-first-drop-plan branch at exact current head 0fe3cf781362e04a4bc81ea226c58a80c107b7e6; refetch the branch immediately before writing and abort/adapt if it moved. Apply receiving-code-review + TDD + verification-before-completion. Current unresolved Devin thread PRRT_kwDORjvEXs6c4f7s is valid: persisted contract accepts blank/whitespace dropPlan paired with dropPlanSource, but ownedDropPlan trims and refuses it, so data can round-trip yet never become buyer-visible. PR security text also states blank/multiline plans are rejected. Do not weaken the UI guard.

Repair the persisted contract at its owning BandScope layers, smallest scope only:

  1. RED first: strengthen packages/shared-types/test/dropPlanProvenance.test.ts and apps/desktop/core/tests/drop_plan_contract.rs so dropPlanSource: "model" with dropPlan equal to "", whitespace-only, LF-containing, or CR-containing copy is rejected. Preserve the existing valid model/user, unknown-source, and source-without-copy cases.
  2. Production fix: make validateRehearsalRole in packages/shared-types/src/index.ts reject those invisible/multiline strings, and make native validate_drop_plan_provenance in apps/desktop/core/src/lib.rs enforce the same invariant. Keep DropPlanSourcePayload enum validation, deny_unknown_fields, the source-without-plan failure, and all unrelated shared optional-field behavior unchanged; do not fold the separate pre-existing harmonicExplanation/transpositionPlan/transcription/practiceProgress native-contract finding into this repair.
  3. GREEN proof: run focused shared-types provenance tests and Rust drop_plan_contract, then the repository canonical quickcheck/full applicable tests. Require exact 100% owned coverage where exposed and no formatting/lint regressions.
  4. Resolve only PRRT_kwDORjvEXs6c4f7s after the new exact head has focused/full GREEN evidence. Do not resolve the separate optional-shared-fields thread or the intentional density-gating informational thread. Do not self-approve, force-push, weaken gates, alter dependencies, or touch foreign repositories.

Return/record the successor exact head and test evidence on this PR.

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.

1 participant