Skip to content

perf(classroom): GET /classrooms の DynamoDB 読み取り削減とページング対応 (#1146) - #1148

Merged
takaokouji merged 15 commits into
developfrom
topic/autopilot-1146
Sep 1, 2026
Merged

perf(classroom): GET /classrooms の DynamoDB 読み取り削減とページング対応 (#1146)#1148
takaokouji merged 15 commits into
developfrom
topic/autopilot-1146

Conversation

@smalruby3-editor-bot

@smalruby3-editor-bot smalruby3-editor-bot Bot commented Aug 29, 2026

Copy link
Copy Markdown

Summary

GET /classrooms の DynamoDB 読み取りから Scan を全廃し、Scan / Query のページング未対応を解消する。

Scan は RCU も 1MB 上限も フィルタ適用「前」 に効くため、コストと取りこぼしのリスクが「その先生に見える量」ではなく テーブル全体の量(= 他の先生のデータ量) に比例して増える。当初は Scan の統合(2 回 → 1 回)で足りるか見る方針だったが、レビューで逆引きインデックスの導入指示を受けたので、統合ではなく置き換えた。

  • 共同管理(課題単位・クラス単位) → 逆引き索引テーブル ClassroomCoTeacherIndex{suffix}(PK coTeacherEmail / SK resourceKey)の Query + BatchGetItemcoTeacherEmails はリスト属性で GSI のキーにできないため別テーブルになる。
  • クラス(組)に属する課題 → Classrooms の新 GSI groupId-index をクラスごとに Query(旧 groupId IN (...) の Scan を置換)。IN のオペランド 100 個上限と、それに伴うチャンク分割も不要になった。
  • LastEvaluatedKey のページング: 1MB 上限は Query でも効くので、辿らないと エラー無しで黙って取りこぼす。共通ヘルパー queryAll / scanAll / batchGetAll に寄せた。

Changes Made

  • infra/smalruby-classroom/lib/classroom-stack.ts
    • ClassroomCoTeacherIndex{suffix} テーブル(TTL ttl・DESTROY)と Classrooms の groupId-index GSI を追加。env CO_TEACHER_INDEX_TABLE_NAMEDDB_MAX_PAGES を配線。
  • infra/smalruby-classroom/lambda/handler.ts
    • paginateAll / queryAll / scanAll / batchGetAll を追加。DDB_MAX_PAGES(既定 25、不正値は既定へフォールバック)で無限ループを防ぎ、打ち切り時は CloudWatch に warnbatchGetAllUnprocessedKeys を再試行する(スロットリングはエラーではなく未処理キーの返却で表れるため、捨てると同じ「黙って取りこぼす」形になる)。
    • syncCoTeacherIndex(): coTeacherEmails を書き換える 4 経路すべて(課題の共同管理者 追加 / 削除、組の更新、v2 移行での組への引き上げ)から差分だけ Put/Delete。
    • listCoTeacherResourceIds / listAssignmentsCoTaughtBy / listCoManagedGroups / listAssignmentsInGroups を索引・GSI ベースに。listSharedAssignments(OR 統合 Scan + センチネル + 1 行ラッパ)は削除。
    • 一覧系・列挙結果が書き込みを駆動する経路(移行、クラス人数の伝播、課題名の重複連番、前回コメントの recap)をページング版に統一。
  • infra/smalruby-classroom/lambda/coteacher-index-lib.ts + bin/backfill-coteacher-index.ts — 既存データのバックフィル(既定 dry-run・冪等)。判断は純粋関数に置いてテストで固定し、CLI は I/O だけ。
  • docs/classroom/architecture.md — 読み取り構成、逆引き索引の役割(認可は索引を見ない)、ページングの線引きを更新。
  • docs/classroom/operations.md — バックフィルの runbook(デプロイ順を含む)。
  • .claude/rules/infra/smalruby-classroom.md — テーブル一覧(8 → 9)と GSI を追記。

1 リクエストあたりの読み取り(GET /classrooms

当初 after
Classrooms Scan 2 0
ClassroomGroups Scan 1 0
Classrooms Query 1(teacherSub-index 1 + 管理クラス数(groupId-index
逆引き索引 Query 2(課題用 / 組用)
BatchGet 共同管理の件数 / 100

Scan が消えたので、レビューで挙がった 「ページ上限 25 × 1MB × 4 本 vs memorySize: 256」は前提ごと解消した(フィルタ前に読む量がテーブルサイズに比例しなくなった)。

Test Coverage

  • lambda/tests/handler-list-classrooms-reads.test.ts(8 件)— Classrooms / ClassroomGroups への Scan が 0 回、逆引き索引と groupId-index で引くこと、各経路のページング、索引が消し忘れを返しても資源本体のリストで再確認して除外すること、BatchGet の 100 キー分割、壊れたページャで打ち切ること。
  • lambda/tests/handler-coteacher-index-sync.test.ts(4 件・新規)— coTeacherEmails を書き換える 4 経路すべてが索引を同期し、据え置きの email には書かない(差分のみ)こと。書かない経路が残ると、その共同管理者に資源が見えなくなる。
  • lambda/tests/coteacher-index-lib.test.ts(10 件・新規)— バックフィルの行生成(正規化・重複・アーカイブ済みも対象・壊れた行は無視)と BatchWrite のチャンク分割。
  • Scan 前提だったモック(handler-group-co-teacher / handler-group-enumeration / handler-group-seatcount)を索引・GSI に追随。co-teacher のモックは一覧経路で Scan が来たら throw するようにして、退行が green にならないようにした。
  • jest --clearCache && npx jest: 21 suites / 314 tests すべて green(cold cache)
  • npx tsc --noEmit: エラーなし(infra/ はルートの ESLint / Prettier 対象外)

人間の確認事項

  • stg へのデプロイと課題一覧の表示確認デプロイ順を守る必要があるので、こちらからは deploy-stg を付けていない:

    1. cdk deploy(テーブル + GSI + Lambda)
    2. 直後に npx ts-node bin/backfill-coteacher-index.ts --apply(既存の共同管理者の索引行を作る)
    3. 課題一覧を確認

    バックフィル前は共同管理の課題・クラスが一覧に出ない(権限は item 上の coTeacherEmails で判定しているので落ちない)。手順は docs/classroom/operations.md

  • prod へのデプロイは行わない(人間の判断で別途。prod も同じ順序でバックフィルが必要)。

Related Issues

Closes #1146

逆引き索引の導入は元 Issue では「スコープ外(まずは Scan 統合 + ページングで足りるか見る)」としていたが、レビューでの明示的な指示により本 PR に含めている。

GET /classrooms は #1138 で Classrooms のフルスキャンが 2 回になっていた。課題単位の
共同管理 (contains(coTeacherEmails)) とクラス経由の課題 (groupId IN (...)) を 1 つの
Scan に統合する。RCU はフィルタ適用前に読んだ項目に課金されるため Scan 回数がそのまま
課金に効く。IN のオペランド上限 100 のためクラスが 100 を超える場合はチャンク分割し、
email 述語は先頭チャンクにだけ載せて二重評価を避ける。

あわせて Scan/Query のページング未対応を解消する。DynamoDB の 1MB 上限はフィルタ適用
前に効くため、LastEvaluatedKey を辿らないとテーブル肥大時にエラー無しで取りこぼす。
queryAll / scanAll ヘルパーを追加し一覧系の読み取りを寄せた。無限ループ防止に
DDB_MAX_PAGES (既定 25) で打ち切り、打ち切り時は warn を残す。

Closes #1146

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@smalruby3-editor-bot smalruby3-editor-bot Bot added the 🤖 autopilot autopilot が処理対象にした Issue/PR(daemon が付与) label Aug 29, 2026
@smalruby3-editor-bot

smalruby3-editor-bot Bot commented Aug 29, 2026

Copy link
Copy Markdown
Author

🤖 autopilot status

field value
Status Review
AI Status
HITL Yes
Size middle

Linked issue #1146. Maintained by autopilot (single writer); do not edit.

smalruby3-editor-bot Bot and others added 2 commits August 29, 2026 02:49
レビュー指摘の反映 (#1146)。

- 列挙結果がそのまま書き込みを駆動する経路が未ページングのまま残っていた。
  1MB 上限はフィルタ適用前に効くので、取りこぼすと「一部だけ更新されない」
  状態になり、読み取りの取りこぼしより気付きにくい:
  - handleMigrateGroups (組への移行プランが不完全になる)
  - propagateStudentCountToClassrooms (一部の課題が古い人数のまま残る)
  - 課題名の重複連番 (既存名を見落として重複を配ってしまう)
  - 前回コメントの recap (best-effort だが同じ列挙パターン)
- DDB_MAX_PAGES は Lambda の environment 許可リストに無く、どのステージでも
  設定できないのに docs では knob として書かれていた。stack に追加して実態を
  合わせる。あわせて不正値 (NaN / 0 以下) を既定へフォールバックする —
  そのまま使うと 1 ページも読まずに空配列を返し、このヘルパーが無くそうと
  している「黙って取りこぼす」挙動そのものになる。
- テスト: 不正値フォールバック、人数伝播のページング (どちらも RED を確認)。
  「email を持たない教師」テストは dev bypass の identity が常に email を持つ
  ため実態と乖離していたので、実際の性質 (2 述語が 1 Scan に OR 結合される)
  を assert する形に直す。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…isions

import / export を持たないテストファイルは TS の「スクリプト」扱いになり、
ts-jest が 1 プロセスで複数のテストを型付けすると `const mockSend` などが
グローバルスコープで衝突して "Cannot redeclare block-scoped variable" になる。

キャッシュが温まっていると通ってしまうため気付きにくいが、`npx jest
--clearCache` 後は 7 suites が「Test suite failed to run」で丸ごと落ちる
(#1146 で追加したテストも含む = DoD の根拠が実際には走っていなかった)。
smalruby-classroom は ci-infra.yml にジョブが無く CI でも検知されない。

各ファイル末尾に空 export を足してモジュール化し、ファイルごとのスコープに
閉じる。cold cache でも 16 suites / 254 tests green。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[Must] 本 PR が追加したテスト自体が、クリーンな ts-jest キャッシュでは suite ごと実行されない(修正済み: 0286eca

npx jest --clearCache の直後に npx jest を実行すると、handler-list-classrooms-reads.test.ts を含む 7 suites が
● Test suite failed to run — TS2451: Cannot redeclare block-scoped variable 'mockSend' で丸ごと落ちる。

原因は import / export を持たないテストファイルが TS の「スクリプト」扱いになり、ts-jest が 1 プロセスで複数の
テストを型付けするとトップレベルの const mockSend / const DEV_TOKEN がグローバルスコープで衝突すること。
キャッシュが温まっていると通ってしまうため気付きにくい(PR 本文の「16 suites / 252 tests すべて green」は
warm cache の結果)。

影響が「Must」なのは、この PR の DoD の根拠(読み取り回数の固定・ページングの固定)が実際には走っていない
状態で green と報告されていたため。衝突自体は develop 由来(新ファイル無しでも 6 suites が落ちる)だが、
新しいテストがその 7 番目になっている。

対応: 対象 8 ファイルの末尾に空 export {}; を足してモジュール化した。cold cache でも
16 suites / 254 tests green(--maxWorkers=1 / 既定ワーカー数の両方で確認)。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[Must] 「一覧系の呼び出しをページングヘルパーに寄せる」が未完で、列挙結果が書き込みを駆動する経路が取りこぼす(修正済み: ecb3223

Issue #1146 は「Scan / Query を LastEvaluatedKey が無くなるまで辿る共通ヘルパーを用意し、一覧系の呼び出しを
そこに寄せる」を要求している。本 PR が寄せたのは GET /classrooms / GET /classroom-groups の 4 本だけで、
同じ teacherSub-index の全件列挙が 4 箇所そのまま残っていた。うち 2 つは 列挙結果がそのまま書き込みを駆動する
ので、1MB で打ち切られると「一部だけ更新されない」状態になり、読み取りの取りこぼしより気付きにくい:

箇所 取りこぼしたとき何が起きるか
handleMigrateGroupsPOST /classroom-groups/migrate 移行プランが不完全になり、アカウントの一部が未移行のまま残る
propagateStudentCountToClassrooms クラス人数を変えても一部の課題が古い人数のまま残る
課題名の重複連番(handleCreateClassroom 既存名を見落として重複した課題名を配ってしまう
前回コメントの recap(handleJoinClassroom best-effort なので実害は小さいが同じ列挙パターン

.claude/rules の「バグパターンを直すときは報告された 1 箇所だけでなく全インスタンスを確認する」に照らしても、
1 リクエスト内の同種パターンを残したままにすべきではない。

対応: 上記 4 箇所を queryAll に寄せた。加えて人数伝播のページングを固定する回帰テストを追加
LastEvaluatedKey を返すモックで 3 件すべて更新されること。ページングを戻すと RED になることを確認済み)。
単一クラス内に閉じた Query(メンバー・提出物・退室依頼、上限 MAX_STUDENT_COUNT)と Limit 付きの
「上位 N 件」系は対象外のままにし、その線引きを docs/classroom/architecture.md に明記した。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[Question] DDB_MAX_PAGES はどのステージでも設定できない上、不正値だと「黙って空を返す」(修正済み: ecb3223

docs/classroom/architecture.mdDDB_MAX_PAGES(既定 25)を運用 knob として書いているが、
Lambda の environmentlib/classroom-stack.ts明示的な許可リストで、DDB_MAX_PAGES はそこに無い。
つまり .env.<stage> に書いても Lambda には届かず、実質いつでも 25 固定 = ドキュメントと実装の乖離。

さらに parseInt(process.env.DDB_MAX_PAGES || '25', 10) は不正値で NaN になり、page < NaN が常に false なので
1 ページも読まずに空配列を返す。これは本 PR が無くそうとしている「エラーも出さずに黙って取りこぼす」失敗モード
そのもので、しかも全件消失という最悪の形で出る(同ファイルの他の 18 個の parseInt 定数は不正値でも
壊れ方が目に見えるので、ここだけ性質が違う)。

対応: stack の environmentDDB_MAX_PAGES を追加して docs の記述を実態に合わせ、
handler 側で Number.isFinite() && > 0 でなければ既定 25 へフォールバックするようにした
DDB_MAX_PAGES='not-a-number' でも全ページ辿ることを固定するテストを追加、ガードを外すと RED になることを確認済み)。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[Question] テスト「統合後も email を持たない教師では Scan がクラス経由分だけになる」が実態と乖離していた(修正済み: ecb3223

このテストが使う dev bypass の identity は handler.ts:455 で常に
{ sub: 'dev-test-teacher', email: 'dev-test-teacher@example.com' } を返すので、email を持たない教師の経路は
一度も通っていない
。assert も expect(scan.input.FilterExpression).toContain('groupId IN') だけで、
email 述語が載っていても通るため、テスト名が主張する性質を何も守っていなかった。

対応: テスト名と assert を実際の #1146 の性質(2 つの述語が 1 本の FilterExpressionOR 結合され、
Scan が 1 回で済む
)に合わせた。email === null 側の分岐(listAssignmentsInGroups)は
handler-group-co-teacher.test.ts のトピックカスケードのテストが通っているので、カバレッジ自体の穴は無い。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[Question] ページ上限 25 × 1MB は Lambda の memorySize 256MB に対して強気(人間の判断に委ねる。未対応)

MAX_PAGES = 25 は 1 本の読み取りにつき最大 25MB をフィルタ適用前に読んでメモリに積む。
GET /classrooms は 1 リクエストでページングする読み取りを 4 本走らせる(自分の課題 Query / 統合 Scan /
組の Query / 組の Scan、管理クラスが 100 を超えるとチャンク分だけ Scan が増える)ので、理論上の上限は
100MB 超。JS オブジェクトに展開すると生バイト数の数倍になるため、Lambda(memorySize: 256,
timeout: 30s)では テーブルが育ったちょうどその局面で OOM / タイムアウトに化けうる。

つまりトレードオフを「黙って取りこぼす」から「たまに 500 を返す」に付け替えている面がある(検知はできるので
方向としては改善だが、上限の置き方は設計判断)。Issue も上限として「ページ数 or 累計件数」を挙げていたので、
選択肢としては:

  1. 累計件数の上限を足す(打ち切り時は今と同じ warn を残す)
  2. memorySize を上げる(512MB 程度)
  3. 逆引きインデックステーブル(Issue のスコープ外として保留されている本命)

数値の決め(何件を上限とみなすか / どこまでコストをかけるか)は運用判断なので、こちらでは変更せず記載のみ。
現状のテーブル規模(単一組織・課題は 90 日 TTL)では当面問題にならない見込み。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[Question] infra/smalruby-classroom だけ CI にジョブが無い(本 PR では未対応 — .github/workflows/ は bot 権限外)

.github/workflows/ci-infra.yml には mesh-v2 / rubytee-relay / smalruby-api / bug-report の
「Unit Tests」「CDK Build & Synth」「Security Audit」があるが、smalruby-classroom のジョブが 1 つも無い
infra/** の変更で起動するワークフロー自体はあるので、classroom だけ素通りしている状態。

そのため、上の [Must] の cold-cache 破綻(テストが丸ごと走らない)も、npx tsc --noEmit の破綻も CI では
検知されず、「ローカルで green だった」に頼るしかない。classroom は 8 テーブル + 認証/認可を持つ最大の
infra プロジェクトなので、他 4 つと同じ 3 ジョブを足すのが妥当だと思う。

本 PR で対応しない理由: .github/workflows/ は autopilot の bot 権限外パスで、触ると push 経路が個人トークンに
切り替わり 👥 human-review-required が必要になる(.claude/rules/autopilot/prompts.md)。本 PR の主題とも
無関係なので、別 Issue にするのが適切と判断した。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

[FYI] 細かい点(いずれも動作に影響しないため未対応)

  • paginateAlldocClient.send(makeCommand(startKey) as QueryCommand) は、ScanCommand
    QueryCommand と偽るキャスト。実行時は等価なので実害は無いが型としては嘘なので、
    send のパラメータ型に寄せる(as Parameters<typeof docClient.send>[0])ほうが意図は正確。
  • listAssignmentsInGroups は呼び出し 1 箇所だけの 1 行ラッパになった。
    listSharedAssignments(null, [groupId]) を直接呼んでも読めるので、将来整理の余地。
  • listSharedAssignments の「chunks が空なら [] を 1 つ push しておいて、
    節が 0 個なら continue」というセンチネルは、groupIds が空のときに email だけの Scan を出すための工夫だが
    やや遠回り。email 節と groupId チャンクを別に組み立てるほうが素直に読める。
  • 統合 Scan は contains(...)groupId IN (...) を 1 本にしたぶん、CloudWatch の
    ConsumedCapacity からは「どちらの述語で拾ったか」が見えなくなった。困ることは無さそうだが、
    将来どちらが効いているか知りたくなったら分けて計測する必要がある。

@smalruby3-editor-bot

Copy link
Copy Markdown
Author

🤖 敵対的レビュー完了。指摘 7 件(Must 2 / Question 4 / FYI 1)。Must は修正済み。

分類 指摘 対応
Must 追加したテストが cold cache では suite ごと走らない(ts-jest のグローバル衝突) 修正済み 0286eca993
Must 「一覧系をページングヘルパーに寄せる」が未完(書き込みを駆動する列挙 4 箇所が残存) 修正済み ecb3223590
Question DDB_MAX_PAGES が stack env に無く設定不能 + 不正値で黙って空を返す 修正済み ecb3223590
Question 「email を持たない教師」テストがその経路を通っていない 修正済み ecb3223590
Question ページ上限 25 × 1MB × 4 本 vs memorySize: 256 の余裕 記載のみ(設計判断)
Question smalruby-classroom だけ CI ジョブが無い 記載のみ(.github/workflows/ は bot 権限外)
FYI 型キャスト / 1 行ラッパ / センチネルの読みやすさ 対応せず

検証: npx jest --clearCache 後に npx jest / npx jest --maxWorkers=1 の両方で 16 suites / 254 tests green
npx tsc --noEmit エラーなし。Must の 2 件はいずれも修正前に RED になることを確認してから直している。

人間の確認事項(PR 本文のとおり未実施): stg へのデプロイと課題一覧の表示確認。prod へのデプロイは行わない。

@smalruby3-editor-bot smalruby3-editor-bot Bot added the 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影) label Aug 29, 2026
@smalruby3-editor-bot
smalruby3-editor-bot Bot marked this pull request as ready for review August 29, 2026 02:53

@takaokouji takaokouji 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.

conflict を解消して、最新のmainに追従して。

Question | ページ上限 25 × 1MB × 4 本 vs memorySize: 256 の余裕 | 記載のみ(設計判断)

逆引きindexを導入して。

Question | smalruby-classroom だけ CI ジョブが無い | 記載のみ(.github/workflows/ は bot 権限外)

対象外でOK

FYI | 型キャスト / 1 行ラッパ / センチネルの読みやすさ | 対応せず

ここで修正しないと今後も修正しないので、すべて対応して。

@takaokouji takaokouji removed the 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影) label Aug 29, 2026
@smalruby3-editor-bot smalruby3-editor-bot Bot added the 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影) label Aug 29, 2026
# Conflicts:
#	infra/smalruby-classroom/lambda/handler.ts
#	infra/smalruby-classroom/lambda/tests/handler-archive-recovery.test.ts
#	infra/smalruby-classroom/lambda/tests/handler-co-teacher-authz.test.ts
#	infra/smalruby-classroom/lambda/tests/handler-group-co-teacher.test.ts
#	infra/smalruby-classroom/lambda/tests/handler-group-seatcount.test.ts
#	infra/smalruby-classroom/lambda/tests/handler-notifications.test.ts
#	infra/smalruby-classroom/lambda/tests/handler-shared-assignments.test.ts
#	infra/smalruby-classroom/lambda/tests/handler-shared-recommend-projection.test.ts
@smalruby3-editor-bot

Copy link
Copy Markdown
Author

🤖 base 追従のコンフリクトを解決しました(ddcc47a2

develop の #1145(クラス単位の列挙を groupId 起点に統一)と本 PR(読み取り削減 + ページング)が同じ関数を触っていたため衝突していた。両方の意図を残す形で解決した。

解決方針

競合箇所 採用 理由
handler.ts 3 箇所(同名課題のオートナンバリング / 前回コメント再掲 / 人数の波及) develop 側の listAssignmentsInGroups([groupId]) 本 PR ではこの helper が listSharedAssignmentsscanAll に委譲されているので、groupId 起点(#1145)とページング(本 PR)が同時に成立する
同上・v2 以前の課題のフォールバック queryAll に置換 develop 側は生の QueryCommand だった。1MB で打ち切られると既存の課題名を見落として重複した課題名を配ってしまうため、本 PR のページング保証を効かせた
テスト 7 ファイル develop 側(先頭の export {}; + コメント) 同じ問題(#1144 の TS2451 衝突)を両ブランチが別々に直していた。develop 側には回帰テスト test-files-are-modules.test.ts も入っているのでそちらに寄せ、本 PR が足していた重複分を除去した

マージに伴うテスト更新

handler-group-seatcount.test.ts の「LastEvaluatedKey が返るときも全ページ分の課題を更新する」が落ちたので、実装の変化に追随させた(アサーションの意図は変えていない):

  • ページングのモックを QueryCommand → ScanCommand に変更(列挙が teacherSub の Query から groupId 起点の Scan に変わったため)
  • 行データに status: 'active' を追加(status の絞り込みが DynamoDB のフィルタから JS 側に移ったため)

確認

  • npx tsc --noEmit: エラーなし
  • jest --clearCache && npm test: 18 suites / 279 tests すべて green(cold cache)
  • develop から 0 commit behind

人間の判断が要る論点は出なかったのでそのまま進めた(どちらを採るかは「両立できる形が 1 つだけある」ケースだった)。

@smalruby3-editor-bot smalruby3-editor-bot Bot removed the 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影) label Aug 29, 2026
smalruby3-editor-bot Bot and others added 2 commits August 29, 2026 11:47
レビュー指摘(#1148)への対応の第一段。coTeacherEmails は list 属性で GSI に
できないため、email → 資源 の逆引き行を持つ ClassroomCoTeacherIndex テーブルを
追加し、共同管理者リストを書き換えるたびに差分同期する。あわせて課題を組で
引くための groupId-index GSI を追加した(読み取りの切り替えは次段)。

paginateAll の ScanCommand → QueryCommand キャストは型の嘘だったので、
instanceof で分岐して本来の出力型を得る形に直した。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@smalruby3-editor-bot

Copy link
Copy Markdown
Author

🤖 レビューへの対応(途中・チェックポイント)

いただいた 4 点のうち 1 点は対応済み、逆引き index は土台まで進めて時間切れの区切りで一旦止めました(残りは tmp/autopilot-continuation-1146.md に記載)。

ご指摘 状態
conflict を解消して最新に追従して ✅ 完了(ddcc47a2origin/develop から 0 commit behind、この PR は MERGEABLE
逆引き index を導入して 🚧 土台まで(テーブル + GSI + 書き込み同期。読み取り切り替え・バックフィルは残)
CI ジョブが無い → 対象外で OK 対応しません(別 Issue 化も本 PR ではしません)
FYI をすべて対応して 🚧 1/4 完了。残り 3 件は読み取り切り替えで消える構造なので、そこで一緒に対応します

今回入れたもの(f9af3d91

  • ClassroomCoTeacherIndex{suffix} テーブル(PK coTeacherEmail / SK resourceKey = assignment#<id> | group#<id>、TTL は指す資源のものを写す)。coTeacherEmails は list 属性で GSI にできないため、逆引きは別テーブルになります。
  • groupId-index GSI(Classrooms)。groupId IN (...) の全表 Scan を、組ごとの Query に置き換えるための下地。
  • 書き込み同期 syncCoTeacherIndex(): 共同管理者リストを書き換える経路(課題の追加/削除、組の更新)から差分だけ Put/Delete。認可は今も item 上の coTeacherEmails を見るので、索引が遅れてもアクセス権は落ちません。
  • FYI 1(型の嘘): paginateAllas QueryCommand を廃止。ご提案の Parameters<typeof docClient.send>[0] は戻り値が ServiceOutputTypes に潰れて Items / LastEvaluatedKey が引けなくなったので、instanceof ScanCommand で分岐して本来の出力型を得る形にしました。

読み取りはまだ何も変えていないので、この時点で挙動は不変です(npx tsc --noEmit エラーなし、npx jest 18 suites / 279 tests green)。

残り(次の再開で対応)

  1. 読み取りを索引へ切り替え、GET /classrooms から Classrooms / ClassroomGroups の Scan を 0 にする。
  2. その過程で listSharedAssignments を「email 逆引き」と「groupId-index 引き」に分割 → FYI 2(1 行ラッパ)・FYI 3(センチネル)・FYI 4(述語ごとに計測できない)が同時に解消します。
  3. 既存データのバックフィルスクリプト + runbook。読み取り切り替えを反映する前に実行しないと既存の共同管理者が一覧から消えるため、デプロイ順(テーブル/GSI 作成 → backfill → Lambda 反映)を docs に明記します。
  4. テスト更新(Scan 0 の固定、同期の回帰、backfill)と docs/classroom/architecture.md / PR 本文の更新。

なお Scan が無くなるので、Question に挙げていた「ページ上限 25 × 1MB × 4 本 vs memorySize: 256」は前提ごと解消する見込みです(フィルタ前に読む量がテーブルサイズに比例しなくなるため)。

継続してよろしければ 🙋 HITL を外してください。そのまま残タスクの続きから進めます。

@smalruby3-editor-bot smalruby3-editor-bot Bot added the 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影) label Aug 29, 2026
@smalruby3-editor-bot
smalruby3-editor-bot Bot marked this pull request as draft August 29, 2026 11:49
@github-actions

Copy link
Copy Markdown

🚀 Preview deployed: https://smalruby.jp/smalruby3-editor/topic/autopilot-1146/

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@smalruby3-editor-bot

Copy link
Copy Markdown
Author

🤖 レビューへの対応(完了)

前回のチェックポイントで残していた作業を終えました。いただいた 4 点はすべて処理済みです。

ご指摘 状態
conflict を解消して最新に追従して ✅ 完了(ddcc47a2origin/develop から 0 commit behind)
逆引き index を導入して 完了(テーブル + GSI + 書き込み同期 + 読み取り切り替え + バックフィル)
CI ジョブが無い → 対象外で OK 対応しません(本 PR では別 Issue 化もしません)
FYI をすべて対応して ✅ 4/4 完了

読み取りを索引に切り替えた(GET /classrooms の Scan が 0 に)

  • 共同管理(課題単位・クラス単位)→ 逆引き索引 ClassroomCoTeacherIndex の Query + BatchGetItem
  • クラスに属する課題 → Classrooms の新 GSI groupId-index をクラスごとに Query

Scan は RCU も 1MB 上限もフィルタ適用「前」に効くので、コストが「その先生に見える量」ではなく テーブル全体の量(他の先生のデータ量) に比例していました。索引経由になったのでそこが切れています。

認可は索引を見ません(今も item 上の coTeacherEmails が真実)。索引は「一覧に出す候補を集める」ためだけのもので、引いた資源のリストで再確認します。なので索引が遅れても権限は落ちず、一覧に出るのが遅れるだけです。この性質はテストでも固定しました(索引に消し忘れの行があっても一覧に混ざらないこと)。

FYI 4 件の始末

FYI どうなったか
as QueryCommand の型の嘘 instanceof ScanCommand で分岐して本来の出力型を得る形に(ご提案の Parameters<typeof docClient.send>[0] は戻り値が ServiceOutputTypes に潰れて Items / LastEvaluatedKey が引けなくなったため別の形にしました)
listAssignmentsInGroups が 1 行ラッパ groupId-index を引く実体になったのでラッパではなくなりました
chunks のセンチネル listSharedAssignments 自体を削除したので消滅
述語ごとに計測できない 索引 Query と groupId-index Query が別コマンドになったので CloudWatch で分けて見えます

あわせて、「ページ上限 25 × 1MB × 4 本 vs memorySize: 256」の Question も前提ごと解消しました(フィルタ前に読む量がテーブルサイズに比例しなくなったため)。

書き込み側の取りこぼしも埋めました

coTeacherEmails の書き手は 4 経路あり、v2 移行(POST /classroom-groups/migrate)が課題の共同管理者を組へ引き上げる分が同期から漏れていました。索引に書かない経路が残ると その共同管理者にだけ資源が一覧に出ない(権限は落ちないので直リンクでは操作できる)という気付きにくい壊れ方になるため、4 経路すべてを回帰テストで固定しています。

バックフィルが必要です(デプロイ順に注意)

索引は後付けなので、それ以前に登録された共同管理者の行がありませんbin/backfill-coteacher-index.ts(既定 dry-run・冪等)を用意しました。

1. cdk deploy(テーブル + GSI + Lambda)
2. 直後に npx ts-node bin/backfill-coteacher-index.ts --apply
3. 課題一覧を確認

この順序を守る必要があるため、こちらからは deploy-stg ラベルを付けていません(付けるとバックフィル前の状態で stg が動く時間ができます)。手順は docs/classroom/operations.md に runbook として書きました。

確認

  • jest --clearCache && npx jest: 21 suites / 314 tests すべて green(cold cache)
  • npx tsc --noEmit: エラーなし
  • テストは Scan 前提のモックを索引・GSI に追随させ、co-teacher のモックは一覧経路で Scan が来たら throw するようにしています(退行が green にならないように)

なお逆引き index は元 Issue では「スコープ外」としていた項目ですが、ご指示に従って本 PR に含めています。PR 本文にもその旨を残しました。

@smalruby3-editor-bot smalruby3-editor-bot Bot added 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影) deploy-stg この PR の infra 変更を stg にデプロイする (#1162) labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🚀 stg にデプロイしました: smalruby-classroom

項目
commit fb3749d9afcc45ce36c7ec6244f3c206e0875d70
branch topic/autopilot-1146
run https://github.com/smalruby/smalruby3-editor/actions/runs/33498411471

stg は共有環境です。この SHA が載っている前提で 動作確認してください。
別の PR があとからデプロイすると上書きされます(その場合はラベルを付け直して再デプロイ)。

一覧経路の Scan を逆引き索引と GSI の Query に置き換えたことで、handler の
scanAll は呼び出し元がゼロになっていた。残しておくと「ページングしてくれる
Scan」という体裁で表全体を読む経路が再び生えやすいので、paginateAll ごと
Query 専用にする。ScanCommand を narrow するための instanceof 分岐も不要に
なった(Query/Scan の union を送るためだけの分岐だったため)。

docs/classroom/architecture.md のページングの記述もヘルパー名に追随させる。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@smalruby3-editor-bot

Copy link
Copy Markdown
Author

🤖 レビュー対応の再確認(新しいご指摘なし・追加修正 1 件)

差し戻しを受けて PR 全体(diff + 全コメント + レビュー)を読み直しましたが、前回の完了報告(07:04)以降に新しいご指摘は入っていませんでした。既存の指摘がすべて実際に反映されているかを、コードとテストで検証し直しています。

指摘の棚卸し(人間 4 件 / bot 自身 7 件 = 11 件)

出所 指摘 状態
レビュー conflict を解消して最新に追従 origin/develop から 0 commit behind(再確認済み)
レビュー 逆引き index を導入 ✅ 索引テーブル + groupId-index + 書き込み同期 + 読み取り切り替え + バックフィル
レビュー CI ジョブが無い → 対象外で OK 対応不要(ご指示どおり本 PR では扱わない)
レビュー FYI をすべて対応 ✅ 4/4(下記のとおり 1 件だけ残骸があったので今回落とした)
bot Must 2 / Question 4 / FYI 1 ✅ すべて解消(memorySize の Question は Scan 全廃で前提ごと消滅)

実コードでの確認: as QueryCommand のキャストなし / listSharedAssignments は削除済み / DDB_MAX_PAGES は stack の environment にあり不正値は既定 25 へフォールバック / syncCoTeacherIndex の呼び出しは 4 経路。

追加修正 1 件(1161cefff7

FYI 対応の残骸として、scanAll が呼び出し元ゼロの死にコードになっていました(一覧経路の Scan を索引と GSI の Query に置き換えたため)。「ページングしてくれる Scan」という体裁のヘルパーが残っていると、この PR が消したはずの「表全体を読む経路」が将来また生えやすいので落としました。あわせて paginateAll を Query 専用にし(ScanCommand を narrow するための instanceof 分岐も不要になった)、docs/classroom/architecture.md のヘルパー名の記述を追随させています。実行時の挙動は変わりません。

⚠️ stg: デプロイは済んでいますが バックフィルがまだです

deploy-stg10:42 のデプロイfb3749d9)が完了していますが、deploy-infra-stg.ymlcdk deploy しか実行しません。runbook のステップ 2 が未実行です:

1. cdk deploy                                       ← 済(Actions)
2. npx ts-node bin/backfill-coteacher-index.ts --apply   ← 未実行
3. 課題一覧を確認

このままだと既存の共同管理者に課題・クラスが一覧表示されません(権限は item 上の coTeacherEmails が真実なので落ちません。一覧に出ないだけです)。確認の前にバックフィルを流してください。autopilot のコンテナには AWS クレデンシャルが無い(SSO ログインが対話的で無人実行できない)ため、こちらからは実行できません。手順は docs/classroom/operations.md

なお今回の追加コミットは未使用関数の削除だけなので、stg の再デプロイは必須ではありません(SHA を厳密に合わせたい場合は deploy-stg を付け直してください)。

確認

  • jest --clearCache && npx jest: 21 suites / 314 tests green(cold cache・今回の修正後)
  • npx tsc --noEmit: エラーなし

コード変更が入ったので再レビューをお願いします(レビューが CHANGES_REQUESTED のままなのでマージがブロックされています)。

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🚀 stg にデプロイしました: smalruby-classroom

項目
commit 1161cefff75d476802f9a31fb0d0a3936a0963b2
branch topic/autopilot-1146
run https://github.com/smalruby/smalruby3-editor/actions/runs/33499558624

stg は共有環境です。この SHA が載っている前提で 動作確認してください。
別の PR があとからデプロイすると上書きされます(その場合はラベルを付け直して再デプロイ)。

develop 由来(#1133)の `handler-classroom-groups.test.ts` が import/export を
持たないため、admin の `test-files-are-modules.test.ts`(#1144/#1155 の回帰テスト)が
落ち、infra/** を触るすべての PR で `[admin] Unit Tests` が赤になっていた。

本 PR は smalruby-admin を触っていないが、この PR の CI もそれで赤いままなので、
classroom 側で使っているのと同じ `export {};` の宣言を足して解いておく。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@smalruby3-editor-bot

smalruby3-editor-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown
Author

🤖 レビュー対応(CI の赤を 1 件解消・新しいご指摘なし)

差し戻しを受けて PR 全体(diff + 全コメント + レビュー + inline)を読み直しましたが、前回の再確認(10:52)以降に新しいご指摘は入っていません(inline コメントも 0 件)。既存の指摘 11 件(レビュー 4 / bot 自身 7)の対応状況は前回コメントの表のとおりで変化はありません。

かわりに、前回の完了後に回った CI で 新しい赤が出ていたので、それを解消しました。

[admin] Unit Tests が赤だった(08ea801f31 で解消)

前回の push(1161cefff7)で回った run 33499558651[admin] Unit Tests が失敗しています。本 PR とは無関係の develop 由来の破綻です:

本 PR のスコープ外ですが、この PR の CI が赤のままだと再レビュー・マージの判断ができないので、classroom 側で使っているのと同じ 1 行の宣言(export {}; + 由来コメント)を足して解いておきました。実行時の挙動は変わりません。

$ ts.isExternalModule() を admin の tests 9 ファイルに適用
OK   classroom-overview.test.ts / cors-origins.test.ts / handler-classroom-groups.test.ts
OK   handler-notifications.test.ts / handler-recommend-sharing.test.ts
OK   handler-shared-recommendation.test.ts / handler.test.ts / restore-plan.test.ts
OK   test-files-are-modules.test.ts

[FYI] smalruby-api(5 ファイル)・bug-report(2)・rubytee-relay(1)にも import/export を持たないテストファイルが残っています。これらのプロジェクトには test-files-are-modules の回帰テストが無いので現状 CI は緑で、今回は触っていません(同じ cold-cache 破綻の予備軍ではあります)。

⚠️ stg のバックフィルは依然として未実行です

10:55 のデプロイ1161cefff7)まで済んでいますが、deploy-infra-stg.ymlcdk deploy しか行わないため runbook のステップ 2 が未実行のままです:

1. cdk deploy                                            ← 済(Actions)
2. npx ts-node bin/backfill-coteacher-index.ts --apply    ← 未実行(人間の作業)
3. 課題一覧を確認

バックフィル前は既存の共同管理者に課題・クラスが一覧表示されません(権限は item 上の coTeacherEmails が真実なので落ちません。一覧に出ないだけです)。動作確認の前に流してください。autopilot のコンテナには AWS クレデンシャルが無い(SSO ログインが対話的で無人実行できない)ため、こちらからは実行できません。手順は docs/classroom/operations.md

なお今回の追加コミットは admin のテスト 1 ファイルへの宣言追加だけなので、stg の再デプロイは不要です。

マージがブロックされている理由

レビューが CHANGES_REQUESTED のままなので mergeStateStatusBLOCKED です。ご指摘 4 点はすべて処理済み(1 点は「対象外で OK」のご指示どおり非対応)なので、再レビューをお願いします


追記(11:10 時点・コード変更なし)

差し戻しを受けて再度 PR 全体(diff / 全コメント / レビュー / inline)を確認しましたが、新しいご指摘は入っていません(inline 0 件)。既存指摘の対応状況も上表から変化はありません。コードには触っていないので、この追記は状態の更新だけです。

  • CI が全ジョブ green になりました08ea801f31run 33500337776)。前回コメント時点で赤かった [admin] Unit Tests を含め 22 チェックすべて pass。
  • origin/develop から 0 commit behind(再確認済み)。MERGEABLE
  • 実コードでの再検証: handler.tsScanCommand の使用 0 件 / scanAlllistSharedAssignmentsas QueryCommand は残骸なし / syncCoTeacherIndex の呼び出しは 4 経路 / DDB_MAX_PAGES は stack の environment にあり不正値は既定 25 へフォールバック。

残っているのは人間の作業 2 つだけです:

  1. stg のバックフィル(未実行)— npx ts-node bin/backfill-coteacher-index.ts --apply。これを流すまで既存の共同管理者に課題・クラスが一覧表示されません(権限は落ちません)。手順は docs/classroom/operations.md。autopilot のコンテナには AWS クレデンシャルが無いため、こちらからは実行できません。
  2. 再レビューCHANGES_REQUESTED が残っているためマージが BLOCKED です。

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🚀 stg にデプロイしました: smalruby-classroom

項目
commit 08ea801f31ca62b39cb3fe7716bfcbbb0e6e71c3
branch topic/autopilot-1146
run https://github.com/smalruby/smalruby3-editor/actions/runs/33500337911

stg は共有環境です。この SHA が載っている前提で 動作確認してください。
別の PR があとからデプロイすると上書きされます(その場合はラベルを付け直して再デプロイ)。

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🚀 stg にデプロイしました: smalruby-admin

項目
commit 08ea801f31ca62b39cb3fe7716bfcbbb0e6e71c3
branch topic/autopilot-1146
run https://github.com/smalruby/smalruby3-editor/actions/runs/33500337911

stg は共有環境です。この SHA が載っている前提で 動作確認してください。
別の PR があとからデプロイすると上書きされます(その場合はラベルを付け直して再デプロイ)。

@smalruby3-editor-bot
smalruby3-editor-bot Bot marked this pull request as ready for review September 1, 2026 11:10
stg で実際に流したところ、cdk deploy 直後の groupId-index は CREATING で、
項目 100 件程度でも ACTIVE まで約 5 分かかった。この間はその索引への Query が
失敗しうるので、待つ手順を明記する。

あわせて dry-run が索引テーブル無しでも動くこと(デプロイ前に件数を確認できる)
とステージ切り替えの方法を追記する。

Refs #1146

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@smalruby3-editor-bot

Copy link
Copy Markdown
Author

🤖 stg でデプロイ → バックフィル → 検証まで実施しました(人間と一緒に実行)。レビューの材料としてどうぞ。

実施内容と結果

手順 結果
stg デプロイ(deploy-stg ラベル) ✅ 成功。ラベル無しの前 run が skipped だったのでゲートも効いている
dry-run(デプロイ前に実施) scanned: 8 assignments / 93 groups → 2 index rows
--apply wrote 2 index rows(予測と一致)
索引テーブル ClassroomCoTeacherIndex-stg に 2 行
冪等性 ✅ 再実行しても同じ行を上書きするだけ

バックフィルの意味を実データで再現しました

「別の先生が所有し、dev アカウントを共同管理者にしたクラス」を stg に一時作成して確認:

backfill 前: 一覧に出ない        ← 索引に行が無い=まさに障害の形
backfill 後: role="co-teacher" で一覧に出る

検証用データは削除済み(索引も本来の 2 行に戻っています)。

実運用で見つかった手順の抜け(6be7d5a7 に追記済み)

groupId-index は deploy 直後 CREATING で、項目 100 件程度の stg でも ACTIVE まで約 5 分
かかりました。この間はその索引への Query が失敗しうるので、runbook のデプロイ順に
「GSI が ACTIVE になるまで待つ」を 1 手順として追加しました(待機コマンド付き)。

あわせて dry-run は索引テーブルが無くても動く(=デプロイ前に件数を確認でき、一覧に出ない窓を
短くできる)ことも明記しました。

prod の規模(dry-run 済み・書き込みなし)

stage=prod  scanned: 83 assignments / 84 groups → 16 index rows for 7 co-teacher emails

実ユーザーの共同管理者 7 名 / 16 行が対象です。prod にはまだ索引テーブルも groupId-index
ありません(=未適用)。

prod は「この PR をマージしてから develop ベースで適用する」方針で人間と合意しました。
マージ後に「デプロイ → GSI が ACTIVE → backfill --apply → 検証」の順で実施します。

レビュー時に見ていただきたい点

  • 読み取りの切り替え(Scan 全廃 → 逆引き索引 + groupId-index)に見落としが無いか
  • 書き込み同期(syncCoTeacherIndex)の呼び出し漏れが無いか(移行 POST /classroom-groups/migrate を含む)
  • backfill の対象(アーカイブ済みも含む)と冪等性の考え方

参考: 別件で見つけた設定の不整合(この PR の対象外)

infra/smalruby-classroom/.env.stgCORS_ALLOWED_ORIGINShttp://localhost:8601 だけを
固定しており、8601〜8610 に広げた変更が classroom では効いていません(env 指定が既定より優先)。
smalruby-admin は未設定(既定どおり)、bug-report は 8601,8602 を明示、とプロジェクトごとに
バラバラです。8601 が別の dev server に使われていると、ローカルから stg API を叩けません。
別 Issue にするか、ご判断ください。

@smalruby3-editor-bot smalruby3-editor-bot Bot removed the deploy-stg この PR の infra 変更を stg にデプロイする (#1162) label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🚀 stg にデプロイしました: smalruby-admin

項目
commit 826f5deccada6268f0ce63736b9d5ac0048cee86
branch topic/autopilot-1146
run https://github.com/smalruby/smalruby3-editor/actions/runs/33508791248

stg は共有環境です。この SHA が載っている前提で 動作確認してください。
別の PR があとからデプロイすると上書きされます(その場合はラベルを付け直して再デプロイ)。

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🚀 stg にデプロイしました: smalruby-classroom

項目
commit 826f5deccada6268f0ce63736b9d5ac0048cee86
branch topic/autopilot-1146
run https://github.com/smalruby/smalruby3-editor/actions/runs/33508791248

stg は共有環境です。この SHA が載っている前提で 動作確認してください。
別の PR があとからデプロイすると上書きされます(その場合はラベルを付け直して再デプロイ)。

@takaokouji
takaokouji merged commit fca2319 into develop Sep 1, 2026
22 checks passed
@takaokouji
takaokouji deleted the topic/autopilot-1146 branch September 1, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 autopilot autopilot が処理対象にした Issue/PR(daemon が付与) 🙋 HITL 人間の対応待ち(Project HITL=Yes の投影)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(classroom): GET /classrooms の DynamoDB 読み取り削減と Scan/Query のページング対応

1 participant