Skip to content

Conversation

@raghubetina
Copy link
Contributor

@raghubetina raghubetina commented Dec 5, 2025

The ergonomics of demo'ing the gem could be better. This patch attempts to make IRB/console output as beginner-friendly/demo-friendly as possible, as well as adding a to_html method to make visualizing in ERB better.

Summary

This PR improves how AI::Chat, messages, and conversation items display in IRB, Rails console, and ERB templates.

Key Changes

  • AI::Message class: Messages are now AI::Message instances (a Hash subclass) with custom inspect and to_html methods for readable output
  • AI::Items class: New wrapper using SimpleDelegator that provides nice display while delegating all methods to the underlying API response
  • Renamed itemsget_items: Clearer naming to indicate the method makes an API call
  • Reasoning summaries: When reasoning_effort is set, the API now returns chain-of-thought summaries showing the model's reasoning process
  • to_html methods: All display objects support rendering in ERB with a dark terminal-style background
  • TTY-aware: Colors are automatically disabled when output is piped or redirected

Breaking Changes

  • chat.items is now chat.get_items
  • Removes chat.verbose; instead, chat.get_items formats itself more nicely.
  • Returns an AI::Items wrapper instead of raw OpenStruct (all methods still work via delegation)

Test Plan

  • Run bundle exec rspec spec/unit/ - 37 unit tests for display functionality
  • Test in IRB: chat = AI::Chat.new; chat.user("hi"); chat.generate!; chat
  • Test in Rails console with the same commands
  • Test chat.get_items with reasoning_effort = "high" to see reasoning summaries
  • Test chat.to_html and chat.get_items.to_html in an ERB template
  • Run bundle exec ruby examples/16_get_items.rb to see the full demo

Example ERB:

<h1>Pages#home</h1>
<p>Find me in app/views/pages/home.html.erb</p>

<%
  chat = AI::Chat.new
  chat.background = true
  chat.web_search = true
  chat.reasoning_effort = "high"
%>
<hr>
<%= chat.user("What is the DJIA today?").to_html %>
<hr>
<%= chat.generate!.to_html %>
<% chat.get_response(wait: true)%>
<hr>
<%= chat %>
<hr>
<%= chat.to_html %>
<hr>
<%= chat.get_items %>
<hr>
<%= chat.get_items.to_html %>
<hr>
<% chat.get_items.data.each do |item| %>
  <% case item.type.to_s %>
  <% when "message" %>
    <div class="message <%= item.role %>">
      <strong><%= item.role.capitalize %>:</strong>
      <% if item.content&.first %>
        <% content = item.content.first %>
        <% if content.type.to_s == "input_text" %>
          <%= content.text %>
        <% elsif content.type.to_s == "output_text" %>
          <%= content.text %>
        <% end %>
      <% end %>
    </div>
  <% when "reasoning" %>
    <% if item.summary&.first %>
      <details class="reasoning">
        <summary>Reasoning</summary>
        <%= item.summary.first.text %>
      </details>
    <% end %>
  <% when "web_search_call" %>
    <% if item.action.respond_to?(:query) && item.action.query %>
      <div class="web-search">
        Searched: "<%= item.action.query %>"
      </div>
    <% end %>
  <% when "image_generation_call" %>
    <div class="image-generation">
      Image generated
    </div>
  <% end %>
<% end %>

Important

Enhances AI chat message and item inspection in IRB, Rails console, and ERB, introducing AI::Message and AI::Items classes with improved display methods.

  • Behavior:
    • AI::Message class now represents messages with custom inspect and to_html methods for better readability.
    • AI::Items class wraps conversation items with display methods, delegating other methods to the API response.
    • chat.items renamed to chat.get_items to indicate API call, returning AI::Items wrapper.
    • Reasoning summaries added when reasoning_effort is set, showing model's reasoning process.
    • to_html methods added for rendering in ERB with terminal-style background.
    • Console output is TTY-aware, disabling colors when output is piped or redirected.
  • Breaking Changes:
    • chat.items is now chat.get_items.
    • Removes chat.verbose; chat.get_items now provides formatted output.
    • Returns AI::Items instead of raw OpenStruct, with method delegation.
  • Misc:
    • Added examples/16_get_items.rb to demonstrate conversation item inspection.
    • Updated CHANGELOG.md and README.md to reflect changes.
    • Version bump to 0.5.0 in ai-chat.gemspec.

