Skip to content

Replace AI::Response with Hash - #13

Merged
jelaniwoods merged 3 commits into
mainfrom
10-jw-replace-response-with-hash
Aug 15, 2025
Merged

Replace AI::Response with Hash#13
jelaniwoods merged 3 commits into
mainfrom
10-jw-replace-response-with-hash

Conversation

@jelaniwoods

@jelaniwoods jelaniwoods commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

Resolves #10


Important

Replaces AI::Response class with a Hash for response details, updating chat.rb, examples/02_core.rb, and README.md accordingly.

  • Behavior:
    • Replaces AI::Response class with a Hash for storing response details in generate!() in chat.rb.
    • Updates response access in examples/02_core.rb to use Hash keys instead of object attributes.
  • Documentation:
    • Updates README.md to reflect changes in response handling, using Hash syntax for accessing response details.
  • Removals:
    • Deletes response.rb, removing the AI::Response class entirely.

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

@ellipsis-dev ellipsis-dev Bot 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.

Important

Looks good to me! 👍

Reviewed everything up to 73a67a2 in 2 minutes and 5 seconds. Click for details.
  • Reviewed 138 lines of code in 4 files
  • Skipped 1 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:106
  • Draft comment:
    The 'chat_response' hash in generate! is missing a :total_tokens key; add it (e.g., total_tokens: response.usage.to_h[:total_tokens]) to match the tests.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. lib/ai/chat.rb:214
  • Draft comment:
    In prepare_messages_for_api, message[:response] is now a Hash, so replace '.id' with hash access (e.g., message[:response][:id]) to correctly compare with previous_response_id.
  • 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. README.md:585
  • Draft comment:
    There is a potential inconsistency: the README usage example shows keys :prompt_tokens/:completion_tokens, while generate! slices :input_tokens/:output_tokens. Verify and adjust the key names so they are consistent.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_PEB4cugVCPOYdxuL

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

@ellipsis-dev ellipsis-dev Bot 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.

Caution

Changes requested ❌

Reviewed 3077189 in 1 minute and 22 seconds. Click for details.
  • Reviewed 19 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:105
  • Draft comment:
    Extracting response_usage improves clarity. Confirm that response.usage always responds to to_h.
  • 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 first part about clarity is just praise, not actionable. The second part asks for confirmation about API behavior, which violates our rule about not asking authors to verify things. If response.usage didn't respond to to_h, it would fail at runtime and be caught by tests. We should trust that the author tested their code. The comment does point out a potential runtime error that could occur if the API changes. Maybe this is a valid concern about defensive programming? No - we should assume the author tested their code and that the OpenAI API is stable. Asking for confirmation adds no value and just creates unnecessary back-and-forth. Delete the comment. It's partly praise (not actionable) and partly asking for confirmation (which violates our rules).

Workflow ID: wflow_vFTqlRfZhAnK1BGx

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

Comment thread lib/ai/chat.rb
@jelaniwoods
jelaniwoods merged commit 876c7f7 into main Aug 15, 2025
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.

Should we replace Response with just a Hash?

1 participant