Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/ponytail/src/skill-audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ thing, dead flags and config, hand-rolled stdlib.

## Output

One line per finding, ranked: `<tag> <what to cut>. <replacement>. [path]`.
Number each finding. One line per finding, ranked: `<N>. <tag> <what to cut>. <replacement>. [path]`.
End with `net: -<N> lines, -<M> deps possible.` Nothing to cut: `Lean already. Ship.`

## Boundaries
Expand Down
29 changes: 20 additions & 9 deletions crates/ponytail/src/skill-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ description: >
hunts complexity.
---

Review diffs for unnecessary complexity. One line per finding: location, what
to cut, what replaces it. The diff's best outcome is getting shorter.
Review diffs for unnecessary complexity. Number each finding sequentially.
One line per finding: location, what to cut, what replaces it.
The diff's best outcome is getting shorter.

## Format

`L<line>: <tag> <what>. <replacement>.`, or `<file>:L<line>: ...` for
multi-file diffs.
`<N>. <file>:L<line>: <tag> <what>. <replacement>.`

Tags:

Expand All @@ -26,20 +26,31 @@ Tags:
- `yagni:` abstraction with one implementation, config nobody sets, layer with one caller.
- `shrink:` same logic, fewer lines. Show the shorter form.

## Common patterns (pre-filter these before deeper analysis)

| Pattern | Tag | Why |
|---------|-----|-----|
| moment.js import | native | Intl.DateTimeFormat or Temporal |
| lodash import | stdlib | Array.map, Array.filter, Object.keys |
| axios import | native | fetch() is built-in |
| JSON.parse(JSON.stringify(...)) | stdlib | structuredClone() |
| Trivial getter/setter class | yagni | Plain object, no class needed |
| Pass-through wrapper | yagni | Direct call, skip wrapper |

## Examples

❌ "This EmailValidator class might be more complex than necessary, have you
considered whether all these validation rules are needed at this stage?"

✅ `L12-38: stdlib: 27-line validator class. "@" in email, 1 line, real validation is the confirmation mail.`
✅ `1. <file>:L12-38: stdlib: 27-line validator class. "@" in email, 1 line, real validation is the confirmation mail.`

✅ `L4: native: moment.js imported for one format call. Intl.DateTimeFormat, 0 deps.`
✅ `2. <file>:L4: native: moment.js imported for one format call. Intl.DateTimeFormat, 0 deps.`

✅ `repo.py:L88: yagni: AbstractRepository with one implementation. Inline it until a second one exists.`
✅ `3. repo.py:L88: yagni: AbstractRepository with one implementation. Inline it until a second one exists.`

✅ `L52-71: delete: retry wrapper around an idempotent local call. Nothing replaces it.`
✅ `4. <file>:L52-71: delete: retry wrapper around an idempotent local call. Nothing replaces it.`

✅ `L30-44: shrink: manual loop builds dict. dict(zip(keys, values)), 1 line.`
✅ `5. <file>:L30-44: shrink: manual loop builds dict. dict(zip(keys, values)), 1 line.`

## Scoring

Expand Down
96 changes: 96 additions & 0 deletions crates/ponytail/src/sub_skills.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,99 @@ pub fn get(name: &str) -> Option<&'static str> {
_ => None,
}
}

pub struct Finding {
pub line: usize,
pub tag: String,
pub problem: String,
pub replacement: String,
pub snippet: String,
}

pub fn detect_over_engineering(text: &str) -> Vec<Finding> {
let mut findings = Vec::new();
ponytail_engineering_check_internal(text, &mut findings, 0);
findings.sort_by_key(|f| f.line);
findings
}

fn ponytail_engineering_check_internal(text: &str, findings: &mut Vec<Finding>, base_line: usize) {
let patterns: &[(&str, &str, &str, &[&str])] = &[
("lodash", "stdlib", "Use native JS methods: Array.map, Array.filter, Object.keys.", &["from \"lodash\"", "from 'lodash'", "require(\"lodash\")", "require('lodash')"]),
("moment", "native", "Use Intl.DateTimeFormat, Date.toLocaleDateString, or Temporal.", &["from \"moment\"", "from 'moment'", "require(\"moment\")", "require('moment')"]),
("axios", "native", "Use native fetch() instead of axios.", &["from \"axios\"", "from 'axios'", "require(\"axios\")", "require('axios')"]),
("JSON.parse(JSON.stringify(", "stdlib", "Use structuredClone() for deep copy.", &["JSON.parse(JSON.stringify("]),
];

for (line_num, line) in text.lines().enumerate() {
let lower = line.to_lowercase();
for (_name, tag, replacement, needles) in patterns {
if needles.iter().any(|n| lower.contains(&n.to_lowercase())) {
let trimmed = line.trim().to_string();
if !findings.iter().any(|f| f.line == base_line + line_num + 1 && f.problem == trimmed) {
findings.push(Finding {
line: base_line + line_num + 1,
tag: (*tag).to_string(),
problem: trimmed,
replacement: (*replacement).to_string(),
snippet: line.to_string(),
});
}
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn detects_lodash_import() {
let findings = detect_over_engineering("import _ from \"lodash\";\nconst x = 1;");
assert_eq!(findings.len(), 1);
assert_eq!(findings[0].tag, "stdlib");
assert!(findings[0].replacement.contains("Array.map"));
}

#[test]
fn detects_moment_import() {
let findings = detect_over_engineering("import moment from \"moment\";");
assert_eq!(findings.len(), 1);
assert_eq!(findings[0].tag, "native");
}

#[test]
fn detects_axios_import() {
let findings = detect_over_engineering("const axios = require(\"axios\");");
assert_eq!(findings.len(), 1);
assert_eq!(findings[0].tag, "native");
}

#[test]
fn detects_deep_clone_antipattern() {
let findings = detect_over_engineering("const copy = JSON.parse(JSON.stringify(obj));");
assert_eq!(findings.len(), 1);
assert_eq!(findings[0].tag, "stdlib");
assert!(findings[0].replacement.contains("structuredClone"));
}

#[test]
fn clean_code_returns_empty() {
let findings = detect_over_engineering("const x = 1;\nfn foo() { Ok(()) }");
assert!(findings.is_empty());
}

#[test]
fn detects_single_quoted_imports() {
let findings = detect_over_engineering("import lodash from 'lodash';");
assert_eq!(findings.len(), 1);
assert_eq!(findings[0].tag, "stdlib");
}

#[test]
fn ignores_non_lodash_underscore_import() {
let findings = detect_over_engineering("import _ from \"underscore\";");
assert!(findings.is_empty());
}
}
Loading