Skip to content

fix: tighten simplification marker rule#491

Open
sanmaxdev wants to merge 3 commits into
DietrichGebert:mainfrom
sanmaxdev:fix/120-marker-boundary
Open

fix: tighten simplification marker rule#491
sanmaxdev wants to merge 3 commits into
DietrichGebert:mainfrom
sanmaxdev:fix/120-marker-boundary

Conversation

@sanmaxdev

Copy link
Copy Markdown
Contributor

Summary

  • Tightens the simplification marker rule so normal or trivial code gets no ponytail: comment.
  • Syncs the updated wording across the copied rule files, runtime instructions, and generated OpenClaw skill.
  • Updates the Pi helper regression assertion for the new boundary text.

Testing

  • node scripts/check-rule-copies.js
  • node scripts/check-versions.js
  • node --test benchmarks/*.test.js pi-extension/test/helpers.test.js tests/hooks.test.js tests/openclaw-skills.test.js
  • npm test
  • git diff --check

Closes #120

Comment thread AGENTS.md Outdated
Addresses reviewer feedback to eliminate the ponytail: comment
reference that creates confusion without prior context.
@sanmaxdev

Copy link
Copy Markdown
Contributor Author

Good point about the confusing reference. Removed the ponytail: comment mention completely to avoid the 'pink elephant' problem.

  • git diff --check: passed
  • Updated wording is cleaner without the distracting reference

Comment thread AGENTS.md
- Question complex requests: "Do you actually need X, or does Y cover it?"
- Pick the edge-case-correct option when two stdlib approaches are the same size, lazy means less code, not the flimsier algorithm.
- Mark only deliberate simplifications that cut a real corner with a known ceiling (global lock, O(n²) scan, naive heuristic). Normal or trivial code gets no `ponytail:` comment; when a shortcut does need one, the comment names the ceiling and upgrade path.
- Mark deliberate simplifications that cut a real corner with a known ceiling (global lock, O(n²) scan, naive heuristic) with a comment naming the ceiling and upgrade path.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you need to apply this same change in ALL the other places.

@sanmaxdev

Copy link
Copy Markdown
Contributor Author

Updated the copied rule text to match the new wording everywhere.

Checks:

  • node scripts/check-rule-copies.js
  • npm test
  • git diff --check

@kydorn kydorn 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.

Lgtm

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.

Comment and commit behavior.

2 participants