Try to get the robots to not be Architecture Astronauts#549
Try to get the robots to not be Architecture Astronauts#549kmontemayor2-sc wants to merge 3 commits intomainfrom
Conversation
kmontemayor2-sc
changed the title
svij-sc
left a comment
svij-sc
left a comment
There was a problem hiding this comment.
A lot of the instructions here are case by case dependent and I am afraid conflicting with the experience that I have had with CC.
Could we maybe create a skill instead /be-pragmatic that way we can re-use this when needed but not inject it into every single context?
| ### 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. |
There was a problem hiding this comment.
eh..... our code will not be built for the future at all :\
Tbh, with some of the stuff recently - Ive been prompting to go at it the other way.
There was a problem hiding this comment.
eh..... our code will not be built for the future at all :\
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.
There was a problem hiding this comment.
Frankly, I think that this is probably the direction swe is heading in, with code being "disposable".
With gaurdrails ***
The importance of good interfaces and tests is even more important now - the internals (implementation details are not much where we care about). The problem with this prompt is its actually going against good interface design.
| reuse, extensibility, or future configuration. | ||
| - 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, |
There was a problem hiding this comment.
An overuse of this is a code smell with long methods which can be very hard to parse for humans.
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
( 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. )
There was a problem hiding this comment.
Hmmm, I added some Don't re-invent the wheel. Check the code base for utilities that solve the task at hand. here, to try and it them to search for existing functions. I don't think this statement really discourages then looking for existing utils (though it does discourage them from creating new ones potentially).
There was a problem hiding this comment.
Again, this is mainly targeted at codex who tends to be more of an "Architecture Astronaut".
I do sort of want this in all the code, though, I have a feeling like we have a tendency to over-engineer as-is. I do think this does ultimately boil down to some philosophical differences between us (I prefer big dumb code, you have preference for smaller, neater, code). Is your main concern here that this will push the robots to produce code that is too many lines? I feel like I'm also trying to reduce the loc delta here, where often their code can be quite simplified. |
| - 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. |
There was a problem hiding this comment.
claude already does this afaik - we don't need these instructions.
You can try this yourself - when you open CC, type /context it should list all skills that are in context
4 participants
Scope of work done
Where is the documentation for this feature?: N/A
Did you add automated tests or write a test plan?
Updated Changelog.md? NO
Ready for code review?: NO