Skip to content

Fix to_gpt comma punctuation spacing#32

Open
akmhatey-ai wants to merge 2 commits into
zerocracy:masterfrom
akmhatey-ai:codex/fix-to-gpt-comma-26
Open

Fix to_gpt comma punctuation spacing#32
akmhatey-ai wants to merge 2 commits into
zerocracy:masterfrom
akmhatey-ai:codex/fix-to-gpt-comma-26

Conversation

@akmhatey-ai

@akmhatey-ai akmhatey-ai commented May 25, 2026

Copy link
Copy Markdown

Fixes #26.

Summary:

  • Replace the alternation with a character class so to_gpt removes spaces before bare commas as intended.
  • Add regression coverage for spaces before !, ,, :, and ?.

Reproduction:

  • Old regex: "hello , world".replace(/ (\?|!|\|,|:)/g, "$1") returns hello , world.
  • Fixed regex: "hello , world".replace(/ ([?!,:])/g, "$1") returns hello, world.
  • Current helper check: to_gpt("Wait ! Look , here : why ?") returns Wait! Look, here: why?.

Validation:

  • npm ci passed; npm reports existing dependency audit findings.
  • npx jest --config jest.config.ts --no-color --ci test/to_gpt.test.ts --coverage=false passed.
  • npx eslint src/to_gpt.ts test/to_gpt.test.ts --config eslint.config.mjs passed.
  • npx tsc --noEmit passed.
  • git diff --check passed.
  • git diff -- src/to_gpt.ts test/to_gpt.test.ts | gitleaks stdin --no-banner --redact reported no leaks.

Limitations:

  • Full npx jest --config jest.config.ts --no-color --ci currently fails in existing baza/server tests that call live Zerocracy endpoints and now receive token/CSRF HTTP errors. The changed to_gpt test file passes.

@GHX5T-SOL GHX5T-SOL left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified the to_gpt comma-spacing fix for #26. The regex now removes spaces before ?, !, ,, and : without the unreachable |, alternation branch.

Validation:

  • npm ci -> completed; npm reports existing audit findings, and this PR does not change dependencies
  • npx jest --config jest.config.ts --no-color --ci test/to_gpt.test.ts --coverage=false -> 1 suite passed, 2 tests passed
  • npx eslint src/to_gpt.ts test/to_gpt.test.ts --config eslint.config.mjs -> passed
  • npx tsc --noEmit -> passed
  • git diff --check origin/master...HEAD -> passed
  • git diff origin/master...HEAD | gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found

Live pre-review state: open, non-draft, mergeable/UNSTABLE only because no GitHub checks are reported, no existing comments/reviews. Same-scope open PR search for to_gpt comma punctuation spacing returned only this PR. I did not run the full live endpoint suite.

@kreinba kreinba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fix is correct: the character class [?!,:] replaces the broken \?|!|\|,|: alternation, whose third branch was the literal two-char sequence |, rather than a bare , — exactly the defect named in #26. Diff stays scoped to src/to_gpt.ts plus the regression in test/to_gpt.test.ts, which covers each of the four punctuation marks. Verified locally on head 447d947: npx jest test/to_gpt.test.ts is green, npx tsc --noEmit and npx eslint src/to_gpt.ts test/to_gpt.test.ts are clean. No CI checks have been reported on the branch.

@0crat

0crat commented Jun 22, 2026

Copy link
Copy Markdown

@kreinba Thanks for the review! You've earned +6 points for this: +18 as a basis; -8 for absolutely no comments posted; -4 for too few (6) hits-of-code. Your running score is +1458; don't forget to check your Zerocracy account too).

@akmhatey-ai

Copy link
Copy Markdown
Author

Merged current master into this branch and pushed head b75561a; GitHub now reports the PR as CLEAN and MERGEABLE.

Validation:

  • npm ci -> completed. npm reports existing audit/deprecation findings; this PR diff still only touches src/to_gpt.ts and test/to_gpt.test.ts.
  • npm exec jest -- --config jest.config.ts --no-color --ci test/to_gpt.test.ts --coverage=false -> 1 suite, 2 tests passed.
  • npm exec jest -- --config jest.config.ts --no-color --ci --coverage=false -> 6 suites, 22 tests passed.
  • npm exec eslint -- src/to_gpt.ts test/to_gpt.test.ts --config eslint.config.mjs -> passed.
  • npm exec eslint -- . --config eslint.config.mjs -> passed.
  • Makefile-equivalent TypeScript compile for non-test sources -> passed.
  • git diff --check origin/master...HEAD -> clean.
  • git merge-tree --write-tree origin/master HEAD -> c87ba11ef632e5231e256adeed81d301fc097e19.
  • Diff-scoped gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found.

Hosted checks on b75561a pass: actionlint, checkmake, copyrights, eslint, make, markdown-lint, pdd, reuse, typos, xcop, and yamllint.

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.

to_gpt function fails to strip space before commas due to incorrect placement of alternation in regex

4 participants