This description was created by Ellipsis for 3c4a734. You can customize this summary. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 117c9b6 in 2 minutes and 13 seconds. Click for details.
  • Reviewed 208 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai-chat.rb:1
  • Draft comment:
    Good: The addition of require_relative "ai/message" ensures that the Message class is loaded before it’s used in Chat. Verify the load order and dependency consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. lib/ai/amazing_print.rb:31
  • Draft comment:
    In format_ai_chat, the code now maps over chat.inspectable_attributes and removes the previous truncation logic for message contents. If long messages might overwhelm the output, consider adding notes or reintroducing optional truncation.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is speculative - it says "If long messages might overwhelm the output, consider..." which is a conditional suggestion. The rules explicitly state "Do NOT make speculative comments, such as 'If X, then Y is an issue'. Only comment if it is definitely an issue." The comment doesn't identify a definite bug or problem, it's just raising a potential concern about what "might" happen. Additionally, the truncation logic was intentionally removed in this refactor, and the new implementation delegates to inspectable_attributes, which suggests the Chat class itself now handles what should be inspectable. Without evidence that this is causing a real problem, this is just a speculative concern. Perhaps the removal of truncation is actually a bug and will cause real issues with output being overwhelmed. The original code had explicit truncation for a reason, and removing it without replacement could be problematic. Maybe this is a legitimate concern that should be addressed. While truncation was removed, the refactor delegates to chat.inspectable_attributes, which suggests the Chat class now controls what gets displayed. This is likely intentional design - moving the responsibility to the Chat class itself. Without evidence of an actual problem (like a failing test or known issue), this remains speculative. This comment is speculative and conditional ("If long messages might...consider"), which violates the rule against speculative comments. The truncation was intentionally removed as part of a refactor that delegates to inspectable_attributes. Without evidence of a definite problem, this comment should be deleted.
3. lib/ai/chat.rb:87
  • Draft comment:
    Refactored message creation now uses Message[...] instead of a hash literal. This is great for consistent custom display, but please verify that Message[] correctly returns an instance of Message (and not a plain Hash) to preserve your custom inspect behavior.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify the behavior of a function, which violates the rule against asking for confirmation or verification of behavior. It does not provide a specific suggestion or point out a clear issue.
4. lib/ai/chat.rb:233
  • Draft comment:
    The new inspectable_attributes method neatly orders and filters attributes for display. Consider adding inline documentation to clarify the rationale behind which attributes are always shown, conditionally shown, or omitted.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. lib/ai/chat.rb:265
  • Draft comment:
    Using AmazingPrint::Inspector in both inspect and to_html is a neat enhancement. Consider if any fallback might be useful when AmazingPrint isn’t available, though the library already attempts to conditionally load it.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
6. lib/ai/message.rb:4
  • Draft comment:
    Defining Message as a subclass of Hash with custom inspect/to_html is a solid approach. Ensure that using Message[...] (inherited from Hash[]) always returns an instance of Message so that the custom inspect behavior is preserved.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure a specific behavior is maintained, which violates the rule against asking the author to ensure behavior. It doesn't provide a specific suggestion or point out a specific issue with the code.

Workflow ID: wflow_66CIwSt0IjlCPhYa

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed d546688 in 44 seconds. Click for details.
  • Reviewed 31 lines of code in 2 files
  • Skipped 1 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. ai-chat.gemspec:5
  • Draft comment:
    Version bump to 0.4.1 looks good. Ensure that the changelog reflects this update.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. lib/ai/chat.rb:266
  • Draft comment:
    Refactoring inspect/to_html to use the ai helper is concise. Confirm that the ai method from AmazingPrint is available in all contexts (e.g., in non-TTY or HTML environments) to avoid potential runtime errors.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_cprLOz6uDXz54VuV

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 10c7844 in 1 minute and 40 seconds. Click for details.
  • Reviewed 28 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/chat.rb:270
  • Draft comment:
    Removing the custom 'pretty_print' method may break compatibility with libraries using Ruby’s PrettyPrint protocol (e.g., when calling pp chat). Consider retaining a minimal pretty_print(q) that delegates to pretty_inspect (or wraps inspect) to ensure consistent output in both IRB and test environments.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a speculative comment about potential breakage. The comment says "may break compatibility" and uses conditional language. According to the rules, I should NOT make speculative comments like "If X, then Y is an issue" - I should only comment if it's definitely an issue. The comment doesn't provide evidence that this will actually break anything, just that it "may" break. Additionally, the author clearly made this change intentionally, removing the complex pretty_print implementation in favor of a simpler pretty_inspect. If this were going to cause actual problems, it would likely be caught in testing. The comment is asking the author to "consider retaining" something they deliberately removed, which is second-guessing their decision without strong evidence. However, this could be a legitimate concern about Ruby's standard library behavior. The pp method does rely on the PrettyPrint protocol, and removing pretty_print could change behavior. But the author may have intentionally wanted to simplify this, and pretty_inspect is also part of Ruby's pretty printing infrastructure. Without evidence that this actually breaks something, this is still speculative. While it's true that pp uses the PrettyPrint protocol, the comment is still speculative ("may break") and doesn't demonstrate actual breakage. The author clearly intended to simplify this code by removing the complex pretty_print implementation. If this causes issues, they would be caught in testing. The comment is essentially asking the author to reconsider their deliberate change without strong evidence of a problem. This comment should be deleted. It's speculative ("may break compatibility"), doesn't provide evidence of actual breakage, and questions a deliberate design decision by the author. According to the rules, speculative comments should not be kept unless there's strong evidence of a definite issue.

