Refine public docs and React ownership - #1
Conversation
|
@codex review this |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f39bc0d490
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { describe, expect, it } from 'vitest' | ||
| import { AudioSampleRing } from './audioSampleRing' | ||
| import { AudioSampleRing } from '../../src/audio/audioSampleRing' |
There was a problem hiding this comment.
Moving the specs under tests/ takes them outside both TypeScript projects: tsconfig.app.json includes only src, while tsconfig.node.json still includes the now-deleted e2e directory. As a result, the tsc -b command behind pnpm typecheck no longer checks any unit or Playwright spec, allowing type-invalid test fixtures and API usage to pass that CI stage; include tests/ in an appropriate project or add a dedicated test tsconfig.
AGENTS.md reference: AGENTS.md:L46-L49
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
Fixed in 8202c63. Added a dedicated tsconfig.test.json project, referenced it from the root build, and removed the stale e2e include. I also verified the coverage by temporarily introducing a type error under tests/: pnpm typecheck failed with TS2322 before the probe was removed. Full pnpm run ci passes.
| For monophonic listening, `audio/usePitchDetection.ts` captures microphone samples through the Web Audio API, estimates a frequency, and converts it to a `PitchReading` through `music`. | ||
|
|
||
| For experimental polyphonic listening, `audio/usePolyphonicPitchDetection.ts` captures PCM through the public AudioWorklet, maintains a bounded sample ring, and sends analysis work to a Worker. The Worker loads Basic Pitch and returns note candidates; `music/chordAnalysis.ts` turns a result into a displayable chord reading. The feature is intentionally not source separation or continuous-song transcription. |
There was a problem hiding this comment.
The documented files audio/usePitchDetection.ts and audio/usePolyphonicPitchDetection.ts no longer exist after this commit; their implementations were moved under src/audio/react/. The same stale-path problem occurs for the learning, playback, and MIDI hooks on line 31, so the new maintainer-facing runtime guide directs readers to five removed files and obscures the ownership layout it is intended to document.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
Fixed in 8202c63. Updated all five hook references to their owner-local react/ paths in docs/architecture.md.
|
@codex review this |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: 鈩癸笍 About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 馃憤. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Outcome
Make Fret & Key easier to understand as a public work and establish clearer ownership boundaries before the next source-layout migration.
What changed
Explicitly preserved
Non-goals
Risk and review notes
The test-directory move inflates the file count but is isolated in its own commit. The main behavior-sensitive areas are learning-mode transitions, microphone and playback lifecycle cleanup, persisted preference compatibility, and pointer glissando behavior.
Verification