Skip to content

Commit dbf43c3

Browse files
committed
refactor: reduce humanize finding state
Keep the same locations and labels with fewer objects and passes.
1 parent 7811911 commit dbf43c3

2 files changed

Lines changed: 41 additions & 84 deletions

File tree

.github/scripts/test_humanize.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,18 @@ def test_reports_each_file_location_with_context(self):
100100
"""Report every file finding with line, column, fix, and context."""
101101
reason = run_hook(
102102
"Write",
103-
{"file_path": "README.md", "content": f"First line.\nAI sections{SEMICOLON} it does not{SEMICOLON}\n"},
103+
{
104+
"file_path": "README.md",
105+
"content": f"First line.\nAI sections{SEMICOLON} it does not{SEMICOLON}\nWe leverage tools.\nIn conclusion, done.\n",
106+
},
104107
)
105108
self.assertIn(
106109
f'- semicolon at README.md:2:12, use a period or comma: "AI sections{SEMICOLON} it does not{SEMICOLON}"',
107110
reason,
108111
)
109112
self.assertEqual(reason.count("- semicolon at"), 2)
113+
self.assertIn('"leverage" at README.md:3:4, use "use"', reason)
114+
self.assertIn('"In conclusion" at README.md:4:1, drop it', reason)
110115

111116
def test_caps_pileup_locations(self):
112117
"""Report five pile-up locations and count the remainder."""

plugins/humanize/hooks/scripts/humanize.py

Lines changed: 35 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
import re
1414
import shlex
1515
import sys
16-
from collections import Counter
17-
from dataclasses import dataclass
16+
from collections import namedtuple
1817
from pathlib import Path
1918

2019
# TODO: build a companion guidance skill from that page's pitfalls, not only its word list
@@ -78,47 +77,17 @@
7877
REDIRECT = re.compile(r"(?<![0-9&])>>?[ \t]*(\"[^\"]*\"|'[^']*'|[^\s'\"|&;<>]+)")
7978

8079
# MCP input keys that carry human-facing message text, checked as markdown
81-
TEXT_KEYS = {"body", "text", "markdown_text", "content", "description", "title",
82-
"comment", "message", "subject", "note", "summary", "richtext", "rich_text"}
80+
MESSAGE_KEYS = {"body", "text", "markdown_text", "content", "comment", "message"}
81+
TEXT_KEYS = MESSAGE_KEYS | {"description", "title", "subject", "note", "summary", "richtext", "rich_text"}
8382

8483
MD_EXT = {".md", ".markdown", ".mdx"}
8584
HASH_EXT = {".py", ".sh", ".bash", ".zsh", ".rb", ".yaml", ".yml", ".toml"}
8685
C_EXT = {".js", ".ts", ".jsx", ".tsx", ".c", ".cc", ".cpp", ".h", ".hpp", ".java", ".go", ".rs", ".css", ".scss", ".swift", ".kt", ".php"}
8786
# fmt: on
8887

8988

90-
@dataclass(frozen=True)
91-
class Region:
92-
"""Store checkable text without losing its source coordinates.
93-
94-
Attributes:
95-
source (str): File path or non-file source label.
96-
text (str): Source-length text with excluded content masked.
97-
"""
98-
99-
source: str
100-
text: str
101-
102-
103-
@dataclass(frozen=True)
104-
class Finding:
105-
"""Store one detector match and its source span.
106-
107-
Attributes:
108-
region (Region): Source region containing the match.
109-
start (int): Match start offset in the region.
110-
end (int): Match end offset in the region.
111-
rule (str): Human-readable rule name.
112-
replacement (str): Suggested correction.
113-
pileup (bool): Whether the rule depends on repeated use.
114-
"""
115-
116-
region: Region
117-
start: int
118-
end: int
119-
rule: str
120-
replacement: str
121-
pileup: bool = False
89+
Region = namedtuple("Region", "source text")
90+
Finding = namedtuple("Finding", "region start end rule replacement")
12291

12392

12493
def masked(text, keep):
@@ -129,10 +98,9 @@ def masked(text, keep):
12998
def md_text(text):
13099
"""Mask fenced and inline Markdown code without changing source offsets."""
131100
keep = [True] * len(text)
132-
lines = text.splitlines(keepends=True)
133101
offset = 0
134102
fence = None
135-
for line in lines:
103+
for line in text.splitlines(keepends=True):
136104
match = re.match(r" {0,3}(`{3,}|~{3,})(.*)$", line.rstrip("\r\n"))
137105
if fence:
138106
keep[offset : offset + len(line)] = [False] * len(line)
@@ -265,8 +233,7 @@ def bash_text(command):
265233
gh = re.search(r"\bgh\b", command)
266234
if not (git_commit or gh):
267235
return []
268-
is_pr = re.search(r"\bgh\s+pr\b", command)
269-
body_source = "PR body" if is_pr else "GitHub body"
236+
body_source = "PR body" if re.search(r"\bgh\s+pr\b", command) else "GitHub body"
270237
parts = [
271238
("commit message" if re.search(r"\bgit\b", head) else body_source, body)
272239
for head, body in heredocs(command)
@@ -351,16 +318,16 @@ def patch_regions(patch):
351318

352319

353320
def mcp_text(obj):
354-
"""Return human-facing field names and values from an MCP tool input."""
321+
"""Yield human-facing field names and values from an MCP tool input."""
355322
if isinstance(obj, dict):
356-
items = [
357-
[(k.lower(), v)] if k.lower() in TEXT_KEYS and isinstance(v, str) else mcp_text(v) for k, v in obj.items()
358-
]
323+
for key, value in obj.items():
324+
if key.lower() in TEXT_KEYS and isinstance(value, str):
325+
yield key.lower(), value
326+
else:
327+
yield from mcp_text(value)
359328
elif isinstance(obj, list):
360-
items = [mcp_text(v) for v in obj]
361-
else:
362-
return []
363-
return [text for group in items for text in group]
329+
for value in obj:
330+
yield from mcp_text(value)
364331

365332

366333
def extract(tool, tool_input):
@@ -369,18 +336,11 @@ def extract(tool, tool_input):
369336
if isinstance(command, list): # Codex sends the shell tool an argv array, Claude Code a string
370337
command = " ".join(str(c) for c in command)
371338
if tool.startswith("mcp__"):
372-
server = tool.split("__", 2)[1]
373-
if server.startswith("claude_ai_"):
374-
server = server.removeprefix("claude_ai_")
375-
elif server == "codex_apps":
376-
server = tool.split("__", 2)[2].split("_", 1)[0]
339+
namespace, name = tool.split("__", 2)[1:]
340+
server = name.split("_", 1)[0] if namespace == "codex_apps" else namespace.removeprefix("claude_ai_")
377341
regions = []
378342
for field, text in mcp_text(tool_input):
379-
kind = (
380-
"message"
381-
if field in {"body", "text", "markdown_text", "content", "comment", "message"}
382-
else field.replace("_", " ")
383-
)
343+
kind = "message" if field in MESSAGE_KEYS else field.replace("_", " ")
384344
regions.append(Region(f"{server.replace('_', ' ').title()} {kind}", md_text(text)))
385345
return regions
386346
if tool == "Bash":
@@ -397,32 +357,29 @@ def extract(tool, tool_input):
397357
def detect(regions):
398358
"""Return every rule occurrence with its source span."""
399359
findings = []
400-
often = []
360+
piles = {}
401361
for region in regions:
402362
for match in MARK_RE.finditer(region.text):
403363
rule, replacement = MARKS[match.group()]
404-
findings.append(Finding(region, match.start(), match.end(), rule, replacement))
364+
findings.append(Finding(region, *match.span(), rule, replacement))
405365
for match in SWAP_RE.finditer(region.text):
406366
word = match.group().lower()
407367
replacement = SWAP[word]
408368
findings.append(
409369
Finding(
410370
region,
411-
match.start(),
412-
match.end(),
371+
*match.span(),
413372
f'"{word}"',
414373
replacement if replacement == "drop it" else f'use "{replacement}"',
415374
)
416375
)
417376
for pattern, replacement in PHRASES:
418377
for match in re.finditer(rf"\b(?:{pattern})\b", region.text, re.IGNORECASE):
419-
findings.append(Finding(region, match.start(), match.end(), f'"{match.group()}"', replacement))
420-
often += [
421-
Finding(region, match.start(), match.end(), f'"{match.group().lower()}"', "vary it", True)
422-
for match in OFTEN_RE.finditer(region.text)
423-
]
424-
counts = Counter(finding.rule for finding in often)
425-
return findings + [finding for finding in often if counts[finding.rule] >= LIMIT]
378+
findings.append(Finding(region, *match.span(), f'"{match.group()}"', replacement))
379+
for match in OFTEN_RE.finditer(region.text):
380+
rule = f'"{match.group().lower()}"'
381+
piles.setdefault(rule, []).append(Finding(region, *match.span(), rule, "vary it"))
382+
return findings, {rule: matches for rule, matches in piles.items() if len(matches) >= LIMIT}
426383

427384

428385
def location(finding):
@@ -437,22 +394,18 @@ def context(finding, width=60):
437394
line_start = text.rfind("\n", 0, finding.start) + 1
438395
line_end = text.find("\n", finding.end)
439396
line_end = len(text) if line_end < 0 else line_end
440-
start = max(line_start, finding.start - width // 2)
441-
end = min(line_end, finding.end + width // 2)
442-
snippet = ("..." if start > line_start else "") + text[start:end].strip() + ("..." if end < line_end else "")
397+
clip_start = max(line_start, finding.start - width // 2)
398+
clip_end = min(line_end, finding.end + width // 2)
399+
snippet = ("..." if clip_start > line_start else "") + text[clip_start:clip_end].strip()
400+
snippet += "..." if clip_end < line_end else ""
443401
return json.dumps(snippet)
444402

445403

446-
def format_findings(findings):
404+
def format_findings(findings, piles):
447405
"""Format all findings into one actionable denial message."""
448406
lines = [
449-
f"- {finding.rule} at {location(finding)}, {finding.replacement}: {context(finding)}"
450-
for finding in findings
451-
if not finding.pileup
407+
f"- {finding.rule} at {location(finding)}, {finding.replacement}: {context(finding)}" for finding in findings
452408
]
453-
piles = {}
454-
for finding in (finding for finding in findings if finding.pileup):
455-
piles.setdefault(finding.rule, []).append(finding)
456409
for rule, matches in piles.items():
457410
shown = ", ".join(location(finding) for finding in matches[:5])
458411
more = f", +{len(matches) - 5} more" if len(matches) > 5 else ""
@@ -461,17 +414,16 @@ def format_findings(findings):
461414

462415

463416
data = json.load(sys.stdin)
464-
regions = extract(data.get("tool_name", ""), data.get("tool_input") or {})
465-
findings = detect(regions)
417+
findings, piles = detect(extract(data.get("tool_name", ""), data.get("tool_input") or {}))
466418

467-
if findings:
419+
if findings or piles:
468420
print(
469421
json.dumps(
470422
{
471423
"hookSpecificOutput": {
472424
"hookEventName": "PreToolUse",
473425
"permissionDecision": "deny",
474-
"permissionDecisionReason": format_findings(findings),
426+
"permissionDecisionReason": format_findings(findings, piles),
475427
}
476428
}
477429
)

0 commit comments

Comments
 (0)