Workflow ID: wflow_DoBL5dkuLAj6oIwt

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed a7cd738 in 47 seconds. Click for details.
  • Reviewed 15 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/message.rb:9
  • Draft comment:
    The new pretty_inspect method simply appends a newline to the inspect result. Consider checking if inspect might already include a trailing newline (to avoid duplicate newlines) and adding a brief comment clarifying its purpose for future maintainers.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_R9puWQFKFzL58GkH

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@raghubetina raghubetina changed the title Improve interactive display for AI::Chat Improve inspect output for IRB, Rails console, and ERB Dec 5, 2025
@raghubetina raghubetina force-pushed the rb-improve-inspect branch 2 times, most recently from f85ee53 to b18b3e4 Compare December 5, 2025 20:15
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed f85ee53 in 2 minutes and 13 seconds. Click for details.
  • Reviewed 784 lines of code in 8 files
  • Skipped 2 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/chat.rb:246
  • Draft comment:
    The inspectable_attributes method uses msg.except(:response), but Hash#except is provided by ActiveSupport. Consider requiring 'active_support/core_ext/hash/except' or providing a fallback to ensure compatibility in non-Rails environments.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% Looking at the full file context, I don't see a definition of the Message class within this file. The code uses Message[...] syntax which could be a Hash literal or a custom class constructor. The comment assumes these are plain Hashes and that except comes from ActiveSupport. However, I cannot see the definition of Message in this file. The diff shows that the code was refactored to use Message[...] instead of plain hash literals {...}. This is a significant change that suggests Message might be a custom class defined elsewhere. Without seeing the Message class definition, I cannot definitively say whether except is available or not. The comment might be incorrect if Message is a custom class that implements except. I'm assuming that Message is just a Hash, but the refactoring in the diff from plain hash literals to Message[...] suggests it might be a custom class. If Message is defined elsewhere in the codebase and implements except, then this comment would be incorrect. While I cannot see the Message class definition in this file, the fact that the code was specifically refactored to use Message[...] instead of plain hashes suggests intentional design. If the author introduced this pattern and is using except, they likely ensured Message supports it. Without cross-file context showing Message is just a Hash alias, I should assume the author knows what they're doing. The comment makes an assumption about Message being a plain Hash without evidence. The refactoring to use Message[...] suggests it's likely a custom class that may implement except. Without seeing the Message definition or evidence that this is actually a problem, this comment should be deleted.
2. lib/ai/chat.rb:86
  • Draft comment:
    The add method has two branches for constructing messages (simple vs multimodal) with similar key assignments. Consider refactoring or extracting a helper to reduce duplication and improve clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. ai-chat.gemspec:24
  • Draft comment:
    Base64 is a Ruby standard library. Consider removing it as a runtime dependency to avoid unnecessary dependency declarations.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_f2Okl0gThWUCO61p

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed b18b3e4 in 1 minute and 43 seconds. Click for details.
  • Reviewed 785 lines of code in 8 files
  • Skipped 2 files when reviewing.
  • Skipped posting 7 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. ai-chat.gemspec:24
  • Draft comment:
    Consider removing the runtime dependency on 'base64' and 'ostruct' since these are part of Ruby's standard library. If intentional, please document the rationale.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. lib/ai/chat.rb:86
  • Draft comment:
    The updated #add method now returns an AI::Message instance instead of a plain Hash, which improves consistency with generate! and the other helper methods. Good job!
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining a change in the code without suggesting any action or asking for clarification. It doesn't align with the rules for useful comments.
3. lib/ai/chat.rb:234
  • Draft comment:
    The 'inspectable_attributes' method is well structured, selectively including attributes and ordering them logically. Consider adding inline comments on the ordering rationale for future maintainers.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
