Stg#245
Merged
Merged
Conversation
…V1EOQ main.tf dev space remove
- flake.nix に opentofu パッケージを追加し、dev shell から `tofu` を提供
- .github/workflows/terraform-ci.yml を opentofu-ci.yml にリネームし、
hashicorp/setup-terraform を opentofu/setup-opentofu@v1 に差し替え
- 旧 .terraform.lock.hcl を削除（初回 tofu init で registry.opentofu.org
ベースに再生成する想定）
- Makefile に infra-fmt / infra-validate-{dev,stg,prod} を追加
- .claude/rules/infra/terraform.md を opentofu.md にリネームし内容更新
- README / infra/README / runbook の terraform 表記を tofu / OpenTofu に更新
GCS backend 互換のため state はそのまま流用可能。Branch protection の
status check 名は terraform-* から opentofu-* に変わる点に注意。
BE/FE refactaring
📝 WalkthroughWalkthroughUnifies async-task orchestration with handler/service patterns, consolidates intelligence taxonomy and pipeline (adds GitHub analysis service), moves large seed data to JSON, updates schemas/migrations, extracts/refactors frontend components and hooks, and refreshes docs, configs, and tests. ChangesEnd-to-end Task Orchestration and Intelligence Updates
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (10)
flake.nix (1)
55-56: ⚡ Quick winRemove the duplicated
opentofupackage entry.
opentofuis already declared at Lines 52-53, so this duplicate block should be removed.Suggested fix
- # --- IaC --- - opentofu # OpenTofu CLI（Terraform 互換 / インフラ管理）🤖 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 `@flake.nix` around lines 55 - 56, There is a duplicate package entry for opentofu in the package list; remove the second occurrence so only one opentofu entry remains (search for the symbol "opentofu" in the package list and delete the duplicate block), then re-run the flake build/check to verify no duplicates remain.frontend/src/components/blog/BlogArticleList.tsx (1)
46-53: ⚡ Quick winExpose active filter state to assistive tech.
These filter buttons are stateful controls; adding
aria-pressedwill make the selected state explicit for screen readers.♿ Proposed update
<button key={f} type="button" className={`${styles.filterTab} ${filter === f ? styles.filterTabActive : ""}`} + aria-pressed={filter === f} onClick={() => handleFilterChange(f)} >🤖 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 `@frontend/src/components/blog/BlogArticleList.tsx` around lines 46 - 53, The filter buttons in BlogArticleList are stateful and need an accessible pressed state; update the button elements (the ones using key={f}, className with styles.filterTab and styles.filterTabActive, and onClick={() => handleFilterChange(f)}) to include aria-pressed set to a boolean expression reflecting the active filter (e.g., aria-pressed={filter === f}) so screen readers can announce the selected state; keep the existing class-based visual styling and click handler unchanged.backend/app/services/intelligence/github_analysis_service.py (3)
37-40: ⚡ Quick winConsider raising an exception for missing cache.
The function returns early when the cache is not found, logging an error. This might silently hide data consistency issues. Consider raising a
RuntimeErroror custom exception to ensure the caller is aware of the missing cache, as this represents an unexpected state that could indicate a race condition or data corruption.💡 Proposed change
cache = db.query(GitHubAnalysisCache).filter_by(user_id=user_id).first() if not cache: logger.error("GitHub 分析キャッシュが見つかりません", extra={"user_id": user_id}) - return + raise RuntimeError(f"GitHub analysis cache not found for user_id={user_id}")🤖 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 `@backend/app/services/intelligence/github_analysis_service.py` around lines 37 - 40, The code silently returns when the GitHubAnalysisCache lookup fails; instead of returning, raise a descriptive exception so callers cannot ignore this unexpected state. Replace the early return after the db.query(GitHubAnalysisCache).filter_by(user_id=user_id).first() check: log the error with user_id via logger.error as before and then raise a RuntimeError (or a project-specific custom exception) that includes the user_id and a short message like "GitHub analysis cache not found" so upstream callers can handle or fail loudly; ensure this change is applied to the function containing the cache lookup and update any callers/tests that expect a None return if needed.
119-121: ⚡ Quick winCatch specific exceptions instead of bare except.
The bare
except Exception:on line 119 will catch all exceptions, potentially hiding bugs or masking unexpected failure modes (e.g., database errors, network issues, programming errors). Consider catching specific expected exceptions (e.g.,LLMError,TimeoutError,ConnectionError) and re-raising unexpected ones after logging, or at minimum useexc_info=Trueto ensure stack traces are logged for debugging.🔍 Suggested improvement
- except Exception: + except Exception as e: logger.warning("学習アドバイスの生成に失敗しましたが、分析結果は保存します", exc_info=True) + # Consider re-raising unexpected exceptions in non-production environments + # if isinstance(e, (KeyboardInterrupt, SystemExit)): + # raise return None, True🤖 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 `@backend/app/services/intelligence/github_analysis_service.py` around lines 119 - 121, Replace the bare "except Exception:" in github_analysis_service.py (the block that logs "学習アドバイスの生成に失敗しましたが、分析結果は保存します") with specific exception handlers for expected errors (e.g., except (LLMError, TimeoutError, ConnectionError) as e: logger.warning(..., exc_info=True); return None, True) and add a final broad handler that logs and re-raises unexpected exceptions (e.g., except Exception as e: logger.exception("Unexpected error generating advice", exc_info=True); raise) so only known failures are swallowed while truly unexpected errors bubble up.
86-91: ⚖️ Poor tradeoffClarify error_message usage for completed status with LLM failure.
The function sets
status="completed"anderror_message="LLM処理が利用できません"when analysis succeeds but LLM advice generation fails. This overloadserror_messageto serve both as a failure indicator and a warning message. Consider whether a separate field (e.g.,warning_messageorllm_status) would better distinguish "analysis failed" from "analysis succeeded with partial features unavailable."🤖 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 `@backend/app/services/intelligence/github_analysis_service.py` around lines 86 - 91, The code currently sets cache.status = "completed" while using cache.error_message to record a non-fatal LLM failure (llm_failed), which conflates errors with warnings; add a distinct field (e.g., cache.warning_message or cache.llm_status) to represent non-fatal LLM issues and populate that when llm_failed is true, leaving cache.error_message for true failures only; update the DB model/migrations to add the new column, change the save logic in the block where cache.status, cache.error_message, cache.completed_at are set (referencing cache.status, cache.error_message, cache.completed_at, llm_failed), and adjust any consumers/readers of cache.error_message to treat the new field for LLM warnings.backend/app/services/tasks/worker.py (1)
30-34: 💤 Low valueMove
import timeto module level.The
timemodule is imported twice: inside_monotonic_ms_since()(line 32) and insideexecute_task()(line 51). Move the import to module level for clarity and slight performance improvement.+import time from datetime import datetime, timezone from sqlalchemy.orm import Session ... def _monotonic_ms_since(start: float) -> int: """``time.monotonic()`` の開始時点からの経過ミリ秒を返す。""" - import time return int((time.monotonic() - start) * 1000) async def execute_task(...) -> None: ... - import time - user_id = payload.get("user_id", "unknown")Also applies to: 51-52
🤖 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 `@backend/app/services/tasks/worker.py` around lines 30 - 34, Move the repeated local imports of the time module to the top-level of the module: add a single "import time" at module level and remove the inline "import time" statements inside _monotonic_ms_since and execute_task, so _monotonic_ms_since(start: float) and execute_task() both use the module-level time reference.backend/tests/test_intelligence.py (2)
259-278: 💤 Low valueConsider moving imports to module level.
The inline imports within each test method work but are unconventional. Moving these to the module's import section would improve consistency and make dependencies clearer.
+from app.services.intelligence.github.repo_analyzer import parse_go_mod, parse_pom_xml + class TestDependencyParsing: def test_parse_pom_xml(self): - from app.services.intelligence.github.repo_analyzer import parse_pom_xml - content = ( ... ) result = parse_pom_xml(content) ... def test_parse_go_mod(self): - from app.services.intelligence.github.repo_analyzer import parse_go_mod - content = ( ... ) result = parse_go_mod(content) ...🤖 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 `@backend/tests/test_intelligence.py` around lines 259 - 278, The tests currently perform inline imports inside test_parse_pom_xml and test_parse_go_mod; move these to the module level by adding a single import statement like "from app.services.intelligence.github.repo_analyzer import parse_pom_xml, parse_go_mod" at the top of the test file and remove the inline imports from the two test functions so the tests call parse_pom_xml(...) and parse_go_mod(...) directly.
348-348: 💤 Low valueUse
asyncio.run()instead of deprecatedget_event_loop().run_until_complete().
asyncio.get_event_loop()is deprecated in Python 3.10+ when no running loop exists. Useasyncio.run()for cleaner async test execution.- result = asyncio.get_event_loop().run_until_complete(run_pipeline(username="testuser")) + result = asyncio.run(run_pipeline(username="testuser"))🤖 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 `@backend/tests/test_intelligence.py` at line 348, The test uses deprecated asyncio.get_event_loop().run_until_complete to run the coroutine; replace that call with asyncio.run to execute the async coroutine directly. Specifically, change the invocation that assigns result (currently calling asyncio.get_event_loop().run_until_complete(run_pipeline(username="testuser"))) to use asyncio.run(run_pipeline(username="testuser")) so run_pipeline is executed with the modern API and result remains the returned value.backend/app/db/bootstrap.py (1)
8-39: ⚡ Quick winKeep only one
_validate_jwt_keysdefinition.Line 8 introduces a duplicate of the same function redefined again at Line 25. The first definition is overwritten and should be removed.
🧹 Suggested cleanup
-def _validate_jwt_keys() -> None: - """起動時に JWT 鍵ペアの整合性を検証する。署名 → 検証が通らない場合は RuntimeError を送出する。""" - from jose import JWTError, jwt - - from ..core.settings import get_jwt_private_key, get_jwt_public_key - - try: - priv = get_jwt_private_key() - pub = get_jwt_public_key() - tok = jwt.encode({"sub": "__bootstrap_check__"}, priv, algorithm="RS256") - jwt.decode(tok, pub, algorithms=["RS256"]) - except JWTError as e: - raise RuntimeError( - f"JWT 鍵ペアが不正です（秘密鍵と公開鍵が一致しないか、フォーマットが壊れています）: {e}" - ) from e - - def _validate_jwt_keys() -> None:🤖 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 `@backend/app/db/bootstrap.py` around lines 8 - 39, Remove the duplicate function definition of _validate_jwt_keys so there is only a single implementation; locate both definitions (they both import get_jwt_private_key and get_jwt_public_key and use jose.jwt/JWTError) and delete one of them, keeping the other intact (ensure the surviving _validate_jwt_keys still imports from ..core.settings and uses jwt.encode/jwt.decode as shown).frontend/src/hooks/useBlogAccountManager.test.ts (1)
227-264: ⚡ Quick winRemove the duplicated
handleUpdatetest case.Line 227 repeats the same test name and assertions already defined at Line 188, so this is redundant and makes failures noisier.
✂️ Suggested cleanup
- it("handleUpdate を呼ぶと updateBlogAccount が呼ばれ未同期状態に更新される", async () => { - api.getBlogAccounts - .mockResolvedValueOnce(dummyAccounts) - .mockResolvedValueOnce([ - { - ...dummyAccounts[0], - username: "updated-user", - last_synced_at: null, - }, - ]); - api.getBlogArticles - .mockResolvedValueOnce(dummyArticles) - .mockResolvedValueOnce([]); - api.updateBlogAccount.mockResolvedValue({ - ...dummyAccounts[0], - username: "updated-user", - last_synced_at: null, - }); - - const { result } = renderHook(() => useBlogAccountManager("all")); - - await waitFor(() => { - expect(result.current.loading).toBe(false); - }); - - let updated = false; - await act(async () => { - updated = await result.current.handleUpdate("zenn", "updated-user"); - }); - - expect(updated).toBe(true); - expect(api.updateBlogAccount).toHaveBeenCalledWith("zenn", "updated-user"); - await waitFor(() => { - expect(result.current.accounts[0]?.username).toBe("updated-user"); - }); - expect(result.current.accounts[0]?.last_synced_at).toBeNull(); - expect(result.current.success).toBe("usernameを更新しました。再同期してください。"); - });🤖 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 `@frontend/src/hooks/useBlogAccountManager.test.ts` around lines 227 - 264, Remove the duplicated test block that re-tests handleUpdate in the useBlogAccountManager tests: delete the redundant it("handleUpdate を呼ぶと updateBlogAccount が呼ばれ未同期状態に更新される", ...) block so only the original test remains (the duplicate repeats assertions for api.updateBlogAccount, result.current.handleUpdate, accounts[0]?.username/last_synced_at and result.current.success); keep the first occurrence and remove the later one to avoid noisy, redundant failures and run the test suite to verify.
🤖 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 @.claude/rules/infra/terraform.md:
- Around line 8-12: The fenced code block that shows the infra directory tree
(the block containing "infra/ ├── modules/ ..." in
.claude/rules/infra/terraform.md) is missing a language identifier and triggers
markdownlint MD040; update the opening fence from ``` to ```text (or another
appropriate language like ```bash) so the fence becomes ```text and leave the
closing fence unchanged, ensuring the snippet renders and satisfies the linter.
In `@backend/app/db/seeds/technology_stacks.json`:
- Around line 613-621: The JSON seed data contains conflicting global sort_order
values: middleware entries "Nginx" (sort_order: 121) and "Apache" (122) collide
with monitoring entries "cloudWatch" and "cloudMonitoring"; decide whether
sort_order is global or per-category and, if it must be globally unique, update
the "sort_order" for "Nginx" and "Apache" (or alternatively adjust
"cloudWatch"/"cloudMonitoring") to non-colliding integers so all entries have
unique sort_order values, preserving desired display order and keeping the
unique identifiers "Nginx", "Apache", "cloudWatch", and "cloudMonitoring"
intact.
In `@backend/app/routers/blog.py`:
- Around line 271-276: The current creation flow using
BlogSummaryCacheRepository(db, user.id).get() then creating BlogSummaryCache and
db.flush() can race under concurrent requests; change to a get-or-create
pattern: attempt to fetch via BlogSummaryCacheRepository.get(), if missing
create a BlogSummaryCache and db.add() but wrap the commit/flush in a try/except
catching sqlalchemy.exc.IntegrityError (relying on a unique constraint on
BlogSummaryCache.user_id), on IntegrityError rollback the session and re-query
BlogSummaryCacheRepository.get() to obtain the existing row, then instantiate
AsyncTaskCacheService(db, cache) with the resolved cache; ensure the unique
constraint exists for BlogSummaryCache.user_id and use those symbols
(BlogSummaryCacheRepository.get, BlogSummaryCache, db.flush/db.commit,
IntegrityError, AsyncTaskCacheService) when making the change.
In `@backend/app/services/intelligence/position_weights.json`:
- Line 83: Replace the incorrect mapping for the "Git workflow" skill (currently
mapped to ["CI/CD"]) so it scores against git/version-control related signals
instead of continuous-integration; update the value for the "Git workflow" key
in position_weights.json from ["CI/CD"] to a more appropriate array such as
["git", "version control"] (or other repo/git-related skill tokens used
elsewhere) so candidate git experience is evaluated correctly.
In `@backend/app/services/intelligence/skill_extractor.py`:
- Around line 126-128: The loop over repo.detected_frameworks currently calls
add(framework, "root_file"), mislabeling dependency-derived frameworks; update
the add invocation in the loop that iterates repo.detected_frameworks to use the
dependency provenance label (e.g., add(framework, "dependency") or
"dependency_derived") instead of "root_file" so the recorded source correctly
reflects dependency-derived frameworks.
In `@backend/app/services/tasks/dispatch_service.py`:
- Around line 54-75: is_in_progress() + reset_to_pending() allow a TOCTOU race;
replace them with a single transactional conditional transition method (e.g.,
transition_to_pending_if_in_progress or reset_to_pending_atomic) that acquires a
row lock or performs a conditional UPDATE inside a DB transaction using self._db
(use SELECT ... FOR UPDATE or UPDATE ... WHERE id=? AND status IN
('pending','processing') RETURNING id), checks the current status atomically,
sets status="pending", clears error_message and optionally resets
retry_count/started_at/completed_at based on a reset_retry_count flag, commits
only on success, and returns a boolean indicating whether the transition
occurred; call this new atomic method instead of calling is_in_progress() then
reset_to_pending().
In `@backend/app/services/tasks/handlers/blog_summarize.py`:
- Around line 36-37: The handler reads payload["user_id"] without checking
presence which can raise KeyError; change the handler in blog_summarize.py to
validate payload contains a user_id before indexing: use payload.get("user_id")
or an explicit if "user_id" not in payload check, log/return/raise a clear error
(instead of letting KeyError bubble), and only call self.get_record(db, payload)
after the user_id is validated so downstream logic (get_record, db operations)
always receives a well-formed payload.
In `@backend/app/services/tasks/handlers/career_analysis.py`:
- Line 59: In the except block in career_analysis.py (the handler where you
currently use "raise exc"), replace the explicit "raise exc" with a bare "raise"
to re-raise the currently handled exception; locate the try/except around the
code that catches the exception (the block ending with "raise exc") and change
it so the except simply uses "raise" to preserve the original traceback and
follow Python convention.
In `@backend/app/services/tasks/worker.py`:
- Around line 177-195: The three async handlers _run_github_analysis,
_run_blog_summarize, and _run_career_analysis currently use "assert handler is
not None" which can be removed under Python -O; replace each assert with an
explicit check and raise a clear exception (e.g., raise RuntimeError or
ValueError) if get_handler(TaskType...) returns None, and optionally log the
error before raising to provide context; ensure the check occurs before awaiting
handler.run so handler.run is only called when handler is non-None.
In `@frontend/src/components/blog/BlogPage.tsx`:
- Around line 73-82: The passed callbacks handleSave, handleSync, and
handleDelete are async (return Promise<void>) but BlogPlatformListProps declares
onSave/onSync/onDelete as synchronous (platform: PlatformKey) => void; update
the BlogPlatformListProps type to accept async handlers by changing those three
signatures to (platform: PlatformKey) => Promise<void> (or alternatively wrap
the async calls at the call site), and ensure the BlogPlatformList component and
any consumers treat these handlers as Promise-returning functions; reference the
prop names onSave/onSync/onDelete and the async functions handleSave,
handleSync, handleDelete when making the change.
In `@frontend/src/components/blog/BlogPlatformList.tsx`:
- Around line 95-97: The Enter-key handler currently calls onSave(pf.key)
unconditionally, allowing duplicate concurrent saves; update the onKeyDown in
BlogPlatformList.tsx to first check the same savingPlatform guard used by the
Save button (e.g., ensure savingPlatform !== pf.key or !savingPlatform[pf.key],
matching how savingPlatform is implemented) and only call onSave(pf.key) when
not already saving, also optionally call e.preventDefault()/e.stopPropagation()
when triggering the save to mirror button behavior.
In `@frontend/src/components/career-analysis/result/CareerAnalysisResultView.tsx`:
- Around line 45-47: The component CareerAnalysisResultView force-unwraps
selected.result into const r = selected.result! which can throw if result is
null/undefined; update the function to guard selected.result (e.g., if
(!selected?.result) return null or render a fallback/loading/error UI) and
remove the non-null assertion so subsequent references use the safe value (e.g.,
const r = selected.result) after the guard; ensure all usages of r inside the
component assume it is defined only after the guard.
In `@frontend/src/hooks/useCareerExperienceMutators.ts`:
- Around line 112-132: In removeProject, simplify the blank project replacement
by removing the redundant explicit technology_stacks override: when replacing
the single project use just a shallow copy of blankCareerProject (i.e., {
...blankCareerProject }) instead of `{ ...blankCareerProject, technology_stacks:
[{ ...blankCareerTechnologyStack }] }`; update the return inside clients'
projects mapping in the removeProject function to reference blankCareerProject
only so the existing initialization of technology_stacks on blankCareerProject
is used.
In `@Makefile`:
- Around line 102-106: The Makefile adds two command-only targets dev-proxy and
dev-proxy-only but they aren't listed in the .PHONY declaration, so a same-named
file could block execution; update the .PHONY line to include dev-proxy and
dev-proxy-only (or add a separate .PHONY declaration) so both targets are always
treated as phony; look for the .PHONY definition and append these exact symbols:
dev-proxy dev-proxy-only.
In `@README.md`:
- Around line 152-170: Remove the duplicated "#### Turso (libSQL) ローカル起動" block
that repeats the TURSO_DATABASE_URL/TURSO_AUTH_TOKEN code fence and the "#####
TablePlus からローカル libSQL に接続する" steps; keep the original canonical section
elsewhere, delete this repeated fenced code and the TablePlus subsection to
avoid MD040 and doc drift, and ensure no empty/incorrect fenced-code-language
markers remain in the README.
---
Nitpick comments:
In `@backend/app/db/bootstrap.py`:
- Around line 8-39: Remove the duplicate function definition of
_validate_jwt_keys so there is only a single implementation; locate both
definitions (they both import get_jwt_private_key and get_jwt_public_key and use
jose.jwt/JWTError) and delete one of them, keeping the other intact (ensure the
surviving _validate_jwt_keys still imports from ..core.settings and uses
jwt.encode/jwt.decode as shown).
In `@backend/app/services/intelligence/github_analysis_service.py`:
- Around line 37-40: The code silently returns when the GitHubAnalysisCache
lookup fails; instead of returning, raise a descriptive exception so callers
cannot ignore this unexpected state. Replace the early return after the
db.query(GitHubAnalysisCache).filter_by(user_id=user_id).first() check: log the
error with user_id via logger.error as before and then raise a RuntimeError (or
a project-specific custom exception) that includes the user_id and a short
message like "GitHub analysis cache not found" so upstream callers can handle or
fail loudly; ensure this change is applied to the function containing the cache
lookup and update any callers/tests that expect a None return if needed.
- Around line 119-121: Replace the bare "except Exception:" in
github_analysis_service.py (the block that logs "学習アドバイスの生成に失敗しましたが、分析結果は保存します")
with specific exception handlers for expected errors (e.g., except (LLMError,
TimeoutError, ConnectionError) as e: logger.warning(..., exc_info=True); return
None, True) and add a final broad handler that logs and re-raises unexpected
exceptions (e.g., except Exception as e: logger.exception("Unexpected error
generating advice", exc_info=True); raise) so only known failures are swallowed
while truly unexpected errors bubble up.
- Around line 86-91: The code currently sets cache.status = "completed" while
using cache.error_message to record a non-fatal LLM failure (llm_failed), which
conflates errors with warnings; add a distinct field (e.g.,
cache.warning_message or cache.llm_status) to represent non-fatal LLM issues and
populate that when llm_failed is true, leaving cache.error_message for true
failures only; update the DB model/migrations to add the new column, change the
save logic in the block where cache.status, cache.error_message,
cache.completed_at are set (referencing cache.status, cache.error_message,
cache.completed_at, llm_failed), and adjust any consumers/readers of
cache.error_message to treat the new field for LLM warnings.
In `@backend/app/services/tasks/worker.py`:
- Around line 30-34: Move the repeated local imports of the time module to the
top-level of the module: add a single "import time" at module level and remove
the inline "import time" statements inside _monotonic_ms_since and execute_task,
so _monotonic_ms_since(start: float) and execute_task() both use the
module-level time reference.
In `@backend/tests/test_intelligence.py`:
- Around line 259-278: The tests currently perform inline imports inside
test_parse_pom_xml and test_parse_go_mod; move these to the module level by
adding a single import statement like "from
app.services.intelligence.github.repo_analyzer import parse_pom_xml,
parse_go_mod" at the top of the test file and remove the inline imports from the
two test functions so the tests call parse_pom_xml(...) and parse_go_mod(...)
directly.
- Line 348: The test uses deprecated asyncio.get_event_loop().run_until_complete
to run the coroutine; replace that call with asyncio.run to execute the async
coroutine directly. Specifically, change the invocation that assigns result
(currently calling
asyncio.get_event_loop().run_until_complete(run_pipeline(username="testuser")))
to use asyncio.run(run_pipeline(username="testuser")) so run_pipeline is
executed with the modern API and result remains the returned value.
In `@flake.nix`:
- Around line 55-56: There is a duplicate package entry for opentofu in the
package list; remove the second occurrence so only one opentofu entry remains
(search for the symbol "opentofu" in the package list and delete the duplicate
block), then re-run the flake build/check to verify no duplicates remain.
In `@frontend/src/components/blog/BlogArticleList.tsx`:
- Around line 46-53: The filter buttons in BlogArticleList are stateful and need
an accessible pressed state; update the button elements (the ones using key={f},
className with styles.filterTab and styles.filterTabActive, and onClick={() =>
handleFilterChange(f)}) to include aria-pressed set to a boolean expression
reflecting the active filter (e.g., aria-pressed={filter === f}) so screen
readers can announce the selected state; keep the existing class-based visual
styling and click handler unchanged.
In `@frontend/src/hooks/useBlogAccountManager.test.ts`:
- Around line 227-264: Remove the duplicated test block that re-tests
handleUpdate in the useBlogAccountManager tests: delete the redundant
it("handleUpdate を呼ぶと updateBlogAccount が呼ばれ未同期状態に更新される", ...) block so only the
original test remains (the duplicate repeats assertions for
api.updateBlogAccount, result.current.handleUpdate,
accounts[0]?.username/last_synced_at and result.current.success); keep the first
occurrence and remove the later one to avoid noisy, redundant failures and run
the test suite to verify.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b6f5a35-1431-4946-b0df-66803edbf53d
📒 Files selected for processing (69)
.claude/rules/infra/terraform.md.vscode/settings.jsonMakefileREADME.mdbackend/app/db/bootstrap.pybackend/app/db/seed.pybackend/app/db/seeds/qualifications.jsonbackend/app/db/seeds/technology_stacks.jsonbackend/app/routers/auth/endpoints.pybackend/app/routers/auth/oauth_flow.pybackend/app/routers/blog.pybackend/app/routers/career_analysis.pybackend/app/routers/intelligence.pybackend/app/schemas/intelligence.pybackend/app/services/blog/collector.pybackend/app/services/intelligence/career_paths/__init__.pybackend/app/services/intelligence/career_paths/definitions.pybackend/app/services/intelligence/career_paths/matcher.pybackend/app/services/intelligence/career_predictor.pybackend/app/services/intelligence/career_simulator.pybackend/app/services/intelligence/confidence_scorer.pybackend/app/services/intelligence/github_analysis_service.pybackend/app/services/intelligence/llm_summarizer.pybackend/app/services/intelligence/pipeline.pybackend/app/services/intelligence/position_scorer.pybackend/app/services/intelligence/position_weights.jsonbackend/app/services/intelligence/skill_extractor.pybackend/app/services/intelligence/skill_growth_analyzer.pybackend/app/services/intelligence/skill_taxonomy/__init__.pybackend/app/services/intelligence/skill_taxonomy/ownership_map.pybackend/app/services/intelligence/skill_timeline_builder.pybackend/app/services/progress_service.pybackend/app/services/tasks/__init__.pybackend/app/services/tasks/base.pybackend/app/services/tasks/dispatch_service.pybackend/app/services/tasks/handlers/__init__.pybackend/app/services/tasks/handlers/base.pybackend/app/services/tasks/handlers/blog_summarize.pybackend/app/services/tasks/handlers/career_analysis.pybackend/app/services/tasks/handlers/github_analysis.pybackend/app/services/tasks/worker.pybackend/tests/test_career_analysis_api.pybackend/tests/test_intelligence.pybackend/tests/test_llm_summarizer.pybackend/tests/test_retry_flow.pybackend/tests/test_worker_extended.pybackend/tests/test_worker_timeout.pyflake.nixfrontend/src/api/client.test.tsfrontend/src/components/analysis/TechBar.tsxfrontend/src/components/blog/BlogArticleList.tsxfrontend/src/components/blog/BlogPage.tsxfrontend/src/components/blog/BlogPlatformList.tsxfrontend/src/components/career-analysis/CareerAnalysisPage.tsxfrontend/src/components/career-analysis/result/CareerAnalysisResultView.tsxfrontend/src/components/forms/ProjectModal.test.tsxfrontend/src/components/forms/ProjectModal.tsxfrontend/src/components/forms/sections/CareerExperienceSection.tsxfrontend/src/hooks/analysis/useAsyncAnalysisPage.test.tsfrontend/src/hooks/useBlogAccountManager.test.tsfrontend/src/hooks/useBlogAccountManager.tsfrontend/src/hooks/useCareerExperienceMutators.tsfrontend/src/hooks/useNotifications.test.tsfrontend/src/hooks/useProjectModalState.test.tsfrontend/src/hooks/useProjectModalState.validation.test.tsfrontend/src/payloadBuilders.test.tsinfra/environments/dev/main.tfinfra/environments/prod/.terraform.lock.hclinfra/environments/stg/.terraform.lock.hcl
💤 Files with no reviewable changes (10)
- backend/app/services/intelligence/career_paths/init.py
- backend/app/services/intelligence/confidence_scorer.py
- backend/app/services/intelligence/career_paths/matcher.py
- frontend/src/hooks/useProjectModalState.validation.test.ts
- backend/app/services/intelligence/skill_timeline_builder.py
- backend/app/services/intelligence/career_simulator.py
- backend/app/services/intelligence/skill_growth_analyzer.py
- backend/app/services/intelligence/career_predictor.py
- backend/app/services/intelligence/career_paths/definitions.py
- frontend/src/components/analysis/TechBar.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Repository rule violations found Changes must be made through a pull request. Required status check "test" is expected. Cannot update this protected ref. |
code rabbit RV fix
code rabbit RV fix
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/app/repositories/blog.py`:
- Around line 241-245: The except block that catches IntegrityError in
get_or_create() currently rollbacks and re-queries via self.db.scalar(...), but
that re-query can still return None which violates the function's
BlogSummaryCache return type; update the except handler to check the result of
the re-query and if it's None either (a) raise a clear runtime exception (e.g.
RuntimeError/ValueError) explaining the cache row could not be retrieved after
retry, or (b) create and return a new BlogSummaryCache record atomically—ensure
you reference IntegrityError, self.db.rollback(),
self.db.scalar(select(BlogSummaryCache).where(BlogSummaryCache.user_id ==
self.user_id)), and get_or_create() so callers always receive a concrete
BlogSummaryCache instance or a clear error.
In `@backend/app/services/tasks/handlers/blog_summarize.py`:
- Around line 41-43: The handler currently treats a missing cache (the variable
cache) as a successful early return after logging (logger.error(...)), which
hides failure from upstream; replace the early return with an explicit failure
path by raising a descriptive exception (e.g., CacheNotFoundError or
RuntimeError) or invoking the task failure/dead-letter API used in this service
so the task is marked failed; keep the existing logger.error call (including
extra={"user_id": user_id}) and include the exception message so upstream
systems observe the failure instead of a silent success.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f63244f-e8cf-467b-a8d3-a6ca8d9537a4
📒 Files selected for processing (32)
.claude/rules/infra/terraform.md.gitignoreMakefileREADME.mdbackend/alembic_migrations/versions/0031_add_warning_message_to_github_analysis_cache.pybackend/app/core/errors.pybackend/app/core/security/auth.pybackend/app/db/bootstrap.pybackend/app/db/seeds/technology_stacks.jsonbackend/app/models/cache.pybackend/app/repositories/blog.pybackend/app/routers/blog.pybackend/app/routers/career_analysis.pybackend/app/routers/intelligence.pybackend/app/schemas/intelligence.pybackend/app/services/intelligence/github_analysis_service.pybackend/app/services/intelligence/position_weights.jsonbackend/app/services/intelligence/skill_extractor.pybackend/app/services/tasks/dispatch_service.pybackend/app/services/tasks/handlers/blog_summarize.pybackend/app/services/tasks/handlers/career_analysis.pybackend/app/services/tasks/worker.pybackend/pyproject.tomlbackend/tests/test_intelligence.pybackend/tests/test_worker_extended.pyfrontend/src/api/intelligence.tsfrontend/src/components/blog/BlogArticleList.tsxfrontend/src/components/blog/BlogPlatformList.tsxfrontend/src/components/career-analysis/result/CareerAnalysisResultView.tsxfrontend/src/hooks/useBlogAccountManager.test.tsfrontend/src/hooks/useCareerExperienceMutators.tspyrightconfig.json
💤 Files with no reviewable changes (1)
- .gitignore
✅ Files skipped from review due to trivial changes (7)
- backend/alembic_migrations/versions/0031_add_warning_message_to_github_analysis_cache.py
- pyrightconfig.json
- backend/app/services/intelligence/position_weights.json
- backend/pyproject.toml
- backend/app/core/errors.py
- backend/app/db/seeds/technology_stacks.json
- README.md
🚧 Files skipped from review as they are similar to previous changes (12)
- backend/app/services/tasks/handlers/career_analysis.py
- Makefile
- backend/app/services/tasks/dispatch_service.py
- frontend/src/components/career-analysis/result/CareerAnalysisResultView.tsx
- frontend/src/components/blog/BlogArticleList.tsx
- backend/app/routers/intelligence.py
- backend/app/routers/career_analysis.py
- frontend/src/components/blog/BlogPlatformList.tsx
- backend/app/services/intelligence/skill_extractor.py
- frontend/src/hooks/useCareerExperienceMutators.ts
- backend/app/services/tasks/worker.py
- backend/app/routers/blog.py
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.claude/CLAUDE.md:
- Around line 45-47: The fenced code block containing the error message `error:
opening lock file "~/.cache/nix/fetcher-locks/...lock": Operation not permitted`
is missing a language tag and triggers markdownlint MD040; fix it by adding a
language identifier (e.g., `text`) immediately after the opening
triple-backticks for that block in .claude/CLAUDE.md so the block becomes
```text ... ``` to satisfy the linter.
In `@backend/app/services/tasks/handlers/blog_summarize.py`:
- Around line 37-40: The handler currently logs and returns when payload lacks
user_id (variable user_id) which treats the task as a success; instead raise
NonRetryableError to mark the task as failed: in the blog_summarize handler
replace the early return after logger.error with raising NonRetryableError
(include a concise message and optionally payload keys) so upstream treats this
as a non-retriable failure.
- Around line 82-85: The current branch that handles a missing summary sets
cache.error_message to a generic "LLM処理が利用できません" which duplicates the earlier
availability message; update the failure message in the block that checks "if
not summary:" (in the blog summarization handler where variables cache and
_now() are used) to a distinct, descriptive text like "要約に失敗しました" (or
equivalent) so it clearly indicates a summarization failure, keep setting
cache.status = "dead_letter" and cache.completed_at = _now() unchanged.
In `@docs/deployment.md`:
- Around line 85-98: The fenced code block in docs/deployment.md that begins
with "infra/" is missing a language identifier; update that fence to include a
language (e.g., add ```text or ```bash) so the block becomes a proper fenced
code block with a language tag to satisfy markdownlint MD040, keeping the block
contents unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7004774f-5a51-46d6-a9a1-5b6664cf0b8e
📒 Files selected for processing (27)
.claude/AGENT.md.claude/CLAUDE.md.claude/rules/backend/architecture.md.claude/rules/backend/auth-security.md.claude/rules/backend/python.md.claude/rules/backend/test.md.claude/rules/frontend/architecture.md.claude/rules/frontend/test.md.claude/rules/frontend/typescript.md.claude/rules/github.md.claude/rules/infra/opentofu.md.claude/rules/infra/test.md.claude/skills/backend_refacter/SKILL.md.claude/skills/frontend_refacter/SKILL.md.vscode/settings.jsonAGENT.mdREADME.mdbackend/app/repositories/blog.pybackend/app/services/tasks/handlers/blog_summarize.pybackend/app/services/tasks/worker.pybackend/tests/test_worker_extended.pydocs/api.mddocs/data-model.mddocs/deployment.mddocs/development.mddocs/ollama-setup.mdinfra/README.md
💤 Files with no reviewable changes (4)
- .claude/rules/github.md
- .claude/AGENT.md
- docs/ollama-setup.md
- infra/README.md
✅ Files skipped from review due to trivial changes (10)
- AGENT.md
- .claude/rules/infra/opentofu.md
- .claude/rules/frontend/typescript.md
- .vscode/settings.json
- .claude/rules/backend/architecture.md
- .claude/rules/frontend/architecture.md
- .claude/rules/backend/test.md
- docs/development.md
- docs/api.md
- .claude/rules/frontend/test.md
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/repositories/blog.py
- backend/tests/test_worker_extended.py
- backend/app/services/tasks/worker.py
code rabbit fix
code rabbit fix
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/tests/test_security_edges.py`:
- Around line 116-117: The unauthenticated endpoint matrix entry using
"/api/notifications/abc/read" uses an invalid ID shape that can trigger route
validation before auth; replace that string with a syntactically valid
notification ID (for example a UUID like
"/api/notifications/00000000-0000-0000-0000-000000000000/read" or the app's
canonical ID shape) in the tuple so the test in the unauthenticated matrix
exercises authentication logic rather than path validation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1366941e-8598-41f9-a6cb-c41eed8fd053
📒 Files selected for processing (4)
.claude/CLAUDE.mdbackend/app/services/tasks/handlers/blog_summarize.pybackend/tests/test_security_edges.pydocs/deployment.md
✅ Files skipped from review due to trivial changes (1)
- docs/deployment.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/CLAUDE.md
Comment on lines
+116
to
+117
| ("patch", "/api/notifications/abc/read"), | ||
| ("post", "/api/notifications/read-all"), |
There was a problem hiding this comment.
Use a type-valid notification ID in the unauthenticated endpoint matrix.
/api/notifications/abc/read can hit path validation/routing before auth, so this case may stop testing auth behavior for this endpoint. Use a syntactically valid ID shape instead.
Suggested minimal fix
- ("patch", "/api/notifications/abc/read"),
+ ("patch", "/api/notifications/1/read"),📝 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
| ("patch", "/api/notifications/abc/read"), | |
| ("post", "/api/notifications/read-all"), | |
| ("patch", "/api/notifications/1/read"), | |
| ("post", "/api/notifications/read-all"), |
🤖 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 `@backend/tests/test_security_edges.py` around lines 116 - 117, The
unauthenticated endpoint matrix entry using "/api/notifications/abc/read" uses
an invalid ID shape that can trigger route validation before auth; replace that
string with a syntactically valid notification ID (for example a UUID like
"/api/notifications/00000000-0000-0000-0000-000000000000/read" or the app's
canonical ID shape) in the tuple so the test in the unauthenticated matrix
exercises authentication logic rather than path validation.
Owner
Author
There was a problem hiding this comment.
指摘を skip します。理由: notification_id: str 型なので "abc" は path validation を発火させない（実際に既存の 108 ケースが 401 で pass
している）。指摘の前提「route validation が auth より先に発火する」は不成立。
There was a problem hiding this comment.
@yusuke0610 ご説明ありがとうございます。おっしゃる通りです。notification_id: str 型の場合、FastAPI のパスパラメータ解析は "abc" を問題なく受け入れるため、型バリデーションが auth より先に発火するという前提が誤りでした。108 ケースが全て 401 で pass しているという事実が正しさを裏付けています。指摘を取り下げます。
(´･ω･`) 🐰
✏️ Learnings added
Learnt from: yusuke0610
Repo: yusuke0610/devforge PR: 245
File: backend/tests/test_security_edges.py:116-117
Timestamp: 2026-05-15T23:07:09.005Z
Learning: In `backend/tests/test_security_edges.py`, the FastAPI route for marking a notification as read uses `notification_id: str` as the path parameter type. This means any string value (including "abc") is accepted at the path-parsing stage without triggering a 422 validation error, so authentication middleware fires before any business logic. When reviewing unauthenticated-access test matrices in this codebase, do not flag string-typed path param values like "abc" as potentially bypassing auth — they are valid and intentional.
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.
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.
#244
#243
Summary by CodeRabbit
New Features
Improvements
Documentation