Skip to content

fix: strip whitespaces in query/response#26

Merged
Ki-Seki merged 1 commit intomainfrom
Ki-Seki/issue18
Sep 21, 2025
Merged

fix: strip whitespaces in query/response#26
Ki-Seki merged 1 commit intomainfrom
Ki-Seki/issue18

Conversation

@Ki-Seki
Copy link
Member

@Ki-Seki Ki-Seki commented Sep 21, 2025

Fixes #18

@Ki-Seki Ki-Seki requested a review from Copilot September 21, 2025 03:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issue #18 by adding whitespace stripping functionality to query/response parsing. The implementation strips whitespace from input strings before validation, allowing for more flexible input handling.

  • Added whitespace stripping to the parse_inp_or_outp function using s.strip()
  • Reorganized test cases by splitting valid and invalid cases into separate test functions
  • Added test coverage for input strings with surrounding whitespace

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/gimkit/schemas.py Added s.strip() to remove whitespace before parsing
tests/test_parsing.py Reorganized tests and added whitespace handling test case

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +33 to +34
m_input = '\n<|M_INPUT|>This is an <|MASKED id="m_0"|><|/MASKED|> text.<|/M_INPUT|>\n\n \t'
m_output = ' \n<|M_OUTPUT|>\n<|MASKED id="m_0"|>example<|/MASKED|><|/M_OUTPUT|>\n'
Copy link

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

[nitpick] The whitespace test case uses mixed whitespace characters (spaces, newlines, tabs) in a way that's hard to read and verify. Consider using more explicit whitespace or adding comments to clarify what specific whitespace scenarios are being tested.

Suggested change
m_input = '\n<|M_INPUT|>This is an <|MASKED id="m_0"|><|/MASKED|> text.<|/M_INPUT|>\n\n \t'
m_output = ' \n<|M_OUTPUT|>\n<|MASKED id="m_0"|>example<|/MASKED|><|/M_OUTPUT|>\n'
# m_input: leading newline, trailing two newlines, space, and tab
m_input = (
"\n" # leading newline
"<|M_INPUT|>This is an <|MASKED id=\"m_0\"|><|/MASKED|> text.<|/M_INPUT|>"
"\n\n \t" # two newlines, space, tab at end
)
# m_output: leading space and newline, newline after prefix, trailing newline
m_output = (
" \n" # leading space and newline
"<|M_OUTPUT|>\n"
"<|MASKED id=\"m_0\"|>example<|/MASKED|><|/M_OUTPUT|>"
"\n" # trailing newline
)

Copilot uses AI. Check for mistakes.
@Ki-Seki Ki-Seki merged commit c5e4e68 into main Sep 21, 2025
1 check passed
@Ki-Seki Ki-Seki deleted the Ki-Seki/issue18 branch September 21, 2025 03:08
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.

fix: strip whitespaces in query/response

2 participants