4. lib/ai/chat.rb:268
  • Draft comment:
    The addition of the 'to_html' method for Chat is a useful improvement for ERB rendering. It appropriately marks the output as html_safe if possible.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it praises the addition of a method without providing any actionable feedback or suggestions for improvement. It does not align with the rules for useful comments.
5. lib/ai/message.rb:5
  • Draft comment:
    The custom inspect, pretty_inspect, pretty_print, and to_html methods in Message enhance output consistency across IRB, Rails console, and ERB. Looks clear and effective.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that the methods enhance output consistency, which does not align with the rules for useful comments.
6. spec/unit/chat_spec.rb:8
  • Draft comment:
    The specs for AI::Chat are comprehensive and effectively cover the new behaviors (e.g., returning AI::Message, inspectable attributes, and HTML output).
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it praises the comprehensiveness of the specs without providing any actionable feedback or suggestions for improvement. It does not align with the rules for useful comments.
7. spec/unit/message_spec.rb:5
  • Draft comment:
    The tests for AI::Message confirm that it behaves as a Hash and that the custom formatting methods work correctly. This is thorough and helps ensure reliability across output formats.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, praising the thoroughness of the tests. It does not provide any actionable feedback or suggestions for improvement.

Workflow ID: wflow_NHyC49NynaxWUfDS

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed c437ec1 in 2 minutes and 6 seconds. Click for details.
  • Reviewed 1282 lines of code in 13 files
  • Skipped 2 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. examples/13_conversation_features_comprehensive.rb:117
  • Draft comment:
    The example calls 'chat.display_items' but no such method is defined in AI::Chat. Consider adding an alias or implementing this method (e.g., aliasing it to 'inspect' or 'pretty_inspect').
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is making an assumption that display_items doesn't exist in the AI::Chat class. However, I can't see the actual class definition in this diff - only the example file. The PR author deliberately changed from verbose to display_items, which strongly suggests they also updated the actual class implementation. This would be a cross-file issue that I can't verify from this diff alone. According to the rules, I should "Ignore cross-file issues. Only think about the file you are reviewing." Also, if this method truly didn't exist, it would be caught when running the example, so this falls under "Do NOT comment on anything that would be obviously caught by the build." Maybe the method really doesn't exist and the PR author forgot to add it? The comment could be catching a legitimate bug where the example was updated but the implementation wasn't. Even if that's true, this would be caught immediately when running the example (a NoMethodError would be raised). The rules explicitly state not to comment on things that would be obviously caught by the build. Additionally, this is a cross-file issue - I'd need to see the AI::Chat class definition to verify, which violates the rule about ignoring cross-file issues. This comment should be deleted. It's speculating about whether a method exists in a different file (cross-file issue), and if the method truly doesn't exist, it would be immediately caught when running the example. There's no strong evidence from this diff alone that the comment is correct.
2. lib/ai/chat.rb:200
  • Draft comment:
    In the get_items proxy branch, the transformation block uses 'hash.key(value) == :type' which doesn’t reliably detect when a key is :type. Since transform_values! only receives the value (not the key), consider iterating over key-value pairs (e.g., using each_with_object) to properly convert values for key :type.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_DK9GsIOVKFSc26w9

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 8e9b26a in 2 minutes and 5 seconds. Click for details.
  • Reviewed 1295 lines of code in 14 files
  • Skipped 2 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/chat.rb:202
  • Draft comment:
    The transformation inside the proxy branch of get_items is problematic. Using hash.transform_values! do |value| if hash.key(value) == :type ... does not correctly check if the key is :type. Consider iterating over key/value pairs (e.g. using each_pair or transform_keys) to properly convert values for the :type key.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. examples/13_conversation_features_comprehensive.rb:53
  • Draft comment:
    The example still uses chat.items instead of the renamed chat.get_items, which conflicts with the breaking change. Update these calls for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. lib/ai/chat.rb:233
  • Draft comment:
    The use of msg.except(:response) in building inspectable attributes assumes availability of ActiveSupport's Hash#except. If running in a plain Ruby environment this may cause issues. Consider providing a fallback implementation.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% Hash#except was added to Ruby core in version 3.0 (released December 2020). This means it's been part of standard Ruby for over 4 years now. The comment assumes this is an ActiveSupport-only method, which is outdated information. Unless this codebase needs to support Ruby versions older than 3.0 (which would be quite unusual in 2024), the comment is incorrect. The suggestion to use reject instead is unnecessary and would make the code less readable. The comment is about changed code (the inspectable_attributes method is new in the diff), so it does relate to the changes. I don't know what Ruby version this project targets. If it needs to support Ruby 2.x for some reason, the comment would be valid. Also, I cannot see if there's a Gemfile or version specification that would clarify this. While I can't see the Ruby version requirement, it's extremely unlikely that a modern project in 2024 would target Ruby versions older than 3.0. Ruby 2.7 reached end-of-life in March 2023. The comment makes an incorrect assumption that Hash#except requires ActiveSupport, when it's been part of Ruby core for years. Without strong evidence that this project needs to support ancient Ruby versions, this comment should be deleted. The comment is based on outdated information. Hash#except has been part of Ruby core since version 3.0 (December 2020) and doesn't require ActiveSupport. Unless there's clear evidence this project supports Ruby 2.x (which would be unusual), this comment is incorrect and should be deleted.

