Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve CODE_BLOCK_PATTERN for a more robust code match #571

Merged
merged 6 commits into from
Nov 21, 2023
Merged

improve CODE_BLOCK_PATTERN for a more robust code match #571

merged 6 commits into from
Nov 21, 2023

Conversation

DearVa
Copy link
Collaborator

@DearVa DearVa commented Nov 6, 2023

Why are these changes needed?

In the output of agent_human_feedback.ipynb, I notice that agent's code was not extracted correctly at line 69.

exitcode: 1 (execution failed)
Code output: 
  File "", line 2
    Step 2: Define the equations
         ^
SyntaxError: invalid syntax

That's because there is a space character before python.

Which means ``` python cannot be parsed.

What's more, redundant spaces after language name also cannot be parsed, ```python for example.

Related issue number

Checks

@afourney
Copy link
Member

afourney commented Nov 7, 2023

Thank you for documenting your regular expression.

There are a few relevant issues worth linking. Notably, #414, and #399

I believe there are also cases where \r\n is output by the model and needs to be accommodated.

@DearVa
Copy link
Collaborator Author

DearVa commented Nov 9, 2023

@microsoft-github-policy-service agree

@DearVa
Copy link
Collaborator Author

DearVa commented Nov 9, 2023

Thank you for documenting your regular expression.

There are a few relevant issues worth linking. Notably, #414, and #399

I believe there are also cases where \r\n is output by the model and needs to be accommodated.

Sorry about my late reply. I've added a \r? to my regex to handle with the \r\n case.
By the way, I'm not sure if check 1 was necessary, so I left it unchecked.

@afourney
Copy link
Member

Thanks. I will look into this ASAP.

Ideally we should be more flexible, and should adopt the code block definition specified in markdown here: https://spec.commonmark.org/0.30/#fenced-code-blocks

I'll look into this degree of compatibility tomorrow

@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (143e49c) 29.81% compared to head (6ea84a5) 47.50%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #571       +/-   ##
===========================================
+ Coverage   29.81%   47.50%   +17.69%     
===========================================
  Files          27       27               
  Lines        3448     3448               
  Branches      780      822       +42     
===========================================
+ Hits         1028     1638      +610     
+ Misses       2346     1649      -697     
- Partials       74      161       +87     
Flag Coverage Δ
unittests 47.27% <100.00%> (+17.51%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@afourney
Copy link
Member

@victordibia @sonichi Please have a look at this PR. It has been sitting for a while, and I think it's good to go.

@sonichi sonichi added this pull request to the merge queue Nov 21, 2023
Merged via the queue into microsoft:main with commit d22664f Nov 21, 2023
58 checks passed
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
* improve CODE_BLOCK_PATTERN for more robust match

* improve and add tests

* Add support for \r\n

* Updated the regex to support indented code blocks (per the Markdown spec). Added test cases for both.

* Update formatting

---------

Co-authored-by: Adam Fourney <[email protected]>
Co-authored-by: Chi Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-execution execute generated code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants