feat(extension): add Vapor QA Chrome extension#623
Open
MaxLee-dev wants to merge 10 commits into
Hidden character warning
The head ref may contain hidden characters: "feature/vapor-155-4-\ud575\uc2ec-\uac1c\ubc1c"
Open
Conversation
WXT가 자동 생성한 ~/* alias(extension 루트 기준)가 있었으나 코드는 전부 ../../utils/... 상대경로를 써왔다. 디렉토리 경계를 넘는 ../ import을 ~/utils/...로 전환해 상대경로 깊이 계산을 제거. ./ 형제 import은 유지. vitest는 .wxt/tsconfig.json의 alias를 읽지 못하므로 WxtVitest 플러그인을 꽂은 vitest.config.ts를 신설 — 테스트의 크로스 import도 동일하게 resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…kground and popup scripts
…logic with width parameter
- Adjusted memo bubble positioning to ensure it stays within viewport bounds. - Added unit tests for MemoBubble component to verify correct positioning behavior. - Updated ApiKeyForm and App components to use consistent padding styles. - Introduced RegisterBar tests to validate error handling and issue creation flow. - Enhanced session store to include tabId and page information for QA items. - Refactored image sharing logic to prevent reuse across different tabs and pages. - Improved messaging protocol to handle fiber responses more robustly. - Updated wxt configuration to require activeTab permission for better functionality.
|
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 22 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR adds an extension-based QA capture and review flow with shared storage and messaging, DOM and React Fiber utilities, Linear issue builders, background capture and lightbox handling, and popup and sidepanel screens. ChangesExtension QA workflow
Sequence Diagram(s)sequenceDiagram
participant InspectorUi
participant InspectorContent as "inspector.content/index.ts"
participant Background as "background.ts"
participant SessionStore as "session-store"
participant ImageStore as "image-store"
InspectorUi->>InspectorContent: onSaveMemo(memo)
InspectorContent->>Background: sendMessage('captureAndStore', ...)
Background->>SessionStore: getItems()
alt shared capture exists
Background-->>InspectorContent: { imageRef, index, tabId }
else new capture
Background->>ImageStore: putImage(dataUrlToBlob(...))
Background-->>InspectorContent: { imageRef, index: 1, tabId }
end
InspectorContent->>SessionStore: addItem({...})
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
apps/extension/package.json (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
compilescript.
compileduplicatestypecheck(both runtsc --noEmit). Having two aliases for the same command risks confusion about which to use in CI/docs. Keeptypecheckas the canonical name perCLAUDE.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/package.json` at line 8, The package.json scripts include a redundant compile alias that runs the same tsc --noEmit command as typecheck, so remove the compile entry and keep typecheck as the single canonical script name. Update the scripts block in package.json to avoid duplicate aliases and ensure any references align with typecheck.apps/extension/utils/data/image-store.test.ts (1)
11-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one automated round-trip test for the IndexedDB path.
The current suite only exercises pure conversions, so a regression in
putImage/getImage/clearImageswould surface only in the integrated extension flow. Covering one real persistence round trip here would materially increase confidence in this module’s core behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/utils/data/image-store.test.ts` around lines 11 - 24, The current tests only verify conversion helpers, so add one automated IndexedDB round-trip test that exercises the real persistence flow through putImage, getImage, and clearImages in image-store.test.ts. Use the existing image-store APIs to store a Blob or data URL, retrieve it back, assert the persisted payload matches, and then clear it to confirm cleanup works. Keep the new test aligned with the existing helper-focused style, but make it cover the actual IDB-backed path end to end.apps/extension/utils/linear/build-issue.test.ts (1)
92-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that
head.widthis forwarded toannotateImage.These tests prove the data-URL/no-upload path, but not the scaling contract. If
buildDescription()stops passinghead.width, Linear screenshots regress to misaligned boxes and this suite still stays green.Suggested assertion
const description = await buildDescription( [item({ imageRef: 'image-1', width: 1200 })], {}, ); + expect(mocks.annotateImage).toHaveBeenCalledWith( + expect.any(Blob), + expect.any(Array), + 1200, + ); expect(mocks.uploadImage).not.toHaveBeenCalled(); expect(description).toContain('');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/utils/linear/build-issue.test.ts` around lines 92 - 106, The `buildDescription()` test covers the no-upload data-URL path but does not verify the scaling input passed into `annotateImage`. Update the test around `buildDescription`, `mocks.annotateImage`, and the `item({ imageRef, width })` setup to assert that the `head.width` value is forwarded as the width argument when `annotateImage` is called. Keep the existing no-upload assertions and add a check on the `annotateImage` mock call so this contract cannot regress silently.apps/extension/utils/dom/style-extract.test.ts (1)
13-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert computed values too.
This only checks key presence/count, so
extractStyle()could return empty strings for every property and still pass. Add at least thecolorandfont-sizeassertions from the setup.Suggested test tightening
const style = extractStyle(el); + expect(style.color).toBe('rgb(255, 0, 0)'); + expect(style['font-size']).toBe('14px'); expect(style).toHaveProperty('color'); expect(style).toHaveProperty('font-size'); expect(style).toHaveProperty('padding-top'); expect(style).toHaveProperty('display');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/utils/dom/style-extract.test.ts` around lines 13 - 20, The test for extractStyle() only verifies that style keys exist and that the total key count matches, so it can still pass even if the returned values are empty. Tighten the assertions in style-extract.test.ts by checking the computed values for the properties seeded in the setup, especially color and font-size, alongside the existing property presence checks. Use extractStyle() and the style object in the test to assert the expected concrete values rather than only key existence.apps/extension/utils/linear/index.ts (1)
8-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding a request timeout to the GraphQL fetch.
gqlhas noAbortController/timeout, so a stalled connection leavesverifyKey/listTeams/createIssuepending indefinitely, with no recovery path for the popup/sidepanel UIs that await them.♻️ Proposed timeout via AbortSignal
const gql = async <T>(key: string, query: string, variables?: object): Promise<T> => { const res = await fetch(ENDPOINT, { method: 'POST', // Linear Personal API Key는 Authorization 헤더에 값 그대로. Bearer 접두사 없음. headers: { 'Content-Type': 'application/json', Authorization: key }, body: JSON.stringify({ query, variables }), + signal: AbortSignal.timeout(15_000), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/extension/utils/linear/index.ts` around lines 8 - 23, The gql helper currently has no timeout or abort handling, so requests used by verifyKey, listTeams, and createIssue can hang forever. Update gql to use an AbortController-based timeout for the fetch call, and make sure timeout-triggered aborts surface as a clear error path that the callers can handle. Keep the change localized to the gql function so all Linear GraphQL requests inherit the same recovery behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/extension/entrypoints/fiber-reader.ts`:
- Line 16: Restrict the `window.postMessage` call in `fiber-reader` so it no
longer uses the wildcard target origin; update the `FIBER_RESPONSE` response
path to send to `window.location.origin` instead of `'*'`. Keep the existing
`requestId` and `components` payload unchanged, and make sure the same-window
request/response flow still works while satisfying origin validation.
In `@apps/extension/entrypoints/inspector.content/index.ts`:
- Around line 94-97: The capture sequence in the inspector content flow still
risks grabbing the overlay/FAB because a single nextFrame call may happen before
the hide changes are painted. Update the capture path in index.ts around
overlay?.setVisible(false), uiHost.style.display, and the await nextFrame() wait
so it uses an after-paint delay such as a double rAF before captureVisibleTab is
called.
In `@apps/extension/entrypoints/inspector.content/inspector-ui.tsx`:
- Around line 33-45: The MemoBubble in inspector-ui.tsx is being reused when pin
changes, so partially typed memo state can leak across targets. Update the
MemoBubble render so it remounts whenever the pinned target changes by giving it
a stable key derived from the current pin target identity (not just the
component presence), and keep the existing onSave/onCancel behavior in the
Inspector UI path.
In `@apps/extension/entrypoints/inspector.content/lightbox.ts`:
- Around line 13-15: The lightbox replacement path in showLightbox removes the
existing DOM node directly, which skips the cleanup done by close() and leaves
resize/keydown listeners behind. Update showLightbox to tear down any existing
lightbox by invoking the same close/cleanup flow used by close() before creating
the new lightbox, so reopening the inspector lightbox does not accumulate
handlers.
In `@apps/extension/entrypoints/inspector.content/memo-bubble.test.tsx`:
- Around line 12-17: The test in memo-bubble.test.tsx mutates the global
window.innerHeight and leaves it changed for later tests. Update the existing
afterEach alongside document.body.replaceChildren() to restore
window.innerHeight after the memo bubble viewport test, using the same test
setup around innerHeight so the global browser state does not leak between
happy-dom tests.
In `@apps/extension/entrypoints/popup/useApiKey.ts`:
- Around line 28-50: The key-loading flow in useApiKey() only handles the
successful keyStore.getValue() path, so a rejection leaves status stuck at
loading and the popup spinner never exits. Add error handling to the getValue()
promise chain in useApiKey() so any rejection falls back to setStatus('needKey')
(and optionally clears any stale key state) while still respecting the active
guard, keeping App.tsx from rendering the spinner indefinitely.
In `@apps/extension/entrypoints/sidepanel/ItemCard.tsx`:
- Around line 12-28: The ItemCard preview effect is leaving stale object URLs in
state when imageRef changes or when getImage returns no blob. Update the
useEffect in ItemCard to clear the current url before starting a new lookup, so
the old screenshot is removed immediately when the reference changes. Keep the
existing object URL cleanup in the effect teardown and make sure the reset
happens even when imageRef is empty or the fetch result is missing.
In `@apps/extension/entrypoints/sidepanel/RegisterBar.tsx`:
- Around line 27-48: The RegisterBar useEffect that calls listTeams(apiKey) is
preserving the previous teams and teamId while a new API key is loading, which
can leave a stale team selected. In the RegisterBar component, reset the current
team-related state as soon as apiKey changes by clearing teams and teamId before
or when starting the listTeams request, then repopulate them from the latest
response so submit uses only the current credential’s team selection.
In `@apps/extension/utils/data/session-store.ts`:
- Around line 33-41: The addItem helper is doing a non-atomic read-modify-write
against itemsStore, which can drop entries when multiple extension contexts save
at the same time. Update addItem to use an atomic write path or serialize all
mutations through a single owner so concurrent appends cannot overwrite each
other. Keep the fix localized to addItem and the itemsStore access pattern in
session-store.ts.
In `@apps/extension/utils/linear/image-sharing.ts`:
- Around line 15-28: The viewport lookup in the image-sharing helper is too
permissive when multiple existing items share the same `tabId`, `pageUrl`,
`scrollX`, `scrollY`, and `width` but have different `imageRef`s. Update the
logic in the function that builds `sameViewport` so it detects more than one
distinct `imageRef` and returns `null` instead of picking the first ref and
mixing `nextIndex` values; keep sharing only when all matching items point to
the same screenshot reference.
---
Nitpick comments:
In `@apps/extension/package.json`:
- Line 8: The package.json scripts include a redundant compile alias that runs
the same tsc --noEmit command as typecheck, so remove the compile entry and keep
typecheck as the single canonical script name. Update the scripts block in
package.json to avoid duplicate aliases and ensure any references align with
typecheck.
In `@apps/extension/utils/data/image-store.test.ts`:
- Around line 11-24: The current tests only verify conversion helpers, so add
one automated IndexedDB round-trip test that exercises the real persistence flow
through putImage, getImage, and clearImages in image-store.test.ts. Use the
existing image-store APIs to store a Blob or data URL, retrieve it back, assert
the persisted payload matches, and then clear it to confirm cleanup works. Keep
the new test aligned with the existing helper-focused style, but make it cover
the actual IDB-backed path end to end.
In `@apps/extension/utils/dom/style-extract.test.ts`:
- Around line 13-20: The test for extractStyle() only verifies that style keys
exist and that the total key count matches, so it can still pass even if the
returned values are empty. Tighten the assertions in style-extract.test.ts by
checking the computed values for the properties seeded in the setup, especially
color and font-size, alongside the existing property presence checks. Use
extractStyle() and the style object in the test to assert the expected concrete
values rather than only key existence.
In `@apps/extension/utils/linear/build-issue.test.ts`:
- Around line 92-106: The `buildDescription()` test covers the no-upload
data-URL path but does not verify the scaling input passed into `annotateImage`.
Update the test around `buildDescription`, `mocks.annotateImage`, and the
`item({ imageRef, width })` setup to assert that the `head.width` value is
forwarded as the width argument when `annotateImage` is called. Keep the
existing no-upload assertions and add a check on the `annotateImage` mock call
so this contract cannot regress silently.
In `@apps/extension/utils/linear/index.ts`:
- Around line 8-23: The gql helper currently has no timeout or abort handling,
so requests used by verifyKey, listTeams, and createIssue can hang forever.
Update gql to use an AbortController-based timeout for the fetch call, and make
sure timeout-triggered aborts surface as a clear error path that the callers can
handle. Keep the change localized to the gql function so all Linear GraphQL
requests inherit the same recovery behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a9e53a8c-479c-4566-a89a-2271a6f6d324
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (56)
.prettierignoreapps/extension/.gitignoreapps/extension/CLAUDE.mdapps/extension/entrypoints/background.tsapps/extension/entrypoints/fiber-reader.tsapps/extension/entrypoints/inspector.content/fab.tsxapps/extension/entrypoints/inspector.content/index.tsapps/extension/entrypoints/inspector.content/inspector-ui.tsxapps/extension/entrypoints/inspector.content/lightbox.tsapps/extension/entrypoints/inspector.content/memo-bubble.test.tsxapps/extension/entrypoints/inspector.content/memo-bubble.tsxapps/extension/entrypoints/inspector.content/overlay.test.tsapps/extension/entrypoints/inspector.content/overlay.tsapps/extension/entrypoints/popup/ApiKeyForm.tsxapps/extension/entrypoints/popup/App.tsxapps/extension/entrypoints/popup/index.htmlapps/extension/entrypoints/popup/main.tsxapps/extension/entrypoints/popup/useApiKey.tsapps/extension/entrypoints/popup/useInspecting.tsapps/extension/entrypoints/popup/useQaSession.tsapps/extension/entrypoints/sidepanel/App.tsxapps/extension/entrypoints/sidepanel/ItemCard.tsxapps/extension/entrypoints/sidepanel/RegisterBar.test.tsxapps/extension/entrypoints/sidepanel/RegisterBar.tsxapps/extension/entrypoints/sidepanel/index.htmlapps/extension/entrypoints/sidepanel/main.tsxapps/extension/entrypoints/sidepanel/useQaItems.tsapps/extension/entrypoints/sidepanel/useStoredApiKey.tsapps/extension/eslint.config.mjsapps/extension/package.jsonapps/extension/tsconfig.jsonapps/extension/utils/browser/active-tab.tsapps/extension/utils/browser/background.test.tsapps/extension/utils/data/image-store.test.tsapps/extension/utils/data/image-store.tsapps/extension/utils/data/session-store.tsapps/extension/utils/dom/annotate-image.test.tsapps/extension/utils/dom/annotate-image.tsapps/extension/utils/dom/fiber-component.test.tsapps/extension/utils/dom/fiber-component.tsapps/extension/utils/dom/selector.tsapps/extension/utils/dom/style-extract.test.tsapps/extension/utils/dom/style-extract.tsapps/extension/utils/linear/build-issue.test.tsapps/extension/utils/linear/build-issue.tsapps/extension/utils/linear/group-by-image.test.tsapps/extension/utils/linear/group-by-image.tsapps/extension/utils/linear/image-sharing.test.tsapps/extension/utils/linear/image-sharing.tsapps/extension/utils/linear/index.tsapps/extension/utils/messaging/fiber-protocol.test.tsapps/extension/utils/messaging/fiber-protocol.tsapps/extension/utils/messaging/index.tsapps/extension/vitest.config.tsapps/extension/wxt.config.tsturbo.json
Comment on lines
+33
to
+41
| export const addItem = async (item: Omit<QaItem, 'id' | 'createdAt'>): Promise<QaItem> => { | ||
| const entry: QaItem = { | ||
| ...item, | ||
| id: crypto.randomUUID(), | ||
| createdAt: Date.now(), | ||
| }; | ||
|
|
||
| const current = await itemsStore.getValue(); | ||
| await itemsStore.setValue([...current, entry]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make appends atomic across contexts.
addItem() does a read-modify-write on local:qaItems. If two saves overlap, both can read the same array and the later setValue() drops the earlier entry. In this PR the store is shared across multiple extension contexts, so this can surface as missing QA items under concurrent saves. Serialize writes through one owner or switch this helper to an atomic update path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/utils/data/session-store.ts` around lines 33 - 41, The addItem
helper is doing a non-atomic read-modify-write against itemsStore, which can
drop entries when multiple extension contexts save at the same time. Update
addItem to use an atomic write path or serialize all mutations through a single
owner so concurrent appends cannot overwrite each other. Keep the fix localized
to addItem and the itemsStore access pattern in session-store.ts.
Comment on lines
+15
to
+28
| const sameViewport = items.filter( | ||
| (item) => | ||
| item.tabId === tabId && | ||
| item.pageUrl === pageUrl && | ||
| item.scrollX === scrollX && | ||
| item.scrollY === scrollY && | ||
| item.width === width && | ||
| item.imageRef != null, | ||
| ); | ||
| if (sameViewport.length === 0) return null; | ||
|
|
||
| const imageRef = sameViewport[0].imageRef!; | ||
| const nextIndex = Math.max(...sameViewport.map((item) => item.index ?? 0)) + 1; | ||
| return { imageRef, nextIndex }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Refuse ambiguous viewport matches.
If sameViewport already contains two distinct imageRefs, this returns the first ref but computes nextIndex across both groups. The next item then gets attached to an arbitrary screenshot with numbering taken from another one. Detect multiple refs and return null so the caller captures a fresh image instead.
Possible fix
if (sameViewport.length === 0) return null;
- const imageRef = sameViewport[0].imageRef!;
+ const imageRefs = new Set(sameViewport.map((item) => item.imageRef));
+ if (imageRefs.size !== 1) return null;
+
+ const [imageRef] = [...imageRefs] as string[];
const nextIndex = Math.max(...sameViewport.map((item) => item.index ?? 0)) + 1;
return { imageRef, nextIndex };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const sameViewport = items.filter( | |
| (item) => | |
| item.tabId === tabId && | |
| item.pageUrl === pageUrl && | |
| item.scrollX === scrollX && | |
| item.scrollY === scrollY && | |
| item.width === width && | |
| item.imageRef != null, | |
| ); | |
| if (sameViewport.length === 0) return null; | |
| const imageRef = sameViewport[0].imageRef!; | |
| const nextIndex = Math.max(...sameViewport.map((item) => item.index ?? 0)) + 1; | |
| return { imageRef, nextIndex }; | |
| const sameViewport = items.filter( | |
| (item) => | |
| item.tabId === tabId && | |
| item.pageUrl === pageUrl && | |
| item.scrollX === scrollX && | |
| item.scrollY === scrollY && | |
| item.width === width && | |
| item.imageRef != null, | |
| ); | |
| if (sameViewport.length === 0) return null; | |
| const imageRefs = new Set(sameViewport.map((item) => item.imageRef)); | |
| if (imageRefs.size !== 1) return null; | |
| const [imageRef] = [...imageRefs] as string[]; | |
| const nextIndex = Math.max(...sameViewport.map((item) => item.index ?? 0)) + 1; | |
| return { imageRef, nextIndex }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/extension/utils/linear/image-sharing.ts` around lines 15 - 28, The
viewport lookup in the image-sharing helper is too permissive when multiple
existing items share the same `tabId`, `pageUrl`, `scrollX`, `scrollY`, and
`width` but have different `imageRef`s. Update the logic in the function that
builds `sameViewport` so it detects more than one distinct `imageRef` and
returns `null` instead of picking the first ref and mixing `nextIndex` values;
keep sharing only when all matching items point to the same screenshot
reference.
Source: Coding guidelines
noahchoii
approved these changes
Jul 9, 2026
noahchoii
left a comment
noahchoii
left a comment
Contributor
There was a problem hiding this comment.
제가 AI를 이용해서 코드를 작성하면서 많이 체감했던 부분인데, 아무래도 인간보다 엣지 케이스에 대해 더 고려할 수 있는 부분이 많기 때문에 그런지 if 분기문이 점점 많아지는 것 같아요. 또 삼항연산자도 점점 많아지고 삼항연산자를 중첩으로 사용하는 경우도 꽤나 자주 발생했던 것 같고요. 그래서 코드를 읽을 때 인지 부하가 더욱 커지는 것 같습니다..!
그래서 우리도 앞으로 이런 식의 제품을 제작할 일이 많아질텐데, 이전에 Gemini codereview를 이용했던 것처럼 공용으로 사용할 컨벤션 스킬을 만들어둘 필요가 있겠다는 생각이 들었습니다!
--
폴더명이 extension이라고만 되어 있는데, qa-extensions처럼 이름을 확실히 구분할 수 있도록 수정하는 것은 어떨까요?
--
코드양이 많기도 하고, 어차피 우리가 사용할 도구라서 일단 추가 후 도그푸딩할 겸 우선 승인해두겠습니다!
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
2 participants
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.
Related Issues
Description of Changes
Implements Vapor QA, a Chrome (WXT + React) extension for visual design QA: pin UI elements on any page, leave inline memos, and file them as a single Linear issue. This is the initial feature drop for the QA extension under
apps/extension/.The extension is split across four execution contexts that communicate via
@webext-core/messaging:captureVisibleTab· IndexedDB image store · message routinginspector.content/— overlay/memo/capture (isolated world) + React Fiber ancestry reading (MAIN world viafiber-reader.ts)Key capabilities:
*Context/*Providerwrappers excluded).tabId/pageUrl/scrollX/scrollY/widthviewport.A few non-obvious architectural decisions are documented in
apps/extension/CLAUDE.md(capture lives in background for origin reasons; Fiber is read only in MAIN world; inspecting state is a single in-memory variable, so a page reload silently turns it off — by design).Screenshots
Checklist
utils/and content-script helpers —vitest.)apps/extension/CLAUDE.md+docs/.)Summary by CodeRabbit
New Features
Bug Fixes