Skip to content

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

Open
seonghobae wants to merge 23 commits into
developfrom
feat/workspace-first-accelerando-plan
Open

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

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Name tonight's first accelerando plan on the mounted rehearsal map so the vocal or bass that lifts into a faster tempo can open that landing.

  • Engine emits accelerandoPlan only when existing tempo-stability reports a sustained speeding (to_bpm > from_bpm) that is not a double-time (~1.9-2.1) or half-time (~0.5) feel flip.
  • The owned copy lands on the highest-priority active named vocal or bass in the section that contains the change. Vocal is preferred on priority ties. Accompaniment never owns. Heuristic/demo topology stays unnamed.
  • Workspace callout opens the matching rendered map section. Fail-closed resolvers ignore inherited, accessor, or Proxy metadata. Customer copy names the next action in EN (Tonight's first accelerando plan) and KO (firstAccelerandoPlanLabel).
  • This is not a new MIR product. test(analysis): govern real YouTube known-stem benchmark #828 owns [Product Gap] Add real-audio MIR accuracy acceptance benchmarks #770. Beat times reuse existing librosa.beat.beat_track / analyze_tempo_stability.

Protected base: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
Exact current head: 90feef25e17745f705680cf9450d038fe6a9f96d.

Do not mix with #811, #828, or #897.

Test plan

  • Python accelerando tests (18) at 100% module coverage
  • Rust accelerando-plan contract tests (5)
  • Vitest resolver, i18n, provenance, callout, particle, and workspace armed-state tests

Follow-up exact-head evidence at 90feef2: shared-types 48 tests with 100% statements, branches, functions, and lines; accelerando UI 27 tests; Rust core and accelerando contract 26 tests; Python accelerando/temporal targeted suite 55 tests; shared/desktop lint and typecheck; Rust format check.

Security Notes

  • accelerandoPlan is untrusted persisted/model-owned copy. The shared, UI, and native boundaries reject empty or Unicode multiline text and require explicit model or user provenance.
  • User-owned guidance is preserved verbatim; only model-owned structured tempo guidance is interpreted. No new filesystem, network, subprocess, IPC, or dependency path is introduced.
  • Map navigation fails closed when the local renderer or target is absent or ambiguous.

Do not self-approve. Do not wait on checks.

Latest exact-head verification

  • Base: 749511c
  • Head: e202f5d
  • Local evidence: Python 705 passed and 24 skipped with 100% statements/branches/functions/lines; API/CLI targeted tests; Ruff; mypy.

Security Notes

  • Temporal cache stores validated BPM/beat/downbeat/duration/sample-rate metadata only; the source path is excluded.
  • Full-analysis and temporal-feature cache hits skip repeat local audio decode/beat tracking; malformed cache metadata fails closed.

Latest exact-head verification

  • Base: 749511c
  • Head: e64bd97
  • Local evidence: Python 705 passed and 24 skipped with 100% statements/branches/functions/lines; API/CLI targeted tests; Ruff format/check; mypy.

Security Notes

  • Temporal cache stores validated BPM/beat/downbeat/duration/sample-rate metadata only; the source path is excluded.
  • Full-analysis and temporal-feature cache hits skip repeat local audio decode/beat tracking; malformed cache metadata fails closed.

Latest exact-head verification

  • Base: 749511c
  • Head: 2d13684
  • Fix: preserve the detected accelerando timestamp through Python, shared-types, native Rust, and the desktop resolver; select sections with unrounded structural boundaries while keeping persisted ranges integer-compatible.
  • Local evidence: Python 706 passed and 24 skipped at 100% statements/branches/functions/lines; shared-types 53 tests at 100% statements/branches/functions/lines; desktop 248 tests at 100% statements/branches/functions/lines; Rust core 19 plus accelerando contract 9; Ruff, mypy, ESLint, TypeScript, Rust format, and diff checks pass.

Security Notes

  • The new timing field is optional for legacy projects, finite, non-negative, and bounded by the existing section-time maximum; it is rejected without the owned plan and explicit provenance at shared and native boundaries.
  • Untrusted persisted/model plan text remains Unicode single-line validated, and user-authored copy remains verbatim. No new file, network, subprocess, IPC, or dependency path is introduced.

Latest exact-head verification

  • Base: 749511c
  • Head: 8f8b2da75acb0387632713212ca86b50e06c734d
  • Follow-up: formatted the added fractional-boundary regression test after hosted release-preflight identified the missed formatting check.
  • Local evidence remains: Python 706 passed and 24 skipped at 100% statements/branches/functions/lines; shared-types 53 tests at 100%; desktop 248 tests at 100%; Rust core 19 plus accelerando contract 9; Ruff, mypy, ESLint, TypeScript, Rust format, and diff checks pass.

Stamp one owned accelerandoPlan from existing tempo-stability speeding
onto the highest-priority active named vocal or bass, and open that
landing on the rehearsal map. Double-time and half-time feel flips
stay unnamed. Heuristic demo topology stays unnamed. This is not a
new MIR product.
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 28 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: ed3139cd-1c22-413d-9de8-5046d78b89c0

📥 Commits

Reviewing files that changed from the base of the PR and between 749511c and 8f8b2da.

📒 Files selected for processing (29)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/core/src/lib.rs
  • apps/desktop/core/tests/accelerando_plan_contract.rs
  • apps/desktop/src/features/workspace/FirstAccelerandoCallout.particle.test.tsx
  • apps/desktop/src/features/workspace/FirstAccelerandoCallout.test.tsx
  • apps/desktop/src/features/workspace/FirstAccelerandoCallout.tsx
  • apps/desktop/src/features/workspace/Workspace.accelerando-state.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/firstAccelerando.test.ts
  • apps/desktop/src/features/workspace/firstAccelerando.ts
  • apps/desktop/src/i18n/index.test.ts
  • apps/desktop/src/i18n/index.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
  • packages/shared-types/src/index.ts
  • packages/shared-types/test/accelerandoPlanProvenance.test.ts
  • packages/shared-types/test/index.test.ts
  • services/analysis-engine/src/bandscope_analysis/api.py
  • services/analysis-engine/src/bandscope_analysis/cli.py
  • services/analysis-engine/src/bandscope_analysis/roles/model.py
  • services/analysis-engine/src/bandscope_analysis/temporal/__init__.py
  • services/analysis-engine/src/bandscope_analysis/temporal/accelerando.py
  • services/analysis-engine/tests/test_accelerando_plan.py
  • services/analysis-engine/tests/test_accelerando_shared_role_isolation.py
  • services/analysis-engine/tests/test_api.py
  • services/analysis-engine/tests/test_cli.py

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.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Repair PR #1048 on the canonical existing branch feat/workspace-first-accelerando-plan only if the exact head is still ee3a22db8ca907fccd8d8312268e729cdd45d96a against protected develop@749511c3ad4000090048718f685c6bee6b3d2c25; refetch both immediately before writing and adapt rather than race if either moved.

Validated current-head defect: Devin thread PRRT_kwDORjvEXs6dI564 is real. cli.py already runs TemporalAnalyzer.analyze() for local audio, but discards those temporal features before run_analysis_job[_updates]; _build_local_audio_features() then carries stems/separation only, so _apply_accelerando() receives no authoritative beat_times and calls derive_beat_times(mix, sr), performing another librosa.beat.beat_track pass. This violates the PR's declared beat-grid reuse contract and adds avoidable latency. Do not suppress/resolve the finding without a causal repair.

Use TDD and keep scope at the owning production boundary. First add/strengthen a realistic regression that proves a local-audio run with an already-computed TemporalAnalyzer beat grid reaches _apply_accelerando without invoking derive_beat_times/a second beat tracker, including the cache-hit path if temporal features are cached. Then make the narrowest architectural repair so temporal BPM/beat-times produced for the local source are threaded into the analysis features exactly once and, if reusable feature caching is in the path, persisted/loaded with fail-closed finite validation. Remove the obsolete duplicate/discarded pre-analysis path rather than adding a third tracker. Preserve fallback behavior when temporal analysis genuinely fails; do not invent accelerando copy from malformed/missing beats, weaken validation, alter stem separation authority, or mix unrelated #811/#828/#897 work.

Run focused Python regressions plus the repository quickcheck/full relevant suite and exact 100% owned Python statement/branch evidence. Only resolve PRRT_kwDORjvEXs6dI564 after the addressed code is on the new exact head and GREEN. Leave informational template-duplication thread PRRT_kwDORjvEXs6dI58Q unresolved unless you independently verify and address a real current defect. Refetch exact new head/base/checks before claiming completion.

devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Repair PR #1048 on the canonical existing branch feat/workspace-first-accelerando-plan only, starting from exact current head 5d4d20d876b8c30487cd855441d747abbda0f48b against independently resolved protected develop@749511c3ad4000090048718f685c6bee6b3d2c25; refetch both immediately before every mutation and adapt rather than race if either moved. Apply receiving-code-review, systematic-debugging, test-driven-development, and verification-before-completion.

First causal defect: reopened thread PRRT_kwDORjvEXs6dI564 is verified at the full integrated entrypoint. services/analysis-engine/src/bandscope_analysis/cli.py still constructs TemporalAnalyzer for every local-audio request, calls analyze(audio_path), logs only BPM, and discards the returned temporal features before run_analysis_job[_updates]. _build_local_audio_features() subsequently carries stems/sample-rate/separation without authoritative beat_times; _apply_accelerando() can therefore call derive_beat_times and run librosa.beat.beat_track again. RED first through the actual CLI/local-audio path: prove an already-computed TemporalAnalyzer grid reaches accelerando without a second tracker, and cover cache store/load if the reusable local-feature cache owns this evidence. Then thread the once-computed finite positive beat times/BPM into analysis features and cache metadata/arrays at the narrowest existing ownership boundary, removing the discarded duplicate computation rather than adding another tracker. Preserve fallback derivation only when temporal analysis genuinely yields no valid grid and fail closed on malformed/NaN/Inf/non-monotonic/negative timing evidence as current contracts require.

Second current-head defect: unresolved thread PRRT_kwDORjvEXs6dLQbA shows ownedAccelerandoPlan() still accepts nonblank accelerandoPlan with absent accelerandoPlanSource, returning source null, while TypeScript/Rust/Python persistence contracts require explicit provenance. Add the smallest resolver/callout RED regression and then fail closed unless the snapshotted own source is exactly model or user; preserve valid user copy, current model-template/speeding/feel-flip checks, and inherited/accessor/Proxy defenses.

Run focused accelerando/CLI/cache/provenance tests first, then repository-pinned lint/format and canonical relevant/full analysis/shared-types/Rust/desktop verification with exact owned coverage evidence. Resolve only the two addressed threads after successor-head GREEN proof. Do not mix #811/#828/#897, #1040 native optional-role-field ownership, central workflows, or unrelated feature lanes. No force-push, destructive rebase, self-approval, or gate weakening.

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

@OpenCode Repair only the existing canonical branch feat/workspace-first-accelerando-plan; do not create a competing PR/branch and do not touch other repositories. Before any write, refetch and require exact head 1778dba18d605a2655191fea6384df72984d9cf1, live protected develop@749511c3ad4000090048718f685c6bee6b3d2c25, the current Workspace.tsx blob, review threads/checks, and active-writer evidence. If this lane has moved or another same-scope writer is active, inspect the intervening delta and stop rather than racing.

Apply superpowers:using-superpowers; use receiving-code-review to verify current thread PRRT_kwDORjvEXs6dNBtq, systematic-debugging for the causal boundary, test-driven-development for the behavior change, and verification-before-completion before claiming the repair.

Validated current-head defect: Workspace mutates localSongUpdateRef, workspaceInstanceRef, and previousSongRef during render when song !== previousSongRef.current. That render-phase mutation is observable component state outside React state and can be committed even when a concurrent/speculative render is abandoned, so a later committed render can inherit identity bookkeeping from a render that never committed. This is a real React purity/concurrency reliability defect, not merely a style note. It is branch-introduced; protected develop does not contain this identity-ref machinery.

TDD contract: first add the smallest realistic RED regression that exercises an abandoned/suspended or otherwise non-committed render followed by the committed song input, and proves workspace-instance authority / accelerando armed-state is derived only from committed song transitions. Do not write a success-shaped mock that only inspects source text. Then move transition bookkeeping out of render into a commit-safe mechanism (effect/layout-effect or another design with equivalent committed-render semantics), while preserving the intended contract: local immutable commitSongUpdate round-trips keep the callout armed, external/re-derived song inputs reset it, and no cross-workspace authority leaks. Keep the fix narrow; do not mix temporal-cache work, #1045/#1047 dependency-root changes, or unrelated UI.

Run the focused concurrency/armed-state tests, full desktop Vitest/lint/typecheck, and repository quickcheck/coverage exposed by the branch. Resolve only PRRT_kwDORjvEXs6dNBtq if exact-head GREEN proves the behavior. Refetch the new exact head/live base and report hosted exact-head evidence; do not self-approve, weaken gates, force-push, or claim merge readiness from predecessor/pending/model-only evidence.

github-code-quality[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.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@devin-ai review this PR at current HEAD SHA 90feef2. The accelerando contract now shares the explicit Unicode text policy and preserves user-owned guidance verbatim; verify this exact head.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Re-requesting review for current HEAD e202f5d after making temporal analysis cacheable and skipping redundant local audio decode on cache hits.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Re-requesting review for current HEAD e64bd97 after the hosted formatting gate fix.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Current HEAD 8f8b2da75acb0387632713212ca86b50e06c734d is ready for hosted recheck. The prior release-preflight formatting failure was fixed in the follow-up style commit; no security gate was weakened.

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