Add production-ready client frontend with in-browser MP3 metadata cleanse#3
Closed
ChrisAdamsdevelopment wants to merge 1 commit into
Closed
Conversation
Reviewer's GuideImplements a new Vite/React-based client frontend that supports authenticated SEO generation and dual MP3 cleansing paths (browser-side ID3 rewrite and server-side processing) with metadata analysis, logging, and upgrade-aware usage metering. Sequence diagram for in-browser quick MP3 cleanse flowsequenceDiagram
title Quick MP3 cleanse in browser sequence
actor User
participant App
participant MetadataUtils
participant BrowserURL
User->>App: Upload MP3 file
App->>App: onFile(file)
App->>MetadataUtils: readFileMetadata(file)
MetadataUtils-->>App: metadata {title, artist, genre, format, risk, detectedMarkers}
App->>App: setAnalysis(metadata)
App->>App: update ctx artist, title, genre
App->>App: setTab(analysis)
User->>App: Edit context fields
User->>App: Click Generate AI SEO Payload
App->>App: build promptText from ctx
App->>BackendAPI: POST /api/generate-seo
BackendAPI-->>App: SEO payload {title, description, tags}
App->>App: setSeo(payload)
App->>App: setTab(seo)
User->>App: Click Quick Cleanse (Browser)
App->>MetadataUtils: writeMP3Metadata(file, seo + ctx)
MetadataUtils-->>App: new MP3 Blob
App->>BrowserURL: URL.createObjectURL(blob)
BrowserURL-->>App: objectUrl
App->>App: setProcessedAsset(objectUrl, name)
App->>App: addLog(success, In-browser cleanse complete)
User->>App: Click Download Processed File link
App->>User: Download cleansed MP3
User->>App: Upload new file or navigate away
App->>BrowserURL: URL.revokeObjectURL(previousObjectUrl)
Sequence diagram for full server cleanse with upgrade handlingsequenceDiagram
title Full server cleanse sequence with usage and upgrade
actor User
participant App
participant BackendAPI
participant Stripe
User->>App: Upload audio file
App->>App: onFile(file) and analysis setup
User->>App: Edit context and SEO fields
User->>App: Click Full Server Cleanse
App->>App: Build FormData(file, title, artist, genre, description, tags, lyrics)
App->>BackendAPI: POST /api/process
alt Unauthorized
BackendAPI-->>App: 401 Unauthorized
App->>App: logout()
else Payment required
BackendAPI-->>App: 402 Payment Required
App->>App: setShowUpgrade(true)
App->>User: Show upgrade modal
User->>App: Click Upgrade (existing flow)
App->>Stripe: Open checkout session
Stripe-->>User: Complete payment
User->>App: Retry Full Server Cleanse
else Success
BackendAPI-->>App: 200 OK with cleansed file Blob
BackendAPI-->>App: X-Usage-This-Month, X-Usage-Limit headers
App->>App: update usage state
App->>App: setProcessedAsset(objectUrl, name)
App->>App: setForensicReport(static summary)
App->>App: addLog(success, Server cleanse complete)
App->>User: Enable Download Processed File
end
Class diagram for React client app and metadata utilitiesclassDiagram
class App {
+string BACKEND_URL
+string TOKEN_KEY
+string USER_KEY
+Tab tab
+User user
+string token
+File file
+any analysis
+ProcessedAsset processedAsset
+ForensicReport forensicReport
+boolean showUpgrade
+Usage usage
+LogItem[] logs
+CtxState ctx
+SeoState seo
+boolean isMp3
+constructor App()
+onFile(file)
+generateSeo()
+quickCleanse()
+serverCleanse()
+addLog(level, message)
+logout()
}
class AuthScreen {
+string email
+string password
+string mode
+string error
+submit(event)
+render()
}
class MetadataUtils {
<<module>>
+readFileMetadata(file) MetadataResult
+writeMP3Metadata(file, metadata) Blob
}
class MetadataResult {
+string title
+string artist
+string genre
+string format
+string risk
+string[] detectedMarkers
}
class User {
+number id
+string email
+string plan
}
class LogItem {
+string id
+string level
+string message
+string ts
}
class CtxState {
+string artist
+string title
+string genre
+string vibe
+string lyrics
}
class SeoState {
+string title
+string description
+string tags
+string lyrics
}
class Usage {
+number thisMonth
+number limit
}
class ProcessedAsset {
+string url
+string name
}
class ForensicReport {
+number removedCount
+string[] removedTags
+string timestamp
}
App --> AuthScreen : uses for auth
App --> MetadataUtils : calls
App --> User : holds current user
App --> LogItem : maintains logs
App --> CtxState : holds context
App --> SeoState : holds SEO data
App --> Usage : tracks usage
App --> ProcessedAsset : holds download
App --> ForensicReport : shows report
MetadataUtils --> MetadataResult : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The metadata utilities and related state in
App.tsxare all effectively untyped (anyforanalysis/forensicReportand a JSmetadata.jsmodule imported into a TSX file); consider adding explicit interfaces and convertingmetadata.jsto TypeScript so you get compile-time safety around the metadata shape and risk markers. - The async flows (
readFileMetadata,writeMP3Metadata,generateSeo,serverCleanse) assume happy-path behavior fromparseBlobandfetch/res.json(); wrapping these in try/catch and logging a structured error to the system log would make the UI more resilient to network or parsing failures instead of potentially throwing at runtime.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The metadata utilities and related state in `App.tsx` are all effectively untyped (`any` for `analysis`/`forensicReport` and a JS `metadata.js` module imported into a TSX file); consider adding explicit interfaces and converting `metadata.js` to TypeScript so you get compile-time safety around the metadata shape and risk markers.
- The async flows (`readFileMetadata`, `writeMP3Metadata`, `generateSeo`, `serverCleanse`) assume happy-path behavior from `parseBlob` and `fetch`/`res.json()`; wrapping these in try/catch and logging a structured error to the system log would make the UI more resilient to network or parsing failures instead of potentially throwing at runtime.
## Individual Comments
### Comment 1
<location path="client/src/App.tsx" line_range="23-24" />
<code_context>
+ const submit = async (e: React.FormEvent) => {
+ e.preventDefault();
+ setError('');
+ const res = await fetch(`${BACKEND_URL}/api/${mode}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ email, password }) });
+ const data = await res.json();
+ if (!res.ok) return setError(data.error || 'Auth failed');
+ localStorage.setItem(TOKEN_KEY, data.token); localStorage.setItem(USER_KEY, JSON.stringify(data.user));
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle network / parsing errors around auth fetch to avoid unhandled promise rejections.
This flow assumes both `fetch` and `res.json()` always succeed. A network/CORS failure or invalid JSON will throw and likely surface as an uncaught error. Consider wrapping this in a try/catch and mapping failures to a user-facing `setError`, and optionally applying the same pattern to other fetch calls for consistent error handling.
</issue_to_address>
### Comment 2
<location path="client/src/utils/metadata.js" line_range="10" />
<code_context>
+];
+
+export async function readFileMetadata(file) {
+ const metadata = await parseBlob(file);
+ const common = metadata.common || {};
+ const format = metadata.format?.container || file.type || 'Unknown';
</code_context>
<issue_to_address>
**issue (bug_risk):** Add error handling around `parseBlob` to handle unsupported or corrupt files.
If `parseBlob` throws for a corrupt/unsupported file, the exception will currently bubble up and break the flow. Consider wrapping this call in a try/catch, returning a safe fallback (e.g., empty metadata and a default risk level) and logging/handling the error so a single bad file doesn’t take down the UI.
</issue_to_address>
### Comment 3
<location path="client/src/utils/metadata.js" line_range="23" />
<code_context>
+ artist: common.artist || '',
+ genre: Array.isArray(common.genre) ? (common.genre[0] || '') : (common.genre || ''),
+ format,
+ risk: detectedMarkers.length > 0 ? 'HIGH' : 'Low',
+ detectedMarkers,
+ };
</code_context>
<issue_to_address>
**suggestion:** Normalize risk level casing for consistency with UI logic.
The `risk` field currently mixes `'HIGH'` and `'Low'`. Since `App.tsx` checks `analysis.risk === 'HIGH'` for styling and otherwise renders the value directly, aligning the values (e.g., `'HIGH'` / `'LOW'`) will keep the semantics clear and avoid future logic mismatches if more cases are added.
```suggestion
risk: detectedMarkers.length > 0 ? 'HIGH' : 'LOW',
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+23
to
+24
| const res = await fetch(`${BACKEND_URL}/api/${mode}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ email, password }) }); | ||
| const data = await res.json(); |
There was a problem hiding this comment.
issue (bug_risk): Handle network / parsing errors around auth fetch to avoid unhandled promise rejections.
This flow assumes both fetch and res.json() always succeed. A network/CORS failure or invalid JSON will throw and likely surface as an uncaught error. Consider wrapping this in a try/catch and mapping failures to a user-facing setError, and optionally applying the same pattern to other fetch calls for consistent error handling.
| ]; | ||
|
|
||
| export async function readFileMetadata(file) { | ||
| const metadata = await parseBlob(file); |
There was a problem hiding this comment.
issue (bug_risk): Add error handling around parseBlob to handle unsupported or corrupt files.
If parseBlob throws for a corrupt/unsupported file, the exception will currently bubble up and break the flow. Consider wrapping this call in a try/catch, returning a safe fallback (e.g., empty metadata and a default risk level) and logging/handling the error so a single bad file doesn’t take down the UI.
| artist: common.artist || '', | ||
| genre: Array.isArray(common.genre) ? (common.genre[0] || '') : (common.genre || ''), | ||
| format, | ||
| risk: detectedMarkers.length > 0 ? 'HIGH' : 'Low', |
There was a problem hiding this comment.
suggestion: Normalize risk level casing for consistency with UI logic.
The risk field currently mixes 'HIGH' and 'Low'. Since App.tsx checks analysis.risk === 'HIGH' for styling and otherwise renders the value directly, aligning the values (e.g., 'HIGH' / 'LOW') will keep the semantics clear and avoid future logic mismatches if more cases are added.
Suggested change
| risk: detectedMarkers.length > 0 ? 'HIGH' : 'Low', | |
| risk: detectedMarkers.length > 0 ? 'HIGH' : 'LOW', |
![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: 987e81749c
ℹ️ 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".
| const url = URL.createObjectURL(blob); | ||
| if (processedAsset) URL.revokeObjectURL(processedAsset.url); | ||
| setProcessedAsset({ url, name: file.name.replace(/(\.[^.]+)$/, '-server-cleanse$1') }); | ||
| setForensicReport({ removedCount: 0, removedTags: ['ID3 private frames', 'vendor signatures'], timestamp: new Date().toISOString() }); |
There was a problem hiding this comment.
The forensic report shown after serverCleanse is hard-coded (removedCount: 0 and fixed tag names), so users always see fabricated results even when the backend reports real removals via X-Forensic-* headers. This directly misstates cleanse outcomes and can invalidate user trust in the analysis output; parse the response headers instead of injecting static values.
Useful? React with 👍 / 👎.
|
|
||
| <div className="bg-slate-900 border border-slate-800 rounded-xl p-4"><h3 className="font-semibold mb-2">System Log</h3><div className="space-y-2 max-h-56 overflow-auto">{logs.map((l)=><div key={l.id} className={`text-sm ${l.level==='error'?'text-red-400':l.level==='success'?'text-emerald-400':'text-cyan-300'}`}>[{l.ts}] {l.message}</div>)}</div></div> | ||
| </main> | ||
| {showUpgrade && <div className="fixed inset-0 grid place-items-center bg-black/60"><div className="bg-slate-900 border border-slate-700 rounded-xl p-5"><p className="mb-3">Usage limit reached. Upgrade required.</p><button className="bg-cyan-600 px-4 py-2 rounded" onClick={()=>setShowUpgrade(false)}>Close</button></div></div>} |
There was a problem hiding this comment.
When /api/process returns 402, the UI opens showUpgrade, but the modal only offers a Close button and no checkout trigger, leaving free-tier users blocked from progressing inside this frontend once they hit the monthly limit. Add a checkout path (for example calling the existing checkout-session API) so users can actually upgrade from this flow.
Useful? React with 👍 / 👎.
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.
Superseded by PR #9, which cleanly integrates browser MP3 quick cleanse into the root app.