Skip to content

fix: read duration units only at a word boundary in the math animator - #2160

Open
kevin9327 wants to merge 1 commit into
HKUDS:devfrom
kevin9327:fix/duration-unit-word-boundary
Open

kevin9327 wants to merge 1 commit into
HKUDS:devfrom
kevin9327:fix/duration-unit-word-boundary

Conversation

@kevin9327

Copy link
Copy Markdown

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:

request before after
Explain the 3 main ideas of calculus 180.0 (3 main) None
Animate 4 matrices being multiplied 240.0 None
A 2 meter rod rotating 120.0 None
Show 3 squares on a grid 3.0 (3 squares) None
Draw a polygon with 5 sides 5.0 None
Explain the 3 main ideas in 30 seconds 180.0 30.0

Because 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, so s, sec, seconds, m, min, minutes must end the word. This also covers the earlier m(?![sS]) guard, so 1ms / 500ms still do not parse as minutes. The Chinese units (秒, 秒钟, 分钟) are unchanged.

Related Issues

  • None filed; found while reading the Math Animator pipeline.

Module(s) Affected

  • agents
  • tests

Checklist

  • This PR targets dev (not main — see the base-branch selector above).
  • I have read and followed the contribution guidelines.
  • My code follows the project's coding standards.
  • I have run pre-commit run --all-files and fixed any issues. (ran ruff check and ruff format --check on the changed files instead; both clean)
  • I have added relevant tests for my changes.
  • I have updated the documentation (if necessary). (not needed)
  • My changes do not introduce any new security vulnerabilities.

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 existing test_duration_ms_not_minute.py still passes, and the new test also checks 1.5m, 10s., 30秒 and 2分钟.

AI-assisted: I used an AI coding assistant for this change, then reviewed it and ran the tests myself.

This branch has not been deployed

No deployments
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.

1 participant