Fix version regex to prevent false matches on compound keys

[ArieLevs]

Summary

• Add negative lookbehind (?<![a-zA-Z0-9_-]) to prevent matching keys like target-version, myversion, etc.
• Add detailed inline comments explaining each regex component
• Use pybump set --auto for unique test.pypi.org versions (appends commit SHA as release)

Test plan

• All 22 existing tests pass
• Verified fix works with pyproject.toml containing target-version = "py312"
• Verified fix works with pyproject.toml containing this-is-a-version-containing-key = "1.1.7"
• Original behavior preserved (inline versions, multiple version detection, __version__ pattern)

Fixes #58

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.52%. Comparing base (304d328) to head (665edab).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
test/test_simulate_pybump.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   95.42%   95.52%   +0.09%     
==========================================
  Files           5        5              
  Lines         459      469      +10     
==========================================
+ Hits          438      448      +10     
  Misses         21       21              

Flag	Coverage Δ
unittests	95.52% <92.85%> (+0.09%)	⬆️

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Code Review for PR #61: Fix version regex to prevent false matches on compound keys

Summary

This PR successfully addresses issue #58 by adding a negative lookbehind assertion to the version regex pattern, preventing false matches on compound keys like target-version or myversion. The implementation is solid and well-documented.

✅ Strengths

1. Core Regex Fix - The negative lookbehind is the correct solution, preventing matching when version is preceded by alphanumeric characters, underscores, or hyphens

2. Excellent Documentation - The inline comments are exceptionally clear and helpful, making future maintenance easier

3. Workflow Improvement - Using pybump set --auto to generate unique versions for test.pypi.org is practical

🔍 Potential Issues

Missing Test Coverage ⚠️ CRITICAL

While the PR description mentions manual testing, there are no new automated tests in the test suite. The test files were not modified.

Recommendation: Add automated tests for compound key scenarios to prevent regression:

• File with target-version alongside actual version
• File with myversion alongside actual version
• File with version at the very beginning of a line

Workflow Change Suggestion
Consider chaining commands with AND operator for better error handling in the workflow file.

🔒 Security & Performance

✅ No security concerns
✅ No performance concerns - regex compiled once at module load

✅ Final Verdict

Approval Status: Approve with minor recommendations

The PR successfully:
✅ Fixes the reported bug
✅ Maintains backward compatibility
✅ Adds helpful documentation
✅ Improves the CI workflow
⚠️ Needs automated test coverage for regression prevention

Risk Level: Low

Great work on the fix and documentation! The implementation is sound. Adding automated test cases will make this PR excellent.

[claude]

Code Review - PR #61: Fix version regex to prevent false matches on compound keys

Overview

This PR fixes issue #58 where PyBump incorrectly matched keys containing "version" as a substring (e.g., target-version, myversion). The fix adds a negative lookbehind to the regex and introduces a new --metadata flag for PEP 440 compliant version metadata.

---

✅ Code Quality and Best Practices

Strengths:

• Excellent documentation: The inline regex comments in src/pybump.py:13-23 are exceptionally clear and educational. This is a model for documenting complex regex patterns.
• Clean refactoring: The test helper function simulate_set_version() was refactored well to handle the new metadata parameter using a dynamic command list approach (test/test_simulate_pybump.py:43-49).
• Good separation of concerns: The --metadata flag properly integrates into the existing --auto flag structure.

Suggestions:

1. Regex completeness: The negative lookbehind (?<![a-zA-Z0-9_-]) doesn't handle all edge cases. Consider scenarios like my.version = "1.0.0" (dot not in lookbehind) or version-old = "1.0.0" at start of line.

2. Docstring update needed: The simulate_set_version() function docstring mentions "auto add git branch / hash" but the code no longer uses branch name - only commit SHA. Update line 33 to reflect current behavior.

---

🐛 Potential Bugs and Issues

Critical:

1. Missing test case: The PR description shows an unchecked test item for this-is-a-version-containing-key = "1.1.7". This test case should be added to verify the fix handles keys ending with "version".

2. Quote matching inconsistency: The regex allows mismatched quotes. Consider using a backreference to ensure matching quotes.

Minor:

1. Capture group numbering: The comment says "Capture group 2: version value" but group indexing in get_version_from_file() at line 49 is fragile. Consider using named groups for clarity.

---

⚡ Performance Considerations

Good:

• Regex compilation happens once at module load time - excellent.
• The negative lookbehind is efficient and won't cause catastrophic backtracking.

No concerns - the changes have minimal performance impact.

---

🔒 Security Concerns

No security issues identified. The changes are low-risk:

• Regex doesn't introduce ReDoS vulnerabilities
• No user input is directly used in the regex pattern
• File operations remain unchanged

---

🧪 Test Coverage

Strengths:

