Skip to content

Add coding standards & principles section to AGENTS.md#3

Merged
don-petry merged 2 commits intomainfrom
chore/coding-standards
Mar 28, 2026
Merged

Add coding standards & principles section to AGENTS.md#3
don-petry merged 2 commits intomainfrom
chore/coding-standards

Conversation

@don-petry
Copy link
Copy Markdown
Contributor

@don-petry don-petry commented Mar 28, 2026

Summary

  • Adds a new Coding Standards & Principles section to AGENTS.md covering SOLID, CLEAN Code, DRY, DDD, KISS, YAGNI, defensive coding at boundaries, separation of concerns, and code organisation
  • Each principle includes actionable rules with practical guidance so agents (and humans) can apply them consistently across all org repos
  • Drawn from patterns already proven and enforced in TalkTerm (CLAUDE.md) and Markets (AGENTS.md)

What's included

Principle Key points
SOLID Table format with rule + practical meaning for each principle
CLEAN Code Meaningful names, small functions, minimal args, no side-effect surprises, error handling
DRY Knowledge duplication vs code duplication; rule of three before abstracting
DDD Ubiquitous language, bounded contexts, aggregates, value objects, repository pattern
KISS Simplest solution first, no clever code
YAGNI No speculative features or abstractions
Defensive coding Validate at boundaries, trust internal code, fail fast
Separation of concerns Layered architecture, inward dependencies, no framework bleed
Code organisation Co-location, no barrel files, consistent naming

Test plan

  • Review that principles are language/framework-agnostic
  • Verify no conflicts with existing TDD, Pre-Commit, or Code Style sections
  • Confirm repo-level override clause is clear
  • Check markdown renders correctly on GitHub

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added a comprehensive "Coding Standards & Principles" section documenting recommended engineering practices: SOLID principles, CLEAN code guidance (naming, function sizing, arguments, formatting, error handling), DRY guidance, DDD concepts, KISS and YAGNI, defensive coding at boundaries, separation of concerns, and code organization conventions.

…AGNI

Establishes org-wide coding principles in AGENTS.md that all repos must
follow. Drawn from patterns already proven in TalkTerm and Markets. Each
principle includes actionable rules so agents can apply them consistently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 28, 2026 21:07
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3e0fcf69-cae8-42cf-bdcb-036607a6fb08

📥 Commits

Reviewing files that changed from the base of the PR and between d695ece and fe2d9ab.

📒 Files selected for processing (1)
  • AGENTS.md

📝 Walkthrough

Walkthrough

Added a new repository-level "Coding Standards & Principles" section to AGENTS.md documenting SOLID, CLEAN code, DRY, DDD, KISS, YAGNI, defensive coding at boundaries, separation of concerns, and code organization rules.

Changes

Cohort / File(s) Summary
Documentation
AGENTS.md
Added "Coding Standards & Principles" section with prescriptive guidelines covering SOLID (SRP, OCP, LSP, ISP, DIP), CLEAN code (naming, function sizing, args, command/query separation, formatting, errors), DRY guidance, DDD concepts, KISS, YAGNI, defensive boundary rules, separation of concerns, and code organization practices.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding a coding standards & principles section to AGENTS.md. It is specific, clear, and directly reflects the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/coding-standards

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an organization-wide “Coding Standards & Principles” section to AGENTS.md so all repos share consistent, actionable guidance on core engineering principles, with a clear override rule for repo-specific standards.

Changes:

  • Introduces a new “Coding Standards & Principles” section covering SOLID, Clean Code, DRY, DDD, KISS, YAGNI, defensive coding at boundaries, separation of concerns, and code organization.
  • Provides practical, rule-oriented guidance intended to be language/framework agnostic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread AGENTS.md Outdated
Comment thread AGENTS.md Outdated
Address Copilot review comments for American English consistency.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@don-petry don-petry merged commit b0ff212 into main Mar 28, 2026
3 checks passed
don-petry pushed a commit that referenced this pull request Apr 7, 2026
Two more fixes from CodeRabbit review:

1. Model selection via claude_args (the documented v1 interface)
   instead of ANTHROPIC_MODEL env var. claude_args takes precedence over
   the env var per the action's docs, so depending on the env var was
   relying on undocumented behavior. The pinned v1.0.89 happens to honor
   ANTHROPIC_MODEL too (verified in TalkTerm run #3 logs), but the
   documented path is more robust against future action upgrades.

2. Re-query existing Ideas discussions before each create. The signals
   snapshot only fetches the first page of discussions (GraphQL caps
   connections at 100 per page) and only covers the Ideas category, not
   the General fallback. Mary now does a fresh query before each create
   to avoid duplicates in repos with >100 idea threads or where Ideas
   doesn't exist.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
don-petry added a commit that referenced this pull request Apr 7, 2026
* feat: add Feature Ideation workflow as a standard for BMAD-enabled repos

Promotes the BMAD Analyst (Mary) feature ideation workflow piloted in
petry-projects/TalkTerm to an org-wide standard for any repo with BMAD
Method installed.

Adds:
- standards/workflows/feature-ideation.yml — the canonical template,
  generalised from TalkTerm. Customisation surface is a single
  PROJECT_CONTEXT env var that describes the project and its market.
- standards/ci-standards.md §8 rewrite — documents the multi-skill
  ideation pipeline (Market Research → Brainstorming → Party Mode →
  Adversarial), the Opus 4.6 model requirement, the github_token
  permissions gotcha, and the show_full_output secrets hazard.
- standards/agent-standards.md — adds a "BMAD Method Workflows"
  section linking the standard from the agent ecosystem docs.

The four critical gotchas baked into the template were each discovered
empirically during the TalkTerm pilot and would silently regress without
the inline comments. Most importantly: the action's auto-generated
claude[bot] App token lacks discussions:write, so the workflow MUST
pass github_token: ${{ secrets.GITHUB_TOKEN }} explicitly or every
Discussion mutation fails silently while the run reports success.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: split feature-ideation into reusable workflow + thin caller stub

Avoids ~600 lines of prompt duplication across every BMAD-enabled repo and
makes the multi-skill ideation pipeline tunable in one place — changes here
propagate to every adopter on next scheduled run.

- .github/workflows/feature-ideation-reusable.yml — the actual reusable
  workflow (workflow_call). Contains both jobs (signal collection +
  analyst), the full Phase 1-8 prompt, and the four critical gotchas
  (Opus 4.6 model, github_token override, no show_full_output, structural
  Phase 2-5 sequence) hard-coded so they cannot regress.
- standards/workflows/feature-ideation.yml — replaced the 600-line copy
  with a ~60-line caller stub that only defines the schedule, the
  workflow_dispatch inputs, and a single required parameter:
  project_context.
- standards/ci-standards.md §8 — documents the reusable + caller stub
  architecture, the inputs/secrets contract, and updated adoption steps.
  Reference implementation pointer updated to note that TalkTerm is now
  also a thin caller stub.

Inputs exposed by the reusable workflow:
- project_context (required) — project description for Mary
- focus_area (default '') — typically wired to workflow_dispatch
- research_depth (default 'standard')
- model (default 'claude-opus-4-6') — escape hatch only
- timeout_minutes (default 60)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(lint): add shellcheck disable for GraphQL variable false positive

The gh api graphql queries use $repo / $owner / $categoryId as GraphQL
variables (not shell expansions), which must remain in single quotes.
shellcheck SC2016 fires anyway — disable it for this script.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(lint): use quoted heredocs for GraphQL queries to satisfy SC2016

actionlint runs shellcheck on the entire run script as one unit and ignores
inline disable directives. Rewriting the gh api graphql calls to use
cat <<'GRAPHQL' heredocs makes the GraphQL variable references ($repo,
$owner, $categoryId) shell-inert without depending on single-quoted
string literals — eliminating the SC2016 false positive.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: expand prompt variables via Actions expressions, add placeholder guard

CodeRabbit caught a critical latent bug inherited from the original TalkTerm
prompt: shell-style $VAR and $(date) syntax inside the action's `prompt:`
input is NOT expanded — the action receives literal text. This silently
broke variable substitution in every prior run, but mattered most for the
new reusable workflow because PROJECT_CONTEXT is now load-bearing.

Changes:
- Replace $PROJECT_CONTEXT, $FOCUS_AREA, $RESEARCH_DEPTH, and $(date ...)
  with ${{ inputs.* }} and ${{ github.run_started_at }} expressions, which
  ARE evaluated by GitHub before passing the prompt to the action.
- Add a "Validate project_context is customised" pre-step that fails fast
  if an adopter copied the caller stub without replacing the TODO
  placeholder. Prevents wasted Opus runs producing generic Discussions.
- scripts/compliance-audit.sh: detect BMAD repos via `_bmad-output/` as
  well as `_bmad/`, matching the broader detection rule documented in
  ci-standards.md §8 (TalkTerm only has `_bmad-output/`).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(lint): drop github.run_started_at (not in actionlint context schema)

The agent can read scan_date from signals.json instead — added a hint
in the Environment section.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(caller): grant cascading permissions on the calling job

CodeRabbit caught: the caller stub had `permissions: {}` at workflow
level and no permissions block on the calling job. Reusable workflows
inherit permissions from the calling job — without an explicit grant,
the reusable workflow's `discussions: write` declaration would have
nothing to apply, and Discussion mutations would fail with FORBIDDEN
just like the original bug we fixed in TalkTerm.

The reusable workflow's job-level permissions are documentation of
what it needs; the caller is what actually grants them.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use claude_args --model interface; instruct re-query before create

Two more fixes from CodeRabbit review:

1. Model selection via claude_args (the documented v1 interface)
   instead of ANTHROPIC_MODEL env var. claude_args takes precedence over
   the env var per the action's docs, so depending on the env var was
   relying on undocumented behavior. The pinned v1.0.89 happens to honor
   ANTHROPIC_MODEL too (verified in TalkTerm run #3 logs), but the
   documented path is more robust against future action upgrades.

2. Re-query existing Ideas discussions before each create. The signals
   snapshot only fetches the first page of discussions (GraphQL caps
   connections at 100 per page) and only covers the Ideas category, not
   the General fallback. Mary now does a fresh query before each create
   to avoid duplicates in repos with >100 idea threads or where Ideas
   doesn't exist.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: DJ <dj@Rachels-MacBook-Air.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

2 participants