Context
These three items were identified during the PR #83 review (weighted scoring classifier merge). None are blocking — the PR was approved. But they're quality issues worth addressing in a follow-up.
S1: Silent exception swallowing in signal collection
File: git_acp/git/classification.py:569
try:
numstat = get_numstat(config)
except (GitError, Exception):
passTwo problems:
Exception subsumes GitError, making the tuple redundant — it's just except Exception.pass silently swallows programming errors (TypeError, AttributeError, KeyError) that indicate bugs in get_numstat or upstream callers. When numstat is empty due to a bug rather than "no staged changes", misclassification becomes impossible to diagnose.
Suggested fix:
try:
numstat = get_numstat(config)
except GitError:
pass # expected: no staged/unstaged changes
except Exception as err:
if config.verbose:
debug_item("Numstat unexpected error", str(err))
S2: Hardcoded .env exclusion pattern in numstat
File: git_acp/git/diff.py:188
for pattern in EXCLUDED_PATTERNS:
if pattern == "/.env$" and Path(filepath).name == ".env":
excluded = True
break
if pattern in filepath:
excluded = True
breakThe generic fallback (pattern in filepath) is substring matching, not regex. So /.env$ would never match via the fallback — the special case is the only thing making it work. Any future regex pattern added to EXCLUDED_PATTERNS would silently fail to filter.
Suggested fix: Use the existing _match_file_path_pattern from file_classifier.py for consistent pattern matching, or at minimum add a comment documenting that EXCLUDED_PATTERNS entries are expected to be literal substrings (except /.env$ which is special-cased).
S3: Duplicated commit-type literals in CLI
File: git_acp/cli/cli.py:157-177
The 11 commit types are hardcoded in both click.Choice([...]) and the help= string. Two independent sources of truth — adding a new type requires updating both, and they can silently drift.
Suggested fix:
CLI_COMMIT_TYPE_CHOICES = tuple(ct.name.lower() for ct in CommitType)
@click.option(
"--type",
"commit_type",
type=click.Choice(CLI_COMMIT_TYPE_CHOICES, case_sensitive=False),
help=(
"Manually specify the commit type instead of using automatic detection. "
f"Supported types: {', '.join(CLI_COMMIT_TYPE_CHOICES)}."
),
metavar="<type>",
)
Priority
S1 > S2 > S3. The exception swallowing is the most impactful for debuggability.
Context
These three items were identified during the PR #83 review (weighted scoring classifier merge). None are blocking — the PR was approved. But they're quality issues worth addressing in a follow-up.
S1: Silent exception swallowing in signal collection
File:
git_acp/git/classification.py:569Two problems:
ExceptionsubsumesGitError, making the tuple redundant — it's justexcept Exception.passsilently swallows programming errors (TypeError, AttributeError, KeyError) that indicate bugs inget_numstator upstream callers. When numstat is empty due to a bug rather than "no staged changes", misclassification becomes impossible to diagnose.Suggested fix:
S2: Hardcoded
.envexclusion pattern in numstatFile:
git_acp/git/diff.py:188The generic fallback (
pattern in filepath) is substring matching, not regex. So/.env$would never match via the fallback — the special case is the only thing making it work. Any future regex pattern added toEXCLUDED_PATTERNSwould silently fail to filter.Suggested fix: Use the existing
_match_file_path_patternfromfile_classifier.pyfor consistent pattern matching, or at minimum add a comment documenting thatEXCLUDED_PATTERNSentries are expected to be literal substrings (except/.env$which is special-cased).S3: Duplicated commit-type literals in CLI
File:
git_acp/cli/cli.py:157-177The 11 commit types are hardcoded in both
click.Choice([...])and thehelp=string. Two independent sources of truth — adding a new type requires updating both, and they can silently drift.Suggested fix:
Priority
S1 > S2 > S3. The exception swallowing is the most impactful for debuggability.