refactor(naming): use semantic temporal and score identifiers - #1145
Draft
seonghobae wants to merge 15 commits into
Draft
refactor(naming): use semantic temporal and score identifiers#1145seonghobae wants to merge 15 commits into
seonghobae wants to merge 15 commits into
Conversation
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Problem
BandScope exposed organization-owned generic single-word identifiers in two bounded contexts where domain language is already available:
run,beats,bpms,window,flagged,changes,before,after,deviation,intervals, andcv.dataprop/argument plus generic internal names such asstatus,zoom,container,observer,canvas,page,scale,viewport,retry, duplicatecancelledauthorities,result,error, and stale test-mock vocabulary.Exact ownership and scope
develop@749511c3ad4000090048718f685c6bee6b3d2c25.refactor/temporal-semantic-identifiers-2026-09-02.b2e7cdb3f1c473b95e6887dacc38f51eb8c3a5b7.services/analysis-engine/src/bandscope_analysis/temporal/stability.pyapps/desktop/src/features/score/ScoreViewer.tsxapps/desktop/src/features/score/ScoreViewer.test.tsxapps/desktop/src/features/score/ScoreView.tsxapps/desktop/src/features/score/ScoreView.test.tsxapps/desktop/src/features/score/pdfjs.tsapps/desktop/src/features/score/pdfjs.test.tsdocs/product-technical-gap-baseline.mdremains canonically introduced by stacked docs(gap): refresh product-technical baseline (74 repos, live census) #1116, so this narrow naming PR does not create a competing baseline owner.Test-first repair lineage
Tempo Stability
3fb96a3f01a127520e726fbe99023dafa4701f14adds an AST naming regression requiring bounded-context temporal identifiers while the production module still contains the legacy names.3f08af6eadf8bb4defcc5ba8020d5f142cee14aareplaces the private implementation vocabulary withtempo_change_run,beat_times_array,local_bpm_values,comparison_window_beats,flagged_boundaries,before_bpm_median,after_bpm_median,relative_bpm_deviation,beat_intervals, andbpm_variation_coefficient.3c6bb2d0a34b9f41b0f4986b6bd9ddf7bc99e1b9keeps the regression test's own non-throwaway identifier semantic (module_identifiers).Score viewer / pdf.js ACL
4e0b2b5133e673f670e4d685500d2e1fa1afd544changes the component contract tests to requirescorePdfBytes; production still expecteddataat that exact commit, so the test is intentionally RED by construction.492346e2183a6bc0714377acf9a58fdab32169c4changesScoreViewerProps.data → scorePdfBytesand internal names toviewerStatus,scoreTranslator,zoomScale,viewerContainer,resizeObserver,scoreCanvas,pdfPage,viewportScale,pageViewport, andretryPdfLoad.3c68966cc3251f4e389382d033fb96b02f31863bupdates the repository consumer inScoreView.tsxfrom<ScoreViewer data={pdfBytes}>to<ScoreViewer scorePdfBytes={pdfBytes}>.3562504091802b658fb9df7645a7e35009dff04bchangesloadScorePdf(data)toloadScorePdf(scorePdfBytes)while intentionally preserving pdf.js's externally mandatedgetDocument({ data: ... })field only at the adapter boundary.5794465d48fb8d9ff2ff941d891053fe8facc5e9updates the adapter regression to prove the semantic internal byte name is translated to the exact vendordatafield while preserving defensive-copy and hardened-parser options.25ea6dff470bbeb0059291757f4a5e85c74dcb05removes the remaining generic callback parameterstatusin the touched component contract in favor ofviewerStatus.bdd1cb2e130d1bc08a89fcf233738cb1d394431aseparates the two independent cancellation authorities aspdfLoadCancelledandpageRenderCancelled.29fd07fb8decdc936904bd1fd6739c34fc6d3363aligns touched ScoreViewer test fixtures with bounded PDF/viewer vocabulary while preserving Promise/pdf.js/ResizeObserver external members.16ea794447afcdcce68ac83c09deedcb6833be78fixes the repository-localScoreView.test.tsxmock that still destructured the obsoletedataprop after production switched toscorePdfBytes; without this propagation, viewer-byte assertions exercisedundefinedinstead of the new contract. The same commit qualifies the mock translator's generickeyastranslationKey.1571ed5ec00cc1a141c566a90fa0017d358a0c4cqualifies ScoreView-owned bridge/view state vocabulary (bridgeError,fallbackMessage,rawErrorMessage,firstErrorLine,scoreTranslator,scoreAttachments,selectedScoreAttachment,scoreError,scoreAttachment,scorePdfBytes,attachmentResult,removalConfirmed,scoreAttachmentEntry).b2e7cdb3f1c473b95e6887dacc38f51eb8c3a5b7immediately restores the pre-existing cyan opening-spinner presentation after the whole-file source update, preserving the naming-only behavior boundary before fresh verification.The cancellation/test/source follow-up commits are behavior-neutral except for repairing the stale mock contract so existing viewer behavior tests exercise the actual
scorePdfBytesprop. No test is skipped, weakened, xfailed, or replaced with model-only evidence.Compatibility boundary
This PR intentionally preserves the existing
TempoChange/TempoStabilityserialized keys (time,from_bpm,to_bpm,stability,tempo_changes) and publicanalyze_tempo_stability(beat_times)entrypoint because those shapes cross existing analysis consumers and require a separately verified migration/ACL if changed.For score parsing, pdf.js owns the external
dataoption name, so it remains exactlydatainside thegetDocumentadapter object. BandScope-owned callers usescorePdfBytes. Promise members such aspromise, pdf.js members such asnumPages/getPage, Tauri command payload fields, React'skey, and ResizeObserver platform contracts remain at their external/framework boundaries. Repository-local ScoreViewer consumers and mocks are translated coherently; no compatibility alias or duplicate generic internal PDF field is retained.No database table/column/index/constraint, ORM mapping, persistence format, dependency, network, filesystem, subprocess, IPC, model, or release-format change occurs here. No DB migration, FK/index rewrite, UPSERT change, locking/read-write change, or rollback data transform is required.
Verification / merge gate
Keep Draft and unmerged until the unchanged exact head has the focused temporal/score regressions plus every applicable repository and central CI/build/release/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, zero valid unresolved current-head findings, and a qualifying independent non-author last-push approval under ordinary branch protection. Predecessor/base/model-only/queued evidence does not transfer.
Fresh exact-head
CodeQL PRrun33642724317onb2e7cdb3...terminatedstartup_failurebefore creating any jobs. This reproduces the shared required-workflow admission incident on the final source head rather than a score-viewer test/job failure. Other exact-head workflows are queued/pending and there are currently no review submissions, so this PR remains Draft and cannot merge.Security Notes
Tempo changes remain pure in-memory numeric computation with existing fail-closed malformed-input behavior. Score parsing remains validated-resource-only and preserves the hardened local-worker,
enableXfa: false,useWorkerFetch: false, defensive-copy boundary. The naming change adds no authority, remote fetch, storage, or execution path.