• Added comprehensive test for new --metadata flag (test/test_simulate_pybump.py:255-267)
• Test uses appropriate regex pattern to validate output format
• Existing 22 tests still pass

Gaps:

1. Missing edge case tests:

   • No test for target-version = "py312" scenario (the actual bug from issue bump crash when version string exists more then once #58)
   • No test for myversion = "1.0.0" (version as suffix)
   • No test for this-is-a-version-containing-key = "1.1.7" (mentioned in test plan)

2. CI/CD change validation: The workflow change (.github/workflows/pythonpackage.yml:110-116) modifies how test.pypi.org versions are generated. Ensure this doesn't break existing deployments.

---

📋 Additional Recommendations

1. Version bump: The version was bumped from 1.13.2 to 1.14.0 (minor bump). This is appropriate given the new --metadata flag is a new feature.

2. Documentation update: Consider updating README.rst to document the new --metadata flag usage with examples.

3. Backward compatibility: The changes are backward compatible - existing functionality is preserved. Good!

4. PEP 440 compliance: The --metadata flag properly uses + for metadata vs - for release identifiers. Excellent adherence to standards.

---

Summary

This is a solid PR that fixes a real bug and adds useful functionality. The code quality is high, documentation is excellent, and the approach is sound.

Before merging:

• Add test case for target-version false positive scenario
• Add test case for compound keys ending with "version"
• Update simulate_set_version() docstring (line 33)
• Consider regex improvements for edge cases mentioned above
• Verify CI/CD changes work in test environment

Estimated effort to address: Low - mostly adding test cases.

Overall: Approved with minor suggestions ✅

---

Review generated with Claude Code

[claude]

Code Review: PR #61 - Fix version regex to prevent false matches on compound keys

Summary

This PR fixes issue #58 by improving the regex pattern to prevent matching compound keys like target-version or myversion. It also adds a new --metadata flag to the set command for PEP 440 compliant metadata handling.

Code Quality & Best Practices ✅

Strengths:

1. Excellent Documentation: The inline regex comments (lines 13-22 in src/pybump.py:13-23) are exceptionally clear and explain each component thoroughly. This is a model for regex documentation.

2. Comprehensive Test Coverage: The test additions are thorough:

   • Tests for compound keys (target-version, myversion)
   • Tests for inline versions (indented in setup.py style)
   • Tests for the new --metadata flag functionality
   • Clear assertion messages that explain what being tested

3. Backward Compatibility: The changes preserve existing behavior while fixing the bug. The --metadata flag is optional and does not break existing workflows.

4. Clean Code Refactoring: The simulate_set_version function refactoring improves maintainability by consolidating the command building logic.

Potential Issues & Concerns

1. Regex Edge Cases ⚠️

The negative lookbehind (?<\![a-zA-Z0-9_-]) might have edge cases:

Concern: What happens with these scenarios?

• Version at start of file: version = "1.0.0" (no preceding character)
• Version after special characters: {version = "1.0.0" or (version = "1.0.0"
• CamelCase: appVersion = "1.0.0" (should this match?)

Status: Likely works fine (lookbehind succeeds at start of string), but explicit tests would confirm.

2. Workflow Change Impact ⚠️

The workflow change modifies the version bumping strategy to use two commands. If the second command fails, you have a bumped version but no metadata set. Consider adding explicit error handling with || exit 1 after each command.

3. Documentation Gap 📝

The new --metadata flag is not documented anywhere except the help text. Users will not know when to use +sha vs -sha format without reading the code.

Recommendation: Add documentation explaining PEP 440 metadata format and when to use the --metadata flag.

Performance Considerations ✅

No performance concerns. The regex is efficient and the changes do not introduce any loops or expensive operations.

Security Concerns ✅

Git Command Injection: The code safely uses GitPython library methods rather than shell commands. Good practice.

No security issues identified.

Test Coverage Assessment

✅ Well Tested:

• Compound key filtering
• Inline version matching
• Metadata flag functionality
• Backward compatibility

🔍 Could Add:

1. Start-of-file version test
2. Version after special characters (braces, parentheses)
3. Multiple compound keys in same file
4. Error handling when git repo not available (TypeError path at line 272)

Recommendations

High Priority:

1. Add edge case tests for start-of-file and special-character-preceded versions
2. Add error handling to the workflow two-step version bump/set process

Medium Priority:

3. Document the --metadata flag usage in README or help documentation
4. Add a test for the git command fallback path (TypeError handling)

Verdict: ✅ APPROVE with Minor Suggestions

This is a well-crafted fix with excellent test coverage and documentation. The regex solution correctly addresses issue #58, and the metadata flag is a useful addition for test.pypi.org workflows.

Strengths: Great inline documentation, comprehensive tests, backward compatible
Concerns: Minor edge case testing gaps, workflow atomicity consideration

The code is production-ready. The suggestions above would make it even more robust, but they are not blockers.

Great work!