Repository navigation
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
parse_target_duration_seconds(Math Animator) reads the target video length from the user's request and style hint. Its unit patterns had no word boundary after the English units, so the first letter of an ordinary word after a number was read as a unit:Explain the 3 main ideas of calculus3 main)Animate 4 matrices being multipliedA 2 meter rod rotatingShow 3 squares on a grid3 squares)Draw a polygon with 5 sidesExplain the 3 main ideas in 30 secondsBecause the function returns the largest candidate, a stray match can also override a duration the user did state (last row). The value is passed to the code generator and the renderer as
duration_target_seconds.The fix adds
(?![a-z])(case-insensitive) after the English units, sos,sec,seconds,m,min,minutesmust end the word. This also covers the earlierm(?![sS])guard, so1ms/500msstill do not parse as minutes. The Chinese units (秒,秒钟,分钟) are unchanged.Related Issues
Module(s) Affected
agentstestsChecklist
dev(notmain— see the base-branch selector above).pre-commit run --all-filesand fixed any issues. (ranruff checkandruff format --checkon the changed files instead; both clean)Additional Notes
New
tests/agents/math_animator/test_duration_unit_word_boundary.py: the five "number before a word" cases and the mixed sentence fail before the change and pass after. The existingtest_duration_ms_not_minute.pystill passes, and the new test also checks1.5m,10s.,30秒and2分钟.AI-assisted: I used an AI coding assistant for this change, then reviewed it and ran the tests myself.