-
Notifications
You must be signed in to change notification settings - Fork 13
Try to get the robots to not be Architecture Astronauts #549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| ______________________________________________________________________ | ||
|
|
||
| ## description: Apply KISS and pragmatic simplification to the current task, plan, or code. Invoke when the user says kiss, keep it simple, be pragmatic, don't overcomplicate, simplify this, reduce abstraction, avoid overengineering, or asks for the smallest correct change. argument-hint: "[optional focus]" | ||
|
|
||
| # Pragmatic | ||
|
|
||
| Use the K.I.S.S. guidance from `CLAUDE.md` to simplify the current task, plan, or code without dropping correctness. | ||
|
|
||
| ## Instructions | ||
|
|
||
| When this skill is invoked with `$ARGUMENTS`, execute the following sections in order. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 1. Anchor on the concrete need | ||
|
|
||
| Identify the current concrete use case before proposing changes. | ||
|
|
||
| - Optimize for today's requirement, not hypothetical future reuse. | ||
| - If `$ARGUMENTS` names a specific area, use it as the simplification target. | ||
| - If the request is broad, infer the narrowest immediate objective from the conversation and code. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 2. Inspect existing code before inventing structure | ||
|
|
||
| Look for existing utilities, patterns, and nearby implementations before proposing new abstractions. | ||
|
|
||
| - Reuse or lightly refactor existing code when it already solves most of the problem. | ||
| - Do not introduce new layers just because they look architecturally neat. | ||
| - Prefer local edits over cross-cutting reorganization unless the current shape is clearly blocking correctness. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 3. Prefer the smallest direct implementation | ||
|
|
||
| Bias toward the simplest implementation that cleanly solves the current problem. | ||
|
|
||
| - Avoid abstraction for hypothetical extensibility or configuration. | ||
| - Keep linear workflows easy to follow from top to bottom. | ||
| - Inline tiny one-off helpers when extraction does not materially improve readability, testability, or error handling. | ||
| - Prefer explicit parameter passing over stateful helper objects when the workflow is local and sequential. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 4. Keep data and control flow lightweight | ||
|
|
||
| Reduce indirection in both data shapes and function structure. | ||
|
|
||
| - Prefer tuples, dicts, or existing objects over tiny one-off dataclasses, wrappers, or config classes. | ||
| - Avoid one-off builders, factories, registries, or strategy objects unless they remove real duplication or support | ||
| multiple active implementations. | ||
| - If a function is accumulating mode flags, optional callbacks, or branching setup paths, prefer separate simple entry | ||
| points over one generic function. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 5. Preserve correctness while simplifying | ||
|
|
||
| Do not simplify away behavior that is required for safe and correct code. | ||
|
|
||
| - Keep validation, error handling, and required invariants intact. | ||
| - Do not remove tests, guards, or explicit exceptions that protect against invalid state. | ||
| - Do not churn stable public APIs or established shared abstractions unless they are part of the actual problem. | ||
| - Defer generalization until a second real use case exists, but do not delete real reuse that is already paying for | ||
| itself. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 6. Produce a pragmatic answer | ||
|
|
||
| When responding to the user, structure the output around the simplest viable path. | ||
|
|
||
| - State the recommended simple approach first. | ||
| - Name the specific complexity to avoid and why it is unnecessary for the current use case. | ||
| - If editing code, make the smallest coherent change set that solves the problem. | ||
| - If reviewing a plan, call out where the plan is over-generalized and replace it with a narrower implementation path. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 7. Apply these default heuristics | ||
|
|
||
| Use these checks when deciding whether to simplify: | ||
|
|
||
| - A helper called once or twice and only a few lines long usually should be inlined. | ||
| - A short-lived internal type with two small fields usually should be a tuple, dict, or existing object. | ||
| - A function with several boolean flags usually should become separate entry points by use case. | ||
| - A new abstraction justified only by "we might need this later" usually should not be added. | ||
| - If the codebase already has a utility that solves the problem, use it instead of re-implementing it. | ||
|
|
||
| ______________________________________________________________________ | ||
|
|
||
| ### 8. Keep these examples in mind | ||
|
|
||
| Prefer inlining tiny one-off helpers: | ||
|
|
||
| ```python | ||
| # Avoid | ||
| def _normalized_node_id(node_id: str) -> str: | ||
| return node_id.strip().lower() | ||
|
|
||
|
|
||
| def load_node(node_id: str) -> Node: | ||
| return node_store[_normalized_node_id(node_id)] | ||
|
|
||
|
|
||
| # Prefer | ||
| def load_node(node_id: str) -> Node: | ||
| normalized_node_id = node_id.strip().lower() | ||
| return node_store[normalized_node_id] | ||
| ``` | ||
|
|
||
| Prefer lightweight internal data over tiny one-off types: | ||
|
|
||
| ```python | ||
| # Avoid | ||
| @dataclass(frozen=True) | ||
| class BatchBounds: | ||
| start_index: int | ||
| end_index: int | ||
|
|
||
|
|
||
| batch_bounds = BatchBounds(start_index=0, end_index=128) | ||
|
|
||
|
|
||
| # Prefer | ||
| batch_bounds: tuple[int, int] = (0, 128) | ||
| start_index, end_index = batch_bounds | ||
| ``` | ||
|
|
||
| Prefer separate simple entry points over one generic function with flags: | ||
|
|
||
| ```python | ||
| # Avoid | ||
| def write_output(records: list[Record], *, validate: bool, compress: bool, upload: bool) -> None: | ||
| ... | ||
|
|
||
|
|
||
| # Prefer | ||
| def write_local_output(records: list[Record]) -> None: | ||
| ... | ||
|
|
||
|
|
||
| def write_uploaded_output(records: list[Record]) -> None: | ||
| ... | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,14 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co | |
| ## Coordination With AGENTS.md | ||
|
|
||
| - CLAUDE.md is the canonical source of truth for project context, architecture intent, and workflow conventions. | ||
| - AGENTS.md should only say to load CLAUDE.md. | ||
| - AGENTS.md should direct agents to read CLAUDE.md and discover/use skills under `.claude/skills`. | ||
|
|
||
| ## Skills | ||
|
|
||
| - Repository-local skills live under `.claude/skills/`. | ||
| - At the start of every session, discover available skills under `.claude/skills` along with reading AGENTS.md and | ||
| CLAUDE.md. | ||
| - When a relevant skill exists for the task, use it and follow its instructions. | ||
|
|
||
| ## Project Overview | ||
|
|
||
|
|
@@ -124,6 +131,80 @@ development. | |
| - Use OOP for model architectures, functional style for data transforms/pipelines. | ||
| - Re-use and refactor existing code as a priority instead of implementing new code. | ||
|
|
||
| ### K.I.S.S. (Keep It Simple, Stupid) | ||
|
|
||
| - Prefer the simplest implementation that solves the current concrete use case. Do not add abstraction for hypothetical | ||
| reuse, extensibility, or future configuration. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eh..... our code will not be built for the future at all :\
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Frankly, I think that this is probably the direction swe is heading in, with code being "disposable". FWIW this is mostly aimed at codex, (I guess I could put it in AGENTS.md but then we have the conflicting source(s) of truth, where codex does tend to over-engineer everything. And anyways code can change much easier now, if we need to expand it, it's easy to do so.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With gaurdrails *** |
||
| - Reduce indirection. If a helper is only called once or twice and is only a few lines, inline it unless extracting it | ||
| clearly improves readability, testability, or error handling. | ||
| - Keep workflows easy to follow from top to bottom. Avoid splitting a linear flow across many tiny functions, classes, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An overuse of this is a code smell with long methods which can be very hard to parse for humans.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've had this conversation a while ago, ime one large function is often easier to read then needing to go through some complicated call tree to find out what's actually happening. I can update this to something like "Functions that are < 5 lines are often not useful. Make sure short functions have real utility (e.g. are commonly used, or provide good type safety) before adding them"?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed some counter examples offline of this thread and https://github.com/Snapchat/GiGL/pull/549/changes#r2962912838 Aligned on creating a skill for this instead to ensure that this context can be added as needed since there is a differential between the models in how verbose / how much they over engineer or don't engineer enough. |
||
| or files just to make each unit smaller. | ||
| - Keep internal data shapes lightweight. For short-lived internal plumbing, prefer tuples, dicts, or existing objects | ||
| over tiny dataclasses, wrapper classes, or new types. | ||
| - Avoid one-off layers. Do not introduce builders, factories, registries, strategy objects, or config classes unless | ||
| they remove real duplication or support multiple active implementations. | ||
| - Defer generalization until there is a second real use case. Solve today's problem directly first, then extract shared | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem i find most often with using CC and agentic development right now is that it is already having a difficult time re-using existing code or refactoring existing code that lives elsewhere to reduce number of lines managed. In this case we are rewarding something that is objectively something I combat with every day.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ( FWIW, I just reviewed another PR (soon after this) where CC completely disregarded a functionality we already had in code base and it just re-implemented it. I am afraid this will make it even harder for CC to re-use and not rebuild. )
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm, I added some
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| structure only when repetition becomes concrete. | ||
| - Prefer explicit parameter passing over stateful objects when the workflow is local and sequential. | ||
| - Avoid boolean soup. If a function starts accumulating mode flags, optional callbacks, or branching setup paths, | ||
| consider splitting it by use case instead of growing a generic entry point. | ||
| - Don't re-invent the wheel. Check the code base for utilities that solve the task at hand. | ||
|
|
||
| **Small examples** | ||
|
|
||
| Prefer inlining tiny one-off helpers: | ||
|
|
||
| ```python | ||
| # Avoid | ||
| def _normalized_node_id(node_id: str) -> str: | ||
| return node_id.strip().lower() | ||
|
|
||
|
|
||
| def load_node(node_id: str) -> Node: | ||
| return node_store[_normalized_node_id(node_id)] | ||
|
|
||
|
|
||
| # Prefer | ||
| def load_node(node_id: str) -> Node: | ||
| normalized_node_id = node_id.strip().lower() | ||
| return node_store[normalized_node_id] | ||
| ``` | ||
|
|
||
| Prefer lightweight internal data over tiny one-off types: | ||
|
|
||
| ```python | ||
| # Avoid | ||
| @dataclass(frozen=True) | ||
| class BatchBounds: | ||
| start_index: int | ||
| end_index: int | ||
|
|
||
|
|
||
| batch_bounds = BatchBounds(start_index=0, end_index=128) | ||
|
|
||
|
|
||
| # Prefer | ||
| batch_bounds: tuple[int, int] = (0, 128) | ||
| start_index, end_index = batch_bounds | ||
| ``` | ||
|
|
||
| Prefer separate simple entry points over one generic function with flags: | ||
|
|
||
| ```python | ||
| # Avoid | ||
| def write_output(records: list[Record], *, validate: bool, compress: bool, upload: bool) -> None: | ||
| ... | ||
|
|
||
|
|
||
| # Prefer | ||
| def write_local_output(records: list[Record]) -> None: | ||
| ... | ||
|
|
||
|
|
||
| def write_uploaded_output(records: list[Record]) -> None: | ||
| ... | ||
| ``` | ||
|
|
||
| ### Fail Fast on Invalid State | ||
|
|
||
| - Use `dict[key]` (bracket access) when the key **must** exist. Only use `.get(key, default)` when absence is a valid, | ||
|
|
@@ -134,7 +215,8 @@ development. | |
|
|
||
| - Always use type annotations for function parameters and return values. | ||
| - Prefer native types (`dict[str, str]`, `list[int]`) over `typing.Dict`, `typing.List`. | ||
| - Use `Final` for constants. Use `@dataclass(frozen=True)` for immutable data containers. | ||
| - Use `Final` for constants. Use `@dataclass(frozen=True)` for immutable data containers when named fields and a stable | ||
| shape add real clarity; do not introduce a dataclass for tiny internal-only plumbing. | ||
| - Always annotate empty containers: `names: list[str] = []` not `names = []`. | ||
|
|
||
| ### Docstrings | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
claude already does this afaik - we don't need these instructions.
You can try this yourself - when you open CC, type
/contextit should list all skills that are in context