Workflow ID: wflow_Ox2IvNoB7N8dbyLQ

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 0161d91 in 2 minutes and 30 seconds. Click for details.
  • Reviewed 1358 lines of code in 15 files
  • Skipped 2 files when reviewing.
  • Skipped posting 6 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/chat.rb:191
  • Draft comment:
    In the proxy branch of get_items, the mapping over response_hash[:data] is executed if the key exists, but then create_deep_struct(response_hash) is always the final returned value. Please clarify if this is intentional or if the transformed data should be returned instead.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. lib/ai/chat.rb:232
  • Draft comment:
    The use of Hash#except in inspectable_attributes may fail in pure Ruby since it's an ActiveSupport method. Consider adding a dependency or providing a fallback implementation.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Hash#except was added to Ruby core in Ruby 3.0, so it's been available in standard Ruby for several years now. The comment is factually incorrect if the project targets Ruby 3.0+. Even if the project targets older Ruby versions, this would be caught immediately by tests or runtime errors, making it an obvious issue that doesn't need a review comment. Additionally, I cannot see the Message class definition to confirm these are even Hashes. The comment is speculative and likely incorrect. I cannot see what Ruby version this project targets, and I cannot see the definition of the Message class to confirm whether these objects are Hashes. Perhaps the project does target Ruby 2.x where except is not available, or perhaps Message objects don't respond to except at all. Even if the project targets Ruby 2.x or if Message objects don't respond to except, this would be immediately caught by any test suite or runtime execution. This is not a subtle bug that needs to be caught in code review. The comment also makes an incorrect claim that except is an ActiveSupport method, when it's been in Ruby core since 3.0. Without strong evidence that this is actually a problem (like seeing a Ruby version constraint in the diff), I should delete this comment. The comment makes a factually incorrect claim about Hash#except being ActiveSupport-only (it's been in Ruby core since 3.0), and even if there were a compatibility issue, it would be immediately caught by tests or runtime. There's no strong evidence this is a real problem, and the rules say not to make speculative comments.
3. lib/ai/chat.rb:636
  • Draft comment:
    The API key validation in validate_api_key relies on the prefix 'sk-proj'. This check might be brittle if the official key format changes. Consider a more robust solution or updating documentation.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50% None
4. lib/ai/chat.rb:397
  • Draft comment:
    In prepare_messages_for_api, if last_response_id is not found, the entire messages array is returned. Verify that this behavior correctly handles edge cases and does not mistakenly resend previous messages.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50% None
5. lib/ai/chat.rb:170
  • Draft comment:
    When setting schema via a String, parsing errors (e.g. JSON::ParserError) might occur. Consider adding explicit error handling to provide a clearer message to users.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
6. spec/unit/chat_spec.rb:111
  • Draft comment:
    The tests for inspectable_attributes assume a specific order of attributes (e.g. @conversation_id followed by @last_response_id). Ensure this order is intentional and documented, as future refactoring might change it.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50% None

