[Regression on closed #585] ReputationService.history orders computedAt ASC again, so the default page returns the oldest 50 snapshots

オープン 初心者向け
  • 何か問題がありますか？
#732 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
79/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
typescript
領域
backend-api-design

調査の方向性

src/reputation/reputation.service.ts の history から始め、次に src/reputation/reputation.controller.ts:71 のコントローラーのエントリーポイントと既存の評判テストを確認してください。リポジトリークエリーがスナップショットをどのようにソートし、ページ分けしているかを確認し、computedAt のタイムスタンプが同じ場合も調べてください。完了条件は、デフォルトページが最新のスナップショットを決定的な順序で返し、ソートをカバーするテストがあることです。また、この issue ではページネーションメタデータの返却も提案されています。

索引モデルが issue の本文から書いたものです。

説明

bug

Location — src/reputation/reputation.service.ts:88-99 (history), reached from src/reputation/reputation.controller.ts:71 (GET /reputation/:userId/history).

Problem — The ordering closed as #585 is back:

return this.snapshotRepo.find({
  where: { userId },
  order: { computedAt: 'ASC' },
  take: limit,
  skip: offset,
});

reputation_snapshots is an append-only table (a new row per recompute), so ASC + skip: 0 returns the first snapshots ever written. With the default limit of 50 (REPUTATION_HISTORY_DEFAULT_LIMIT), a contributor who has accumulated more than 50 snapshots gets a page that contains none of their recent reputation and never changes again, no matter how much their stats move. The computedAt sort is also not tie-broken: computedAt is a @CreateDateColumn timestamp, and several recomputes in the same tick (the reputation.recompute event handler at line 78 can fire in bursts) order arbitrarily between pages, so offset paging can repeat or skip rows.

Why it matters — The history endpoint exists to chart reputation over time; its default response is the least useful 50 rows in the table and is indistinguishable from stale data to a client. To reach current data a caller must know the total row count and compute the final offset, which the endpoint does not return. For long-standing contributors (the users a reputation system is for) the endpoint is effectively broken.

Reproduction / failure path

  1. Pick a user and generate 60 snapshots (call POST /reputation/:userId/recompute repeatedly with changing stats, or emit reputation.recompute 60 times).
  2. GET /reputation/:userId/history with no query params.
  3. The response's last element is snapshot #50, not #60; the 10 most recent snapshots are unreachable without an explicit offset.

Suggested fix

  • Change the order to { computedAt: 'DESC', id: 'DESC' } so the default page is the most recent snapshots and equal timestamps resolve deterministically.
  • If clients need chronological order for charting, let them reverse the returned page (or accept an explicit order=asc|desc query param defaulting to desc) rather than making the un-paginated oldest-first behaviour the default.
  • Return a total count (or a nextOffset) alongside the rows so a client can page without guessing.
  • Add a spec asserting the order passed to snapshotRepo.find is DESC with an id tie-break; #585's fix shipped without one, which is why it regressed.
主要言語
TypeScript
スター
2
フォーク
90
平均マージ
31分
マージ済み PR（30日）
20

環境構築

  • Dockerfile または Docker Compose ファイルあり
  • プルリクエストのテンプレートなし
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/backend のほかの issue

MergeFi/backend の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。