feat: add data-driven presentation methods (SCQA, PREP, Pyramid)#39
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughYAML駆動のプレゼンテーション手法(SCQA/PREP/Pyramid)を扱うPresentationMethod/PresentationMethodRegistryを新規実装し、Deck生成とLLMプロンプト生成に統合。CLIに Changesプレゼンテーションメソッド機能
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Slidict::Cli::App
participant Registry as PresentationMethodRegistry
participant Deck
participant LlmClient as Slidict::Llm::Client
User->>CLI: slidict new --method scqa
CLI->>Registry: method_for("scqa")
Registry-->>CLI: PresentationMethodインスタンス
CLI->>Deck: new(presentation_method: method)
Deck->>Deck: method_slides生成
CLI->>LlmClient: prompt_for(deck)
LlmClient->>LlmClient: method_prompt_for(deck)
LlmClient-->>CLI: 手法反映済みプロンプト
CLI-->>User: 生成されたスライド
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/slidict/cli/app.rb (1)
38-45: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--methodの検証がインタラクティブな質問より後に行われる
method_for(options[:method])はDeck.newのキーワード引数としてask(...)群より後に評価されるため、--topic等が省略されて対話プロンプトが発生するケースでは、ユーザーが4つの質問すべてに回答した後で初めて不正な--methodのエラー(unknown presentation method ...)が表示されます。既存の--frameworkはparse内のoutput_path_for経由で早期に検証されており、同様に--methodも早期解決すべきです。🐛 修正案
client = llm_client_for(config) return 1 if client && !verify_connection(client) + method = method_for(options[:method]) deck = Deck.new( topic: ask("What would you like to talk about?", options[:topic]), duration: ask("How long is the presentation?", options[:duration]), audience: ask("Who is the audience?", options[:audience]), goal: ask("What should the audience remember or do?", options[:goal]), framework: options[:framework], - presentation_method: method_for(options[:method]) + presentation_method: method )🤖 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 `@lib/slidict/cli/app.rb` around lines 38 - 45, `method_for` in `App#run` is being evaluated too late because it is passed directly into `Deck.new` after the interactive `ask(...)` calls. Resolve `options[:method]` earlier in the flow, ideally in `parse` alongside the existing `output_path_for` validation for `--framework`, so an invalid method fails before any prompts are shown. Keep the `Deck.new` call using the already-resolved presentation method value rather than calling `method_for` inline.
🧹 Nitpick comments (6)
spec/slidict/presentation_method_spec.rb (1)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win不正なYAMLに対する
validate!のテストが不足現在のテストはハッピーパスと未知IDのエラーのみをカバーしています。
validate!(lib/slidict/presentation_method.rb)は必須フィールド欠落・不正なid形式・slidesが空/非配列といった複数の分岐を持つ最も複雑なロジックであり、プラグイン向けの検証として重要です。これらの失敗パスに対するテストの追加を推奨します。it "rejects methods missing required fields" do Tempfile.create(["bad", ".yml"]) do |file| file.write({ "id" => "bad" }.to_yaml) file.flush expect { Slidict::PresentationMethod.load_file(file.path) } .to raise_error(ArgumentError, /missing required fields/) end end🤖 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 `@spec/slidict/presentation_method_spec.rb` around lines 1 - 16, Add missing specs for Slidict::PresentationMethod#validate! to cover invalid YAML and failure paths, not just the happy path and unknown ID case. In spec/slidict/presentation_method_spec.rb, extend the existing PresentationMethodRegistry coverage with examples for missing required fields, invalid id format, and slides being empty or not an array, using Slidict::PresentationMethod.load_file and the validate! behavior to assert the expected ArgumentError messages.lib/slidict/presentation_method.rb (1)
81-91: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win組み込み手法とプラグイン間のID重複を検出していない
default_pathsはビルトインとプラグインのYAMLパスを結合しますが、all/find/fetchはID重複のチェックを行いません。プラグインがビルトインID(例:scqa)や他プラグインと同じIDを宣言した場合、sort_by(&:id)後のfindは最初にマッチしたものを黙って返し、他方は理由の説明もなく無視されます。プラグインエコシステムを想定した設計であるため、重複ID検出とエラー化を追加することを推奨します。♻️ 修正案
def all - `@all` ||= `@paths.sort.map` { |path| PresentationMethod.load_file(path) }.sort_by(&:id) + `@all` ||= begin + methods = `@paths.map` { |path| PresentationMethod.load_file(path) }.sort_by(&:id) + methods.group_by(&:id).each do |id, group| + raise ArgumentError, "duplicate presentation method id #{id}: #{group.map(&:source_path).join(', ')}" if group.size > 1 + end + methods + end + end🤖 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 `@lib/slidict/presentation_method.rb` around lines 81 - 91, The PresentationMethod registry currently loads all YAML files in `all` and lets `find`/`fetch` return the first matching ID, so duplicate IDs from built-ins and plugins are silently accepted. Update `PresentationMethod.all` to detect duplicate `id` values after loading, and raise an error that identifies the conflicting methods/paths before memoizing the list. Keep `find` and `fetch` behavior simple, but ensure they can never observe ambiguous IDs by enforcing uniqueness during the load step.lib/slidict/deck.rb (1)
9-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueキーワード引数が7個になり RuboCop の
Metrics/ParameterLists規約に抵触
initializeのキーワード引数が7個となり、静的解析ツールの指摘通り上限(5)を超えています。将来さらに属性が増える可能性を考えると、Options Hash や専用の値オブジェクトへの集約を検討する余地があります。ただし現状は動作に問題なく、優先度は低めです。As per static analysis hints, "Avoid parameter lists longer than 5 parameters. [7/5] (Metrics/ParameterLists)".
🤖 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 `@lib/slidict/deck.rb` around lines 9 - 17, The Deck#initialize signature is over the Metrics/ParameterLists limit because it takes too many keyword arguments. Refactor the constructor to reduce the parameter count by grouping related inputs into a single options hash or a dedicated value object, and keep the existing attribute assignment logic in initialize while preserving the current defaults and behavior.Source: Linters/SAST tools
lib/slidict/llm/client.rb (1)
77-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
method_prompt_forが RuboCop のMetrics/MethodLength規約に抵触12行で上限(10)を超えています。スライド一覧の整形部分(Line 81-83)を小さなヘルパーに切り出すと解消できますが、機能的な問題はなく優先度は低めです。
As per static analysis hints, "Method has too many lines. [12/10] (Metrics/MethodLength)".
♻️ 整形部分をヘルパーに切り出す例
def method_prompt_for(deck) method = deck.presentation_method return "" unless method - slides = method.slides.each_with_index.map do |slide, index| - "#{index + 1}. #{slide.title} — role: #{slide.role}; instructions: #{slide.instructions}" - end.join("\n") + slides = formatted_method_slides(method) instructions = method.ai_instructions.map { |item| "- #{item}" }.join("\n") <<~METHOD Presentation method: #{method.name} (#{method.id}) Method description: #{method.description} Required slide roles: #{slides} Method-specific generation instructions: #{instructions} METHOD end + + def formatted_method_slides(method) + method.slides.each_with_index.map do |slide, index| + "#{index + 1}. #{slide.title} — role: #{slide.role}; instructions: #{slide.instructions}" + end.join("\n") + end🤖 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 `@lib/slidict/llm/client.rb` around lines 77 - 93, The method_prompt_for method is too long and violates Metrics/MethodLength. Extract the slide-list formatting logic in the method_prompt_for flow into a small helper (for example, a dedicated method that builds the required slide roles block) and keep method_prompt_for focused on assembling the final prompt using method, slides, and ai_instructions. Make sure the helper preserves the current formatting and is named clearly so it is easy to locate alongside method_prompt_for.Source: Linters/SAST tools
spec/slidict/cli/app_spec.rb (1)
190-197: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRuboCopが
Style/WordArrayを指摘
["show-method", "scqa"]を%w[show-method scqa]に変更することを推奨します。🎨 修正案
- status = cli.run(["show-method", "scqa"]) + status = cli.run(%w[show-method scqa])🤖 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 `@spec/slidict/cli/app_spec.rb` around lines 190 - 197, RuboCop is flagging the array literal in the `cli.run` expectation setup, so update the `show-method` spec to use a word array instead of a quoted string array. In `app_spec.rb`, replace the `cli.run(["show-method", "scqa"])` argument list with the `%w[...]` form to satisfy `Style/WordArray` while keeping the test behavior unchanged.Source: Linters/SAST tools
lib/slidict/cli/app.rb (1)
253-287: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
show_methodの複雑度がRuboCopの閾値を超過RuboCopが
Metrics/AbcSize(30.03/17)とMetrics/MethodLength(13/10)の違反を報告しています。各出力セクション(suitable_for/slides/review_checklist)を小さなプライベートメソッドに分割すると解消できます。♻️ リファクタ例
def show_method(args) raise ArgumentError, "show-method requires a method id" if args.empty? raise ArgumentError, "show-method accepts exactly one method id" unless args.size == 1 method = method_registry.fetch(args.first) `@output.puts` "#{method.name} (#{method.id})" `@output.puts` "Category: #{method.category}" `@output.puts` "Description: #{method.description}" - `@output.puts` "Suitable for:" - method.suitable_for.each { |item| `@output.puts` " - #{item}" } - `@output.puts` "Slides:" - method.slides.each_with_index { |slide, i| `@output.puts` " #{i + 1}. #{slide.title}: #{slide.role}" } - `@output.puts` "Review checklist:" - method.review_checklist.each { |item| `@output.puts` " - #{item}" } + print_list("Suitable for", method.suitable_for) + print_slides(method.slides) + print_list("Review checklist", method.review_checklist) 0 end + + def print_list(title, items) + `@output.puts` "#{title}:" + items.each { |item| `@output.puts` " - #{item}" } + end + + def print_slides(slides) + `@output.puts` "Slides:" + slides.each_with_index { |slide, i| `@output.puts` " #{i + 1}. #{slide.title}: #{slide.role}" } + end🤖 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 `@lib/slidict/cli/app.rb` around lines 253 - 287, The show_method implementation is too large and too complex for the RuboCop thresholds. Refactor PresentationMethodCLI#show_method by extracting the output sections for suitable_for, slides, and review_checklist into small private helper methods, then have show_method only handle argument validation, lookup via method_registry.fetch, and the high-level output flow.Source: Linters/SAST tools
🤖 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 `@lib/slidict/presentation_method.rb`:
- Around line 43-50: `PresentationMethod.validate!` currently assumes
`attributes["id"]` is a String and calls `match?` directly, which can raise
`NoMethodError` for non-string input. Update the validation to explicitly check
the `id` type before pattern matching, and raise a clear `ArgumentError` when
`id` is not a valid string. Keep the existing `validate!` flow and the
`load_file` rescue behavior aligned so invalid plugin YAML fails with the
intended validation error instead of an unhandled exception.
---
Outside diff comments:
In `@lib/slidict/cli/app.rb`:
- Around line 38-45: `method_for` in `App#run` is being evaluated too late
because it is passed directly into `Deck.new` after the interactive `ask(...)`
calls. Resolve `options[:method]` earlier in the flow, ideally in `parse`
alongside the existing `output_path_for` validation for `--framework`, so an
invalid method fails before any prompts are shown. Keep the `Deck.new` call
using the already-resolved presentation method value rather than calling
`method_for` inline.
---
Nitpick comments:
In `@lib/slidict/cli/app.rb`:
- Around line 253-287: The show_method implementation is too large and too
complex for the RuboCop thresholds. Refactor PresentationMethodCLI#show_method
by extracting the output sections for suitable_for, slides, and review_checklist
into small private helper methods, then have show_method only handle argument
validation, lookup via method_registry.fetch, and the high-level output flow.
In `@lib/slidict/deck.rb`:
- Around line 9-17: The Deck#initialize signature is over the
Metrics/ParameterLists limit because it takes too many keyword arguments.
Refactor the constructor to reduce the parameter count by grouping related
inputs into a single options hash or a dedicated value object, and keep the
existing attribute assignment logic in initialize while preserving the current
defaults and behavior.
In `@lib/slidict/llm/client.rb`:
- Around line 77-93: The method_prompt_for method is too long and violates
Metrics/MethodLength. Extract the slide-list formatting logic in the
method_prompt_for flow into a small helper (for example, a dedicated method that
builds the required slide roles block) and keep method_prompt_for focused on
assembling the final prompt using method, slides, and ai_instructions. Make sure
the helper preserves the current formatting and is named clearly so it is easy
to locate alongside method_prompt_for.
In `@lib/slidict/presentation_method.rb`:
- Around line 81-91: The PresentationMethod registry currently loads all YAML
files in `all` and lets `find`/`fetch` return the first matching ID, so
duplicate IDs from built-ins and plugins are silently accepted. Update
`PresentationMethod.all` to detect duplicate `id` values after loading, and
raise an error that identifies the conflicting methods/paths before memoizing
the list. Keep `find` and `fetch` behavior simple, but ensure they can never
observe ambiguous IDs by enforcing uniqueness during the load step.
In `@spec/slidict/cli/app_spec.rb`:
- Around line 190-197: RuboCop is flagging the array literal in the `cli.run`
expectation setup, so update the `show-method` spec to use a word array instead
of a quoted string array. In `app_spec.rb`, replace the `cli.run(["show-method",
"scqa"])` argument list with the `%w[...]` form to satisfy `Style/WordArray`
while keeping the test behavior unchanged.
In `@spec/slidict/presentation_method_spec.rb`:
- Around line 1-16: Add missing specs for Slidict::PresentationMethod#validate!
to cover invalid YAML and failure paths, not just the happy path and unknown ID
case. In spec/slidict/presentation_method_spec.rb, extend the existing
PresentationMethodRegistry coverage with examples for missing required fields,
invalid id format, and slides being empty or not an array, using
Slidict::PresentationMethod.load_file and the validate! behavior to assert the
expected ArgumentError messages.
🪄 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: 8df2113e-c671-40ff-8143-dc25e76242c3
📒 Files selected for processing (13)
CHANGELOG.mdREADME.mddata/slidict/methods/prep.ymldata/slidict/methods/pyramid.ymldata/slidict/methods/scqa.ymllib/slidict.rblib/slidict/cli/app.rblib/slidict/deck.rblib/slidict/llm/client.rblib/slidict/presentation_method.rblib/slidict/version.rbspec/slidict/cli/app_spec.rbspec/slidict/presentation_method_spec.rb
### Motivation
- Provide a data-first way to encode presentation techniques so anyone can add or improve methods by adding a single YAML file.
- Keep the gem itself minimal (load/validate/mapping only) and enable contributors to publish methods as small data-only PRs or external gems.
- Make AI generation, local templates, and CLI discoverability work with the same method data to support future scale (100+ methods) and localization.
### Description
- Add `Slidict::PresentationMethod` and `Slidict::PresentationMethodRegistry` in `lib/slidict/presentation_method.rb` to load, validate, and map YAML method files to Ruby objects.
- Add three built-in method definitions under `data/slidict/methods/`: `scqa.yml`, `prep.yml`, and `pyramid.yml`, each including `id`, `name`, `category`, `description`, `suitable_for`, `slides` (with `title`, `role`, `instructions`), `ai_instructions`, `review_checklist`, optional `references`, and `locale`.
- Wire methods into the CLI and generation flow by adding `--method` support, a `list-methods` command, a `show-method <id>` command, and accepting `new` as a user-friendly alias for generating slides in `lib/slidict/cli/app.rb`.
- Make `Slidict::Deck` accept an optional `presentation_method` and use the method's slide roles as the built-in (non-LLM) template, and augment `Slidict::Llm::Client` prompts with method description, required slide roles, and AI instructions so the model generates per-method outlines.
- Document architecture, YAML schema, plugin distribution, localization guidance, and contribution workflow in `README.md`, and bump the gem version to `0.5.0` with a changelog entry.
### Testing
- Ran `ruby -Ilib -e 'require "slidict"; r=Slidict::PresentationMethodRegistry.new(include_plugins: false); puts r.all.map(&:id).join(","); puts Slidict::VERSION'` which printed the built-in method IDs and `0.5.0` (success).
- Exercised CLI helpers: `ruby -Ilib bin/slidict list-methods`, `ruby -Ilib bin/slidict show-method scqa`, and `ruby -Ilib bin/slidict new --method prep --topic X --duration 1 --audience A --goal G --output /tmp/t.md`, each producing the expected output and generated file (success).
- Performed Ruby syntax checks with `ruby -c` on new/changed files (`lib/slidict/presentation_method.rb`, `lib/slidict/deck.rb`, `lib/slidict/cli/app.rb`, `lib/slidict/llm/client.rb`) which returned `Syntax OK` (success).
- `bundle exec rspec` and `bundle exec rubocop` were not run because `bundle install` failed in this environment (Rubygems returned `403 Forbidden`), so full test suite and lint were not executed (not run).
### Motivation
- Provide a data-first way to encode presentation techniques so anyone can add or improve methods by adding a single YAML file.
- Keep the gem itself minimal (load/validate/mapping only) and enable contributors to publish methods as small data-only PRs or external gems.
- Make AI generation, local templates, and CLI discoverability work with the same method data to support future scale (100+ methods) and localization.
### Description
- Add `Slidict::PresentationMethod` and `Slidict::PresentationMethodRegistry` in `lib/slidict/presentation_method.rb` to load, validate, and map YAML method files to Ruby objects.
- Add three built-in method definitions under `data/slidict/methods/`: `scqa.yml`, `prep.yml`, and `pyramid.yml`, each including `id`, `name`, `category`, `description`, `suitable_for`, `slides` (with `title`, `role`, `instructions`), `ai_instructions`, `review_checklist`, optional `references`, and `locale`.
- Wire methods into the CLI and generation flow by adding `--method` support, a `list-methods` command, a `show-method <id>` command, and accepting `new` as a user-friendly alias for generating slides in `lib/slidict/cli/app.rb`.
- Make `Slidict::Deck` accept an optional `presentation_method` and use the method's slide roles as the built-in (non-LLM) template, and augment `Slidict::Llm::Client` prompts with method description, required slide roles, and AI instructions so the model generates per-method outlines.
- Document architecture, YAML schema, plugin distribution, localization guidance, and contribution workflow in `README.md`, and bump the gem version to `0.5.0` with a changelog entry.
### Testing
- Ran `ruby -Ilib -e 'require "slidict"; r=Slidict::PresentationMethodRegistry.new(include_plugins: false); puts r.all.map(&:id).join(","); puts Slidict::VERSION'` which printed the built-in method IDs and `0.5.0` (success).
- Exercised CLI helpers: `ruby -Ilib bin/slidict list-methods`, `ruby -Ilib bin/slidict show-method scqa`, and `ruby -Ilib bin/slidict new --method prep --topic X --duration 1 --audience A --goal G --output /tmp/t.md`, each producing the expected output and generated file (success).
- Performed Ruby syntax checks with `ruby -c` on new/changed files (`lib/slidict/presentation_method.rb`, `lib/slidict/deck.rb`, `lib/slidict/cli/app.rb`, `lib/slidict/llm/client.rb`) which returned `Syntax OK` (success).
- `bundle exec rspec` and `bundle exec rubocop` were not run because `bundle install` failed in this environment (Rubygems returned `403 Forbidden`), so full test suite and lint were not executed (not run).
Motivation
Description
Slidict::PresentationMethodandSlidict::PresentationMethodRegistryinlib/slidict/presentation_method.rbto load, validate, and map YAML method files to Ruby objects.data/slidict/methods/:scqa.yml,prep.yml, andpyramid.yml, each includingid,name,category,description,suitable_for,slides(withtitle,role,instructions),ai_instructions,review_checklist, optionalreferences, andlocale.--methodsupport, alist-methodscommand, ashow-method <id>command, and acceptingnewas a user-friendly alias for generating slides inlib/slidict/cli/app.rb.Slidict::Deckaccept an optionalpresentation_methodand use the method's slide roles as the built-in (non-LLM) template, and augmentSlidict::Llm::Clientprompts with method description, required slide roles, and AI instructions so the model generates per-method outlines.README.md, and bump the gem version to0.5.0with a changelog entry.Testing
ruby -Ilib -e 'require "slidict"; r=Slidict::PresentationMethodRegistry.new(include_plugins: false); puts r.all.map(&:id).join(","); puts Slidict::VERSION'which printed the built-in method IDs and0.5.0(success).ruby -Ilib bin/slidict list-methods,ruby -Ilib bin/slidict show-method scqa, andruby -Ilib bin/slidict new --method prep --topic X --duration 1 --audience A --goal G --output /tmp/t.md, each producing the expected output and generated file (success).ruby -con new/changed files (lib/slidict/presentation_method.rb,lib/slidict/deck.rb,lib/slidict/cli/app.rb,lib/slidict/llm/client.rb) which returnedSyntax OK(success).bundle exec rspecandbundle exec rubocopwere not run becausebundle installfailed in this environment (Rubygems returned403 Forbidden), so full test suite and lint were not executed (not run).Codex Task
Summary by CodeRabbit