Workflow ID: wflow_NWsOXca4E2lR6yKt

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 7381ef3 in 39 seconds. Click for details.
  • Reviewed 11 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai-chat.rb:2
  • Draft comment:
    The new HTML_PRE_STYLE update improves text wrapping for demo outputs. Consider using 'overflow-wrap: break-word' in addition to or instead of 'word-wrap', as the latter is deprecated in modern CSS. Also, verify that removing 'overflow-x: auto' doesn't negatively impact the display of long lines.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 50% <= threshold 50% The comment suggests a specific code improvement by recommending the use of overflow-wrap: break-word instead of word-wrap, which is deprecated. This is a valid suggestion as it aligns with modern CSS practices. However, the second part of the comment asks the author to verify the impact of removing overflow-x: auto, which violates the rule against asking the author to verify or ensure behavior. The first part of the comment is useful, but the second part should be removed.

Workflow ID: wflow_IGqgy1wMmCxBgcRC

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 23d7e4f in 1 minute and 17 seconds. Click for details.
  • Reviewed 1361 lines of code in 15 files
  • Skipped 2 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/chat.rb:202
  • Draft comment:
    Bug: In the transformation block for proxy responses in get_items, the code uses hash.key(value) == :type which is incorrect. It seems intended to check if the current key is :type. Consider iterating with each_pair and checking the key directly (e.g. |k, v| k == :type ? v.to_sym : v).
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. lib/ai/chat.rb:636
  • Draft comment:
    Note: The API key validation in validate_api_key relies on checking if the key starts with 'sk-proj'. This string prefix check may be brittle. Consider a more robust or configurable method for distinguishing official from unofficial API keys.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. lib/ai/chat.rb:194
  • Draft comment:
    Clarification: In the proxy branch of get_items, after mutating the response_hash, the code unconditionally calls create_deep_struct(response_hash) as the return value. Ensure that the intended transformed values are preserved and that create_deep_struct handles the mutated hash appropriately.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 20% <= threshold 50% The comment is asking the author to ensure that the intended transformed values are preserved and that a function handles the mutated hash appropriately. This falls under asking the author to ensure behavior, which is not allowed. The comment does not provide a specific suggestion or point out a clear issue, making it more of a request for confirmation rather than a constructive suggestion.
4. lib/ai/amazing_print.rb:6
  • Draft comment:
    Reminder: The monkey patch in AmazingPrint (lines 7-16) is intended as a temporary fix. Be sure to remove it once the upstream PR is merged.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_RCq3vZGgnD7ola1c

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

- Add AI::Message class (Hash subclass) with custom display
- Add AI::Items class (SimpleDelegator) wrapping API response
- Rename items to get_items to indicate API call
- Enable reasoning summaries when reasoning_effort is set
- Add to_html methods with dark terminal-style background
- TTY-aware colorization (disabled when piped/redirected)
- Add examples/16_get_items.rb demonstrating item inspection
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 3c4a734 in 1 minute and 1 seconds. Click for details.
  • Reviewed 1352 lines of code in 15 files
  • Skipped 2 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/ai/chat.rb:202
  • Draft comment:
    The transformation logic in get_items (proxy branch) uses 'hash.key(value) == :type', which does not reliably check for the :type key. This approach may lead to unintended behavior. Consider iterating over each key-value pair (for example, using each_pair or transforming only when the key equals :type) to ensure that the :type field is correctly converted to a symbol.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. lib/ai/amazing_print.rb:7
  • Draft comment:
    The custom formatting relies on AmazingPrint. Consider documenting this dependency or providing a fallback behavior if AmazingPrint is unavailable, so users are aware that inspect output improvements depend on it.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_zcpifRtFNRR513Um

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@bpurinton bpurinton left a comment

Choose a reason for hiding this comment

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

I tested everything here and also created a small Rails app to test the views there. Everything looks good with the formatting.

when :reasoning
puts "Reasoning: #{item.summary.first.text}"
# Reasoning summaries show the model's chain-of-thought
if item.summary&.first
Copy link
Contributor

@jelaniwoods jelaniwoods Dec 8, 2025

Choose a reason for hiding this comment

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

This is maybe the most advanced syntax in the README that I think appdev 1 students wouldn't understand. Do we want to make an effort to refactor this to be more appdev 1 approachable?

maybe something like

if item.summary.length > 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. A lot of stuff has accrued in the README and it's kind of a mess right now. I'll make this change but overall I think I want to make a thorough reference lesson in First Draft with runnable code blocks, and link to that from the README.

Choose a reason for hiding this comment

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

That's a good point. If we do that, we should also probably avoid the case / when :symbol syntax and write the README with e.g.

if item.type == "message"
  # ... etc.

module AI
class Message < Hash
def inspect
ai(plain: !$stdout.tty?, index: false)
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that AI and ai are unrelated confused me for a bit! 😄 I forgot it's amazing print's method.

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.

4 participants