Skip to content
This repository was archived by the owner on May 2, 2026. It is now read-only.

feat: implement fetchThread for BacklogAdapter#18

Open
amotarao wants to merge 5 commits into
mainfrom
amon/pc-31-implement-fetchthread-for-backlogadapter
Open

feat: implement fetchThread for BacklogAdapter#18
amotarao wants to merge 5 commits into
mainfrom
amon/pc-31-implement-fetchthread-for-backlogadapter

Conversation

@amotarao

@amotarao amotarao commented Apr 17, 2026

Copy link
Copy Markdown
Member

Summary

  • BacklogAdapterBacklogClient を追加し、コンストラクタで初期化
  • fetchThread を実装: Backlog API で issue を取得し ThreadInfo に変換して返す
  • fetchThread のテストを追加(issue スレッド・コメントスレッド両パターン)

Test plan

  • pnpm test が全て通ること

closes https://linear.app/plainbrew/issue/PC-31

Summary by CodeRabbit

リリースノート

  • 新機能

    • スレッド情報取得機能を実装しました。Backlog の課題情報を取得し、スレッドID、チャネルID、チャネル名、課題キーや要約などを含むスレッド情報を返します。
  • テスト

    • スレッド取得機能のテストを追加し、複数のスレッドID形式に対する動作を検証しました。
  • Chores

    • パッケージのマイナーバージョン更新用の変更記録を追加しました。

@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@amotarao has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 59 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 59 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef7816c3-011f-4031-a4fb-9538f156a438

📥 Commits

Reviewing files that changed from the base of the PR and between 8937874 and f430386.

📒 Files selected for processing (1)
  • .changeset/pc-31-fetch-thread.md

Walkthrough

BacklogAdapterにBacklogClientを統合し、未実装だったfetchThreadを実装。スレッドIDからissueKeyを抽出してBacklogClient.getIssueでissueを取得し、derivedなchannelId/channelNamemetadataを含むThreadInfoを返す。テストが2ケース追加された。

Changes

Cohort / File(s) Summary
Core Implementation
packages/chat-adapter-backlog/src/adapter.ts
BacklogAdapterBacklogClientメンバーを追加し、コンストラクタで初期化。fetchThreadを実装してスレッドIDを解析(backlog:...:PROJ-123 / backlog:...:PROJ-123:456 形式)、getIssue(issueKey)を呼び出してThreadInfoを構築して返却。
Test Coverage
packages/chat-adapter-backlog/src/adapter.test.ts
vitestvi導入と./backlog-client.jsのモック追加。fetchThreadの2つのIDフォーマットに対するテストを追加し、BacklogClient.prototype.getIssueをスタブして返値および呼び出し引数(issueKey)を検証。
Release Metadata
.changeset/pc-31-fetch-thread.md
chat-adapter-backlogのChangesetを追加(minor)。fetchThread実装をリリースノートに記載。

Sequence Diagram(s)

sequenceDiagram
  participant Caller as Caller
  participant Adapter as BacklogAdapter
  participant Client as BacklogClient
  participant API as Backlog API

  Caller->>Adapter: fetchThread(threadId)
  Adapter->>Client: getIssue(issueKey)
  Client->>API: HTTP GET /issues/{issueKey}
  API-->>Client: issue payload
  Client-->>Adapter: issue payload
  Adapter-->>Caller: ThreadInfo { id, channelId, channelName, metadata }
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 ぼくが掘ったキーは PROJ-123、
ぽんっと呼べばissueがころりん。
アダプターで紡ぎ、スレッドにして、
テストも一緒にぴょんと跳ねる。
🎉✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed プルリクエストのタイトルは、BacklogAdapterに対するfetchThreadの実装という主要な変更を明確に要約しており、ファイルの変更内容と一致しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch amon/pc-31-implement-fetchthread-for-backlogadapter

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/adapter.ts (2)

24-32: BacklogClient は注入可能にしておくと保守しやすいです。

Line 31 の直接 new はテスト時に prototype モックへ依存しやすいため、任意注入にしておくとユニットテストの分離性が上がります。

♻️ リファクタ案
 export class BacklogAdapter implements Adapter<BacklogThreadId, unknown> {
@@
-  constructor(config: BacklogConfig) {
+  constructor(config: BacklogConfig, client?: BacklogClient) {
     this.config = config;
     this.userName = config.userName ?? "Backlog Bot";
-    this.client = new BacklogClient(config.host, config.apiKey);
+    this.client = client ?? new BacklogClient(config.host, config.apiKey);
     this.formatConverter = new BacklogFormatConverter();
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapter.ts` around lines 24 - 32, The constructor currently instantiates
BacklogClient directly (new BacklogClient(...)) which hinders testability;
change the Adapter to accept an optional BacklogClient via dependency injection
(e.g., add an optional parameter client?: BacklogClient to the constructor or a
setter) and fall back to new BacklogClient(config.host, config.apiKey) only when
none is provided; update the class field declaration for client and the
constructor logic in adapter.ts (symbols: BacklogClient, constructor,
this.client) so unit tests can inject a mock client.

90-95: decodeThreadId の再実行は 1 回にまとめられます。

Line 90 で既に分解済みなので、Line 94 の channelIdFromThreadId(threadId) は組み立てに置き換え可能です。小さな最適化ですが可読性も上がります。

🔧 軽微な改善案
   async fetchThread(threadId: string): Promise<ThreadInfo> {
-    const { issueKey, projectKey } = this.decodeThreadId(threadId);
+    const { issueKey, projectKey, spaceKey } = this.decodeThreadId(threadId);
     const issue = await this.client.getIssue(issueKey);
     return {
       id: threadId,
-      channelId: this.channelIdFromThreadId(threadId),
+      channelId: `backlog:${spaceKey}:${projectKey}`,
       channelName: projectKey,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapter.ts` around lines 90 - 95, You already decode threadId into
issueKey and projectKey via decodeThreadId(threadId); avoid re-running that
parsing by replacing the call this.channelIdFromThreadId(threadId) with a direct
construction that uses the decoded values (projectKey and/or issueKey) to
produce the same channelId format that channelIdFromThreadId emits—i.e., reuse
issueKey/projectKey to assemble the channelId inline or via a helper that
accepts those parts, keeping decodeThreadId only once and improving readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/adapter.test.ts`:
- Around line 55-87: Clear the mock call history before each test and assert
exact call counts to avoid false positives: add a beforeEach that calls
vi.clearAllMocks() (or vi.resetAllMocks()) so BacklogClient.prototype.getIssue
is cleared, and in each test that uses adapter.fetchThread assert both that
BacklogClient.prototype.getIssue was calledWith the expected issue key and that
it was called exactly once (e.g., toHaveBeenCalledTimes(1)); refer to the
existing tests that call adapter.fetchThread and
BacklogClient.prototype.getIssue to locate where to add the beforeEach and the
extra toHaveBeenCalledTimes assertions.

---

Nitpick comments:
In `@src/adapter.ts`:
- Around line 24-32: The constructor currently instantiates BacklogClient
directly (new BacklogClient(...)) which hinders testability; change the Adapter
to accept an optional BacklogClient via dependency injection (e.g., add an
optional parameter client?: BacklogClient to the constructor or a setter) and
fall back to new BacklogClient(config.host, config.apiKey) only when none is
provided; update the class field declaration for client and the constructor
logic in adapter.ts (symbols: BacklogClient, constructor, this.client) so unit
tests can inject a mock client.
- Around line 90-95: You already decode threadId into issueKey and projectKey
via decodeThreadId(threadId); avoid re-running that parsing by replacing the
call this.channelIdFromThreadId(threadId) with a direct construction that uses
the decoded values (projectKey and/or issueKey) to produce the same channelId
format that channelIdFromThreadId emits—i.e., reuse issueKey/projectKey to
assemble the channelId inline or via a helper that accepts those parts, keeping
decodeThreadId only once and improving readability.
🪄 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: CHILL

Plan: Pro

Run ID: 21d706c0-fc41-4dfe-b82e-6d8db63f7557

📥 Commits

Reviewing files that changed from the base of the PR and between 410fe4c and 3e7bddd.

📒 Files selected for processing (2)
  • src/adapter.test.ts
  • src/adapter.ts

Comment on lines +55 to +87
it("returns ThreadInfo for an issue thread", async () => {
vi.mocked(BacklogClient.prototype.getIssue).mockResolvedValue(mockIssue);

const result = await adapter.fetchThread("backlog:myspace:PROJ-123");
expect(result).toEqual({
id: "backlog:myspace:PROJ-123",
channelId: "backlog:myspace:PROJ",
channelName: "PROJ",
metadata: {
issueKey: "PROJ-123",
summary: "Test Issue",
issue: mockIssue,
},
});
expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
});

it("returns ThreadInfo for a comment thread", async () => {
vi.mocked(BacklogClient.prototype.getIssue).mockResolvedValue(mockIssue);

const result = await adapter.fetchThread("backlog:myspace:PROJ-123:456");
expect(result).toEqual({
id: "backlog:myspace:PROJ-123:456",
channelId: "backlog:myspace:PROJ",
channelName: "PROJ",
metadata: {
issueKey: "PROJ-123",
summary: "Test Issue",
issue: mockIssue,
},
});
expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 目的: テスト分離ガードの有無を静的に確認する(read-only)
sed -n '1,180p' src/adapter.test.ts
rg -n 'beforeEach|clearAllMocks|resetAllMocks|restoreAllMocks' src/adapter.test.ts
rg -n 'toHaveBeenCalledWith|toHaveBeenCalledTimes' src/adapter.test.ts

Repository: plainbrew/chat-adapter-backlog

Length of output: 3264


モック呼び出し検証が偽陽性になる余地があります。

Line 69 / Line 86 は toHaveBeenCalledWith のみなので、前テストの呼び出し履歴が残っていても通る可能性があります。beforeEach でモックをクリアし、各テストで呼び出し回数も固定してください。

💡 修正案
 describe("fetchThread", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
   const mockIssue = {
@@
   it("returns ThreadInfo for an issue thread", async () => {
@@
-    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1);
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
   });
@@
   it("returns ThreadInfo for a comment thread", async () => {
@@
-    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1);
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
   });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/adapter.test.ts` around lines 55 - 87, Clear the mock call history before
each test and assert exact call counts to avoid false positives: add a
beforeEach that calls vi.clearAllMocks() (or vi.resetAllMocks()) so
BacklogClient.prototype.getIssue is cleared, and in each test that uses
adapter.fetchThread assert both that BacklogClient.prototype.getIssue was
calledWith the expected issue key and that it was called exactly once (e.g.,
toHaveBeenCalledTimes(1)); refer to the existing tests that call
adapter.fetchThread and BacklogClient.prototype.getIssue to locate where to add
the beforeEach and the extra toHaveBeenCalledTimes assertions.

@vercel

vercel Bot commented Apr 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
chat-adapter-backlog-example Error Error Apr 19, 2026 1:40am

Request Review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
packages/chat-adapter-backlog/src/adapter.test.ts (1)

55-87: ⚠️ Potential issue | 🟡 Minor

モック履歴のリセットと呼び出し回数の固定を追加してください。

toHaveBeenCalledWith のみだと前テスト由来の呼び出し履歴で通る余地があります。beforeEach で履歴をクリアし、各テストで toHaveBeenCalledTimes(1) も入れてください。

💡 修正案
 describe("fetchThread", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
   const mockIssue = {
@@
   it("returns ThreadInfo for an issue thread", async () => {
@@
-    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1);
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
   });
@@
   it("returns ThreadInfo for a comment thread", async () => {
@@
-    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1);
+    expect(BacklogClient.prototype.getIssue).toHaveBeenCalledWith("PROJ-123");
   });
 });
#!/bin/bash
# 目的: fetchThreadテストの分離ガード有無をread-onlyで確認
sed -n '1,220p' packages/chat-adapter-backlog/src/adapter.test.ts
rg -n 'describe\("fetchThread"|beforeEach|clearAllMocks|toHaveBeenCalledTimes|toHaveBeenCalledWith' packages/chat-adapter-backlog/src/adapter.test.ts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/chat-adapter-backlog/src/adapter.test.ts` around lines 55 - 87, Add
mock reset and strict call count checks: in the test suite's beforeEach add
vi.clearAllMocks() (or vi.resetAllMocks()) so BacklogClient.prototype.getIssue
history is cleared before each test, and update both tests that call
adapter.fetchThread to assert BacklogClient.prototype.getIssue was called
exactly once by adding
expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1) in addition to
the existing toHaveBeenCalledWith("PROJ-123"); this ensures calls to
BacklogClient.prototype.getIssue are isolated per test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/chat-adapter-backlog/src/adapter.test.ts`:
- Around line 55-87: Add mock reset and strict call count checks: in the test
suite's beforeEach add vi.clearAllMocks() (or vi.resetAllMocks()) so
BacklogClient.prototype.getIssue history is cleared before each test, and update
both tests that call adapter.fetchThread to assert
BacklogClient.prototype.getIssue was called exactly once by adding
expect(BacklogClient.prototype.getIssue).toHaveBeenCalledTimes(1) in addition to
the existing toHaveBeenCalledWith("PROJ-123"); this ensures calls to
BacklogClient.prototype.getIssue are isolated per test.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d91f5c0c-4578-435b-82a4-a449f4af9674

📥 Commits

Reviewing files that changed from the base of the PR and between 3e7bddd and ddbd804.

📒 Files selected for processing (2)
  • packages/chat-adapter-backlog/src/adapter.test.ts
  • packages/chat-adapter-backlog/src/adapter.ts

@amotarao amotarao force-pushed the amon/pc-31-implement-fetchthread-for-backlogadapter branch from 8937874 to 8d40fae Compare April 19, 2026 02:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant