feature: implement crawler detail page#24
Conversation
- AuthenticationContext/use-authentication tests: toBeNull → toBeUndefined for user state - use-crawler-code-runner tests: toBeNull → toBeUndefined for result/error - crawler-code-runner-function: rename normalize tests since field is now omitted when undefined - extensions.ts: use object destructuring for fixture parameter (vitest v4 requirement) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds view and edit for individual crawlers, following the schedulers detail pattern but adapted to the crawler API's full-replacement PUT semantics. - /crawlers/:id route and CrawlerDetailPage with name, url_pattern, code, and schemas - useGetCrawler and useUpdateCrawler hooks with skipToken-guarded detail query - UpdateCrawlerInput as web/data discriminated union matching worker contract - CrawlersPage cards become keyboard-activatable with delete stopPropagation - CodeEditorPanel gains opt-in showDefaultTemplate (off on detail page) - Inline JSON schema validation on blur and DOM-warning-free SchemaHelper - 26 new tests: hook auth headers, type=data rendering, revert, save error toast Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Walkthrough새로운 크롤러 상세 기능이 추가되었습니다: 보호된 경로 Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Page as CrawlerDetailPage
participant Hook as useGetCrawler / useUpdateCrawler
participant Server as API Server
participant Cache as Query Cache
User->>Page: Navigate to /crawlers/:id
activate Page
Page->>Hook: useGetCrawler(id)
activate Hook
Hook->>Server: GET /crawlers/:id
Server-->>Hook: CrawlerRow
Hook->>Cache: Populate detail cache
Hook-->>Page: crawler data
deactivate Hook
Page->>User: Render form with data
deactivate Page
User->>Page: Edit fields
Page->>Page: Track dirty & validate JSON
Page-->>User: Show inline validation
User->>Page: Click Save
Page->>Hook: useUpdateCrawler -> updateCrawler(payload)
activate Hook
Hook->>Server: PUT /crawlers/:id
Server-->>Hook: Updated CrawlerRow
Hook->>Cache: Invalidate list, update detail cache
Hook-->>Page: updated crawler
deactivate Hook
Page->>User: Update form, show success toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
- CrawlerDetailPage: replace useEffect form seeding with render-phase state adjustment (react-hooks/set-state-in-effect rule) - extensions.ts: disable no-empty-pattern for vitest v4 fixture (runtime requires object destructure pattern that the rule forbids) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Review Context이 PR은 주의 깊게 검토가 필요한 영역
Known deferred (out of scope for TES-27)
Internal review summary10 subagent pass 완료 후 반영: form overwrite guard (render-phase adjustment), discriminated union, skipToken, inline validation, prop-forward filter, exact invalidation, readOnly onBlur gate, test 강화 (119 tests pass, lint clean). |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
applications/web/sources/pages/CrawlersPage.tsx (1)
386-412:⚠️ Potential issue | 🟡 Minor상호작용 요소 중첩(접근성) — 추후 Link 리팩토링 시 해결 권장
role="button"이 지정된CrawlerCard내부에 네이티브<button>(DeleteButton)이 중첩되어 있습니다. WAI-ARIA 관점에서 대화형 요소를 다른 대화형 요소 안에 중첩하는 것은 권장되지 않으며, 일부 스크린 리더에서 내부 버튼 포커스/발표가 어색해질 수 있습니다. PR 설명상 "Card → Link 리팩토링"은 본 PR 범위 밖으로 지연된 항목이므로 이번에는 참고만 남깁니다.추가로 한 가지 확인:
onKeyDown에서event.target !== event.currentTarget가드로 삭제 버튼 위에서의 Enter/Space가 카드 네비게이션을 트리거하지 않도록 한 점은 적절합니다. 다만 Space 키의 경우 네이티브<button>의 기본 동작이 이미 click을 발생시키므로, 현재 구현은 의도대로 동작합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@applications/web/sources/pages/CrawlersPage.tsx` around lines 386 - 412, The CrawlerCard (role="button") contains an interactive DeleteButton which can trigger the card's navigation; to avoid nested-interaction issues without a full Link refactor, update the DeleteButton handlers to stop the card from receiving events: inside the DeleteButton onClick wrapper (the call to handleDelete) call event.stopPropagation() before invoking handleDelete, and add an onKeyDown on DeleteButton that calls event.stopPropagation() (and prevents default for Space if needed) so keyboard events on the delete button don't bubble to the CrawlerCard onKeyDown; reference CrawlerCard, onKeyDown, DeleteButton, and handleDelete to locate and change the handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@applications/web/sources/hooks/use-crawler-manager.test.tsx`:
- Around line 249-268: The test relies on a fixed 50ms setTimeout to wait for
microtasks; replace that with a deterministic assertion using Vitest's waitFor
(or assert immediately) so the test isn't flaky. Specifically, remove the `await
new Promise((resolve) => setTimeout(resolve, 50));` and either immediately
assert `expect(result.current.isLoading).toBe(false)` and
`expect(requestCount).toBe(0)` after `renderHook(() => useGetCrawler(undefined),
{ wrapper: createWrapper() })`, or use `await vi.waitFor(() =>
expect(result.current.isLoading).toBe(false))` and then assert
`expect(requestCount).toBe(0)`; keep references to `useGetCrawler`,
`result.current`, `requestCount`, `worker.use` and `MANAGER_URL` to locate the
test.
In `@applications/web/sources/hooks/use-crawler-manager.ts`:
- Around line 25-43: The issue is that validateCrawlerBody unconditionally
normalizes input_schema to { body: 'string' } causing handleUpdateCrawler to
overwrite existing schemas for web crawlers; fix by making input_schema optional
on UpdateCrawlerWebInput (add input_schema?: Record<string, unknown>), change
validateCrawlerBody to only apply the default normalization for creation (POST)
or when input_schema is explicitly absent on POST, and update
handleUpdateCrawler to only include input_schema in the DB update if the request
body actually contains input_schema; reference UpdateCrawlerWebInput,
UpdateCrawlerInput, validateCrawlerBody, and handleUpdateCrawler when making
these changes.
In `@applications/web/sources/pages/CrawlerDetailPage.test.tsx`:
- Around line 242-266: Add a recovery path to the test 'shows inline error and
disables save on invalid JSON schema' by, after filling the invalid JSON and
asserting the error and disabled save, replacing the textarea
(page.getByLabelText('Output schema')) with a valid JSON string (e.g. '{}'),
assert the inline error message is gone and the save button
(page.getByRole('button', { name: /Save/ })) becomes enabled, click save, then
assert putCount increments (ensuring the worker.put handler is invoked) to
verify the PUT is performed; update assertions around saveButton and putCount
accordingly.
In `@applications/web/sources/pages/CrawlerDetailPage.tsx`:
- Around line 486-489: The spinner is currently visual-only; update the
CrawlerDetailPage loading render (the isLoading branch that returns
LoadingContainer and Spinner) to expose loading to screen readers by adding an
accessible status element: keep Spinner visually rendered (optionally
aria-hidden="true") and include a text node with role="status" and
aria-live="polite" (or aria-busy on a wrapper) containing a short message like
"Loading..." that is visually hidden but announced by assistive tech; modify the
LoadingContainer/Spinner rendering in CrawlerDetailPage.tsx to include this
accessible status element so screen readers receive the loading state.
- Around line 459-462: The response handling currently unconditionally
overwrites the user's in-progress edits by calling setForm(next) /
setPristine(next) after updateCrawler(payload); instead, only apply the
server-derived state if the current form still matches the submission payload
(or if a matching request/version token is present). Modify the submit flow
around updateCrawler/deriveFormState to compare the live form state (the form
variable/state used when calling updateCrawler) against payload (or use a
requestId/version on payload and response) and only call setForm(next) and
setPristine(next) when they correspond to the same submission; otherwise ignore
the response-derived overwrite to preserve the user's ongoing edits.
- Around line 550-579: When a schema field is edited the existing validation
error should be cleared or revalidated immediately so Save isn't stuck disabled;
update the onChange handlers for SchemaArea (both input_schema and
output_schema) to also clear or re-run validation for the respective
schemaErrors key (e.g. call setSchemaErrors({...schemaErrors, input_schema:
undefined}) or invoke validateSchemaField('input_schema', newValue) and update
schemaErrors accordingly) in addition to updating form via setForm, so hasError
and aria-invalid reflect the live state and the Save button becomes enabled as
soon as the JSON is corrected (before blur).
- Around line 434-455: The payload for web crawlers currently sends the raw
form.url_pattern which can include leading/trailing whitespace even though
validation used form.url_pattern.trim(); update the submit logic in
CrawlerDetailPage (where payload is built for crawler.type === 'web') to send a
trimmed URL pattern (e.g., compute const trimmedUrlPattern =
form.url_pattern.trim() and use that in the payload's url_pattern) so the stored
regex has no accidental whitespace.
---
Outside diff comments:
In `@applications/web/sources/pages/CrawlersPage.tsx`:
- Around line 386-412: The CrawlerCard (role="button") contains an interactive
DeleteButton which can trigger the card's navigation; to avoid
nested-interaction issues without a full Link refactor, update the DeleteButton
handlers to stop the card from receiving events: inside the DeleteButton onClick
wrapper (the call to handleDelete) call event.stopPropagation() before invoking
handleDelete, and add an onKeyDown on DeleteButton that calls
event.stopPropagation() (and prevents default for Space if needed) so keyboard
events on the delete button don't bubble to the CrawlerCard onKeyDown; reference
CrawlerCard, onKeyDown, DeleteButton, and handleDelete to locate and change the
handlers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64e1c618-2e54-48a1-9a48-a6a0ae91362b
📒 Files selected for processing (12)
applications/web/sources/Application.tsxapplications/web/sources/components/crawlers/CodeEditorPanel.tsxapplications/web/sources/contexts/AuthenticationContext.test.tsxapplications/web/sources/hooks/use-authentication.test.tsxapplications/web/sources/hooks/use-crawler-code-runner.test.tsxapplications/web/sources/hooks/use-crawler-manager.test.tsxapplications/web/sources/hooks/use-crawler-manager.tsapplications/web/sources/pages/CrawlerDetailPage.test.tsxapplications/web/sources/pages/CrawlerDetailPage.tsxapplications/web/sources/pages/CrawlersPage.tsxapplications/web/sources/tests/extensions.tsfunctions/crawler-code-runner-function/tests/index.test.ts
| interface UpdateCrawlerWebInput { | ||
| id: string; | ||
| type: 'web'; | ||
| name: string; | ||
| url_pattern: string; | ||
| code: string; | ||
| output_schema?: Record<string, unknown>; | ||
| } | ||
|
|
||
| interface UpdateCrawlerDataInput { | ||
| id: string; | ||
| type: 'data'; | ||
| name: string; | ||
| code: string; | ||
| input_schema: Record<string, unknown>; | ||
| output_schema?: Record<string, unknown>; | ||
| } | ||
|
|
||
| type UpdateCrawlerInput = UpdateCrawlerWebInput | UpdateCrawlerDataInput; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# input_schema 필수 여부 및 PUT 바디 검증 로직 확인
rg -nP -C5 '\bvalidateCrawlerBody\b'
rg -nP -C3 '\binput_schema\b' --type=ts -g '!**/node_modules/**' -g 'workers/**'
ast-grep --pattern $'function updateCrawler($$$) { $$$ }'Repository: gignac-cha/audio-underview
Length of output: 30698
🏁 Script executed:
# Check usages of UpdateCrawlerInput and related code
cd applications/web && rg -nP 'UpdateCrawler(Web|Data)?Input' --type=ts
# Find the actual PUT request construction
rg -nP -C8 'handleUpdateCrawler|UpdateCrawlerInput' applications/web/sources --type=ts --type=tsx
# Check what the server returns for web crawlers
rg -nP -C3 'type.*web.*input_schema' workers/crawler-manager-worker --type=tsRepository: gignac-cha/audio-underview
Length of output: 688
🏁 Script executed:
# Find the actual implementation of updateCrawlerRequest and how it constructs the request
rg -nP -A20 'async function updateCrawlerRequest' applications/web/sources/hooks/use-crawler-manager.ts
# Check CrawlerDetailPage to see how it constructs the update payload
rg -nP -B5 -A10 'updateCrawlerRequest|UpdateCrawlerInput' applications/web/sources --type=ts
# Look at the actual PUT request body construction
rg -nP -B3 -A5 'method.*PUT' applications/web/sources --type=tsRepository: gignac-cha/audio-underview
Length of output: 8681
🏁 Script executed:
# Check how the client constructs the update payload - where is it called from?
rg -nP -B10 'updateCrawler\(' applications/web/sources --type=ts | grep -A10 'CrawlerDetailPage\|crawler\.'
# Find CrawlerDetailPage to see how it constructs the update
fd -type f -name '*CrawlerDetailPage*' applications/web/sources
# Check the actual update logic - what fields are sent
rg -nP -B5 -A15 'type.*web.*name.*url_pattern' applications/web/sources --type=tsRepository: gignac-cha/audio-underview
Length of output: 2027
🏁 Script executed:
# Examine the exact validation and normalization logic in the worker
cat -n workers/crawler-manager-worker/sources/index.ts | sed -n '65,145p'
# Check if input_schema normalization happens in both create and update paths
rg -nP -B5 -A10 'body.input_schema.*body.*string' workers/crawler-manager-worker/sources/index.ts
# Verify if validateCrawlerBody is truly used for both POST and PUT
rg -nP 'validateCrawlerBody' workers/crawler-manager-worker/sources/index.ts | head -20Repository: gignac-cha/audio-underview
Length of output: 4282
🏁 Script executed:
# Check what handleUpdateCrawler does after validation - does it use normalized body?
cat -n workers/crawler-manager-worker/sources/index.ts | sed -n '245,280p'
# Check if there's any logic that preserves existing input_schema on PUT
rg -nP -B2 -A8 'handleUpdateCrawler' workers/crawler-manager-worker/sources/index.ts | head -50Repository: gignac-cha/audio-underview
Length of output: 2217
웹 크롤러 PUT 업데이트 시 input_schema 자동 정규화로 인한 데이터 손실
UpdateCrawlerWebInput에 input_schema 필드가 없는 것은 의도적 설계이지만, 서버의 검증 로직이 문제를 야기합니다. validateCrawlerBody 함수는 웹 크롤러에 대해 input_schema를 자동으로 { body: 'string' }으로 정규화합니다(index.ts:141-143). 이 정규화된 값이 handleUpdateCrawler에서 직접 데이터베이스 업데이트에 사용되므로(index.ts:267), 클라이언트가 input_schema를 전송하지 않은 PUT 요청 시에도 기존 값을 덮어쓰게 됩니다.
결과: 웹 크롤러를 업데이트할 때마다 input_schema가 항상 기본값으로 리셋되어, 동적으로 설정된 스키마 정보가 손실됩니다.
해결방안:
validateCrawlerBody를 POST(생성)와 PUT(업데이트)에서 다르게 처리하거나- PUT 요청 시 명시적으로 제공된 필드만 정규화하거나
UpdateCrawlerWebInput에input_schema를 선택적 필드로 추가하여 명시적으로 관리
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@applications/web/sources/hooks/use-crawler-manager.ts` around lines 25 - 43,
The issue is that validateCrawlerBody unconditionally normalizes input_schema to
{ body: 'string' } causing handleUpdateCrawler to overwrite existing schemas for
web crawlers; fix by making input_schema optional on UpdateCrawlerWebInput (add
input_schema?: Record<string, unknown>), change validateCrawlerBody to only
apply the default normalization for creation (POST) or when input_schema is
explicitly absent on POST, and update handleUpdateCrawler to only include
input_schema in the DB update if the request body actually contains
input_schema; reference UpdateCrawlerWebInput, UpdateCrawlerInput,
validateCrawlerBody, and handleUpdateCrawler when making these changes.
There was a problem hiding this comment.
리뷰 감사합니다. 제안 검토 결과 이 주장은 부정확하다고 판단되어 수용하지 않습니다.
Worker 동작 근거 (workers/crawler-manager-worker/sources/index.ts:141-143):
if (crawlerType === 'web') {
body.input_schema = { body: 'string' };
}validateCrawlerBody는 POST/PUT 공통 경로이며 web 타입일 때 무조건 body.input_schema를 { body: 'string' }으로 덮어씁니다. 이후 handleUpdateCrawler(line 262-269)가 normalized body로 DB를 업데이트합니다.
즉:
- 기존 web crawler의 DB
input_schema도 이미{ body: 'string' }(create 시점에 normalize됨) - 클라이언트가 input_schema를 보내든 안 보내든 서버가 덮어씀 → data loss 없음
현재 UpdateCrawlerWebInput이 input_schema 필드를 의도적으로 제외하는 이유:
- 클라이언트가 무시될 payload를 보내지 않도록 타입 수준에서 강제
- Worker의 normalize 행동을 타입으로 문서화
input_schema?를 옵셔널로 추가하면 오히려 "web에도 보낼 수 있다"는 잘못된 신호를 주므로 현재 엄격한 타입이 더 안전합니다.
There was a problem hiding this comment.
@gignac-cha, 상세한 설명 감사합니다. 지적이 정확합니다.
validateCrawlerBody가 POST/PUT 공통 경로에서 웹 타입에 대해 무조건 input_schema를 { body: 'string' }으로 덮어쓰기 때문에, 클라이언트가 해당 필드를 전송하지 않아도 데이터 손실이 발생하지 않는다는 점을 잘못 분석했습니다. UpdateCrawlerWebInput에서 input_schema를 의도적으로 제외하여 서버의 정규화 동작을 타입 수준에서 문서화하는 현재 설계가 더 안전한 접근 방식임을 인정합니다. 리뷰 코멘트를 철회합니다.
✏️ Learnings added
Learnt from: gignac-cha
Repo: gignac-cha/audio-underview PR: 24
File: applications/web/sources/hooks/use-crawler-manager.ts:25-43
Timestamp: 2026-04-20T11:50:26.268Z
Learning: In `applications/web/sources/hooks/use-crawler-manager.ts`, `UpdateCrawlerWebInput` intentionally excludes the `input_schema` field. The worker's `validateCrawlerBody` function unconditionally overwrites `input_schema` to `{ body: 'string' }` for web-type crawlers on both POST and PUT paths, so web crawler DB records always have this fixed value. Excluding `input_schema` from the type prevents clients from sending an ignored payload and documents the server-side normalization behavior at the type level.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: gignac-cha
Repo: gignac-cha/audio-underview PR: 13
File: applications/web/sources/hooks/use-crawler-code-runner.ts:31-31
Timestamp: 2026-02-13T20:48:39.602Z
Learning: In TypeScript/JavaScript files across the repository, follow the acronym casing rule: if an acronym (ID, URL, JSON, UUID, etc.) is used as a standalone parameter or variable name, use lowercase (e.g., url, id, json). If the acronym is part of a compound name with a prefix, use uppercase for the acronym part (e.g., baseURL, userID, parseJSON). This pattern is documented in CLAUDE.md and should be applied consistently across TS files.
- #1: replace setTimeout(50ms) with vi.waitFor for deterministic settle - #3: add invalid→valid JSON recovery path test (fill valid JSON → Save enabled → PUT fires) - #4: trim url_pattern before payload so leading/trailing whitespace is not persisted as part of the regex - #5: preserve in-progress edits via submittedForm ref comparison — if the user typed during the PUT round-trip, setForm keeps their draft instead of overwriting with the server response - #6: LoadingContainer gets role=status + aria-live=polite + aria-label; Spinner marked aria-hidden=true - #7: handleSchemaChange clears the inline schema error immediately when a previously invalid field becomes valid JSON (no longer waits for blur) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review fixup commit 9dc5064에서 이전 리뷰 findings 반영 완료:
Finding 2 (input_schema optional 제안)은 worker normalize 동작 ( 119 tests pass, lint clean. |
|
fixup commit에 대한 전체 리뷰를 진행하겠습니다. ✅ Actions performedReview triggered.
|
1 similar comment
|
fixup commit에 대한 전체 리뷰를 진행하겠습니다. ✅ Actions performedReview triggered.
|
Summary
/crawlers/:idroute + CrawlerDetailPage for viewing and editing (name, url_pattern, code, input/output schemas)useGetCrawler+useUpdateCrawlerhooks withskipToken-guarded detail queryUpdateCrawlerInputas web/data discriminated union matching worker contractstopPropagation+ aria-labelshowDefaultTemplate(off on detail page)SchemaHelperLinear Issue
Closes TES-27
Changes
/crawlers/:idProtectedRouteuseGetCrawler(skipToken),useUpdateCrawler(web/data union),exactinvalidationevent.stopPropagationshowDefaultTemplate?: booleanopt-in prop (default true; off on detail page)Review Fixes Applied
Pre-PR internal review (10 subagent passes) addressed:
UpdateCrawlerInputdiscriminated union; empty name/code/url_pattern client validationskipToken+ dedicated disabled queryKey (prevents empty-string collision)CodeEditorPanel.showDefaultTemplateopt-in prevents Monaco stale-template displayaria-invalid+ red borderdoes not fetch when id is undefinedsettle delay to prevent race false-positiveSchemaHelpershouldForwardPropfilter (DOM warning eliminated)useUpdateCrawlerinvalidation usesexact: true(no wasted detail refetch)input_schematextarea onBlur gated for web (readOnly type)Test plan
pnpm --filter @audio-underview/web test— 119 tests pass/crawlers, click a card — lands on/crawlers/:id🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
테스트