Skip to content

FIX: sample within word punctuation insertion from every position - #3566

Open
Manohar Paturi (ManoharPaturi) wants to merge 4 commits into
microsoft:mainfrom
ManoharPaturi:fix/insert-punctuation-positions
Open

Manohar Paturi (ManoharPaturi) wants to merge 4 commits into
microsoft:mainfrom
ManoharPaturi:fix/insert-punctuation-positions

Conversation

@ManoharPaturi

Copy link
Copy Markdown
Contributor

Description

Fixes the linked issue

The insertion index sample drew from range(len minus 1), so punctuation could never land immediately before the last character or after the final character of the prompt. The sample now draws from range(len + 1), the full set of insertion points of an n character list, and the too few characters fallback inserts at the end of the prompt.

Tests and Documentation

Added a reachability regression test that converts the prompt abc across 200 seeds and asserts all four insertion offsets appear. It fails on main, where only offsets 0 and 1 ever appear, and passes with the change. The full converter suite passes (24 tests).

No documentation change needed, so I did not run JupyText.

@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

Neither of the red checks touches this change's code path: the PR modifies one Python converter, and the failures were the frontend unit coverage job and the shared coverage job. This push merges current main, which also brings the branch back up to date, so both re-run. The converter suites still pass locally (24 and 249 tests).

Remove the unreachable insertion fallback and exercise insertion boundaries deterministically. Wait for modal accessibility restoration before reopening the run preview.

Co-authored-by: Copilot App <[email protected]>
@romanlutz

Copy link
Copy Markdown
Contributor

Yes, don't worry Manohar Paturi (@ManoharPaturi) I will take care of it.

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.

2 participants