Skip to content

fix: address PR #492 review comments (Y.Xml extract, testConnection, useRef)#493

Merged
otomatty merged 1 commit into
developfrom
fix/pr-492-review-comments
Apr 6, 2026
Merged

fix: address PR #492 review comments (Y.Xml extract, testConnection, useRef)#493
otomatty merged 1 commit into
developfrom
fix/pr-492-review-comments

Conversation

@otomatty
Copy link
Copy Markdown
Owner

@otomatty otomatty commented Apr 6, 2026

概要

PR #492(develop → main)に付いたインラインレビューへの対応です。develop へ直接 push せず、本ブランチ経由で取り込みます。

Addresses inline review comments on release PR #492; merged via this branch.

変更点

  • server/hocuspocus: Y.Xml からのプレーンテキスト抽出で、インライン相当(bold 等)の XmlElement の後に不要な改行を入れないよう分離(extractPlainTextFromYXml モジュール)。単体テスト追加。
  • src/lib/aiClient.ts: testConnection でプロバイダーが claude-code のときは空 API キーでも共通の「キーを入力」で早期 return しない(専用メッセージへ到達)。
  • AIChatWikiLink: React 19 の useRef に明示初期値。

テスト

  • bun run test:run(ルート + claude-sidecar + hocuspocus)

関連

チェックリスト

  • テスト
  • Lint / format(pre-commit)

- Hocuspocus: extract inline mark elements (bold, etc.) without spurious newlines;
  add extractPlainTextFromYXml module + tests (Gemini review).
- aiClient: allow claude-code branch when API key is empty (CodeRabbit review).
- AIChatWikiLink: useRef initial values for React 19 types (CodeRabbit review).

Made-with: Cursor
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 6, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5d25ee9-d3e0-4651-bb3a-23e1acdbb10b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pr-492-review-comments

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

@otomatty otomatty mentioned this pull request Apr 6, 2026
11 tasks
@otomatty otomatty merged commit e0d93b1 into develop Apr 6, 2026
9 of 10 checks passed
@otomatty otomatty deleted the fix/pr-492-review-comments branch April 6, 2026 02:49
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors plain text extraction from Y.Xml structures to improve formatting, adds support for 'claude-code' as an AI provider that does not require an API key, and updates test coverage. The review identified a performance issue in the text extraction loop due to O(N^2) complexity and a potential database constraint violation in the content preview truncation logic. I have included the suggested improvements to address these issues.

Comment on lines +38 to +47
for (let i = 0; i < node.length; i++) {
const child = node.get(i);
if (child instanceof Y.XmlText) {
text += child.toString();
} else if (child instanceof Y.XmlElement) {
const inner = extractTextFromYXml(child);
const suffix = isInlineXmlElement(child) ? " " : "\n";
text += inner + suffix;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Yjsの共有型(XmlFragmentやXmlElement)において、node.lengthnode.get(i) を組み合わせたループは $O(N^2)$ の計算量になります。これは、lengthget の両方が内部の連結リストを走査するためです。ドキュメントが大きくなった際のパフォーマンス低下を避けるため、node.toArray() を使用して配列に変換してからループを回すか、firstChildnextSibling を使用したイテレーションを推奨します。

  for (const child of node.toArray()) {
    if (child instanceof Y.XmlText) {
      text += child.toString();
    } else if (child instanceof Y.XmlElement) {
      const inner = extractTextFromYXml(child);
      const suffix = isInlineXmlElement(child) ? " " : "\n";
      text += inner + suffix;
    }
  }

export function buildContentPreview(text: string): string {
const trimmed = text.trim().replace(/\s+/g, " ");
if (trimmed.length <= CONTENT_PREVIEW_MAX_LENGTH) return trimmed;
return trimmed.slice(0, CONTENT_PREVIEW_MAX_LENGTH).trim() + "...";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

DBの content_preview カラムのサイズ制限が120文字である場合、現在の実装では slice(0, 120) の後に ... を付与しているため、合計で最大123文字になります。これにより保存時にデータ切り詰めやエラーが発生する可能性があります。制限が厳密に120文字である場合は、省略記号を含めて120文字に収まるように調整(例:slice(0, CONTENT_PREVIEW_MAX_LENGTH - 3))することを検討してください。

Suggested change
return trimmed.slice(0, CONTENT_PREVIEW_MAX_LENGTH).trim() + "...";
return trimmed.slice(0, CONTENT_PREVIEW_MAX_LENGTH - 3).trim() + "...";

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant