Conversation
chore : 버전 2.8.0
fix : package.json 수정
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ector [Refactor] : 크롤링 element selector 명확하게 지정 및 DB limit 상향 (#262)
…racy [Fix] : TJ 번호 크롤링 제목/가수 셀렉터 정확도 개선 (#264)
fix : matrix로 병렬 작업 지원.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
[Chore] : Google Play Store 등록 준비 및 앱 메타데이터 정비 (#268)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Summary by QodoTWA Google Play 등록 준비 및 TJ 크롤링 병렬화
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. themeColor not #1a1a2e
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| "navigationDividerColorDark": "#1a1a2e", | ||
| "backgroundColor": "#1a1a2e", | ||
| "enableNotifications": false, | ||
| "themeColor": "#1A1A2E", |
There was a problem hiding this comment.
1. themecolor not #1a1a2e 📘 Rule violation ≡ Correctness
apps/twa/twa-manifest.json sets themeColor to #1A1A2E instead of the required lowercase #1a1a2e. This can break enforcement checks that expect the exact string and violates the mandated theme color value.
Agent Prompt
## Issue description
`twa-manifest.json` must enforce `themeColor` exactly as `"#1a1a2e"` (lowercase), but the PR sets it to `"#1A1A2E"`.
## Issue Context
Compliance rule requires an exact string match for the TWA manifest theme color.
## Fix Focus Areas
- apps/twa/twa-manifest.json[7-7]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+88
to
91
| export async function getSongsAllDB(max: number = 100000) { | ||
| const supabase = getClient(); | ||
|
|
||
| const { data, error } = await supabase |
There was a problem hiding this comment.
2. Db queries truncate silently 🐞 Bug ≡ Correctness
Several getDB() helpers use .limit(100000) without pagination, but callers treat the results as complete (e.g., building num_tj maps or “already tagged” sets). Once tables exceed 100k rows, this silently breaks dedupe/skip logic and can lead to duplicate inserts or redundant processing.
Agent Prompt
## Issue description
Supabase fetch helpers (e.g., `getSongsAllDB`, `getSongsAllWithTjDB`, `getSongTagSongIdsDB`) hard-cap results via `.limit(100000)` and return only the first page. Downstream jobs assume these are complete datasets for deduplication/skip decisions, so once any underlying table grows beyond the limit the jobs will silently behave incorrectly (re-tag already-tagged songs, miss existing TJ numbers and attempt inserts, etc.).
## Issue Context
- `taggingSongs.ts` loads *all songs* and *all tagged song IDs* to skip work.
- `crawlAllTJSongByNumber.ts` loads *all songs with num_tj* to decide whether to insert/update.
- Current fetch helpers do not implement pagination (`.range(...)`) or any loop until exhaustion.
## Fix Focus Areas
- packages/crawling/src/supabase/getDB.ts[88-115]
- packages/crawling/src/supabase/getDB.ts[155-163]
- packages/crawling/src/cron/taggingSongs.ts[11-28]
- packages/crawling/src/cron/crawlAllTJSongByNumber.ts[27-65]
## Suggested fix
1. Implement pagination utilities in `getDB.ts` (e.g., fetch in pages of 1000/5000 using `.range(offset, offset+pageSize-1)` until a page returns fewer than `pageSize`).
2. Update `getSongsAllWithTjDB()` and `getSongTagSongIdsDB()` to use pagination and return complete results (or, if “complete” is not required, rename functions to reflect partial reads and adjust callers accordingly).
3. Consider filtering server-side to reduce result size (e.g., `not('num_tj','is',null)` for the TJ map case).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+51
to
54
| export async function getSongsKyNotNullDB(max: number = 100000) { | ||
| const supabase = getClient(); | ||
|
|
||
| const { data, error } = await supabase |
There was a problem hiding this comment.
3. Excessive supabase overfetch 🐞 Bug ➹ Performance
The default max for several DB fetch functions was increased to 100,000, but callers often process only a small prefix (e.g., verify job stops at 5,000; tagging stops at 20,000). This creates unnecessary data transfer and memory usage each run.
Agent Prompt
## Issue description
`getSongsKyNotNullDB()` (and other helpers) now default to `max=100000`, but some cron scripts intentionally stop after processing a much smaller number of rows. Fetching far more rows than needed increases bandwidth/memory and can slow down scheduled runs.
## Issue Context
- `crawlYoutubeVerify.ts` breaks after 5,000 items but currently calls `getSongsKyNotNullDB()` with default max.
- `taggingSongs.ts` breaks after 20,000 items but calls `getSongsAllDB()` with default max.
## Fix Focus Areas
- packages/crawling/src/supabase/getDB.ts[36-64]
- packages/crawling/src/supabase/getDB.ts[88-100]
- packages/crawling/src/cron/crawlYoutubeVerify.ts[19-49]
- packages/crawling/src/cron/taggingSongs.ts[11-24]
## Suggested fix
- Pass explicit caps from callers matching their processing limits (e.g., `getSongsKyNotNullDB(5000)` and `getSongsAllDB(20000)`).
- Alternatively, change defaults back to smaller values and require explicit `max` for “full table” use-cases (paired with pagination from the other finding).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+6
to
+8
| permissions: | ||
| contents: write | ||
|
|
There was a problem hiding this comment.
4. Workflow has write permission 🐞 Bug ⛨ Security
The new crawl workflow requests contents: write even though it only checks out code and runs scripts. This unnecessarily expands the blast radius if the workflow is ever compromised.
Agent Prompt
## Issue description
The workflow declares `permissions: contents: write` but does not appear to push commits/tags/releases. Granting write access is unnecessary and increases risk.
## Issue Context
Steps: checkout, setup node/pnpm, install deps, write `.env`, run `pnpm run tj-all-number`.
## Fix Focus Areas
- .github/workflows/crawl_tj_all_number.yml[6-8]
## Suggested fix
- Change to `permissions: contents: read` (or remove the block entirely to use the default minimal permissions), unless a later step truly requires write access.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+25
to
+28
| // title3 내부에는 아이콘용 <p class="no-ico">가 div.flex-box > ul 안에 중첩돼 있으므로 | ||
| // 자손 셀렉터(' p') 대신 직계 자식(' > p')만 선택해 실제 제목만 추출한다. | ||
| const title = gridContainer.find('.grid-item.title3 span').text().trim(); | ||
| const artist = gridContainer.find('.grid-item.title4 span').text().trim(); |
There was a problem hiding this comment.
5. Misleading tj selector comment 🐞 Bug ⚙ Maintainability
The comment says to use a direct-child > p selector to avoid nested icon markup, but the implementation selects span elements instead. This mismatch makes future maintenance risky because the rationale and the actual selector no longer align.
Agent Prompt
## Issue description
`crawlTJSongByNumber()` includes a comment describing a `> p` direct-child strategy, but the code uses `.find('... span')`. This discrepancy is confusing and can cause incorrect future edits.
## Issue Context
A separate TJ crawler parses title text from nested `<p>` elements, so the exact DOM structure is subtle and should be documented accurately.
## Fix Focus Areas
- packages/crawling/src/crawling/crawlTJSongByNumber.ts[25-28]
## Suggested fix
- Either update the comment to match the current `span`-based extraction (and explain why `span` is correct), or update the selector to match the documented `> p` approach.
- Consider adding a small unit/regression test with a saved HTML fixture to lock the intended extraction behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
📌 PR 제목
[Type] : 작업 내용 요약
📌 변경 사항
💬 추가 참고 사항