chore(sdd): bootstrap SDD governance and component conventions - #106
karenkrieger wants to merge 1 commit into
Conversation
Adopts the VTEX SDD Lite model in this repository. Documentation, governance, and agent tooling only — no component source, build config, or workflow was touched. - .specify/memory/constitution.md (v1.0.0): 7 principles derived from conventions already present in the repo — public API as contract, one component per folder, brand-ui styling, type safety and lint, Storybook as the component spec, localization, and the committed dist/ artifact. Plus technology stack, architectural boundaries, project structure, and quality gates. - AGENTS.md and CLAUDE.md: byte-identical agent guides (178 lines) covering overview, sources of truth, stack, prerequisites, build/run, verification, boundaries, conventions, workflow, and guardrails. - .agents/skills/specification and .agents/skills/implementing: vendored verbatim from vtex/vtex-agent-skills (v1.2.0 / v1.2.1). specification writes specs/<feature>.md; implementing consumes an approved one. - .agents/skills/component-authoring: repo-specific component best practices — 9-step authoring flow, canonical file templates, and a pre-review checklist. - specs/README.md: spec lifecycle plus the repo-specific requirements a spec must declare (public API impact, consumer impact, placement, i18n keys, and which stories prove each acceptance criterion). - .gitignore: ignore a local clone of the full agent skills catalog. Three open decisions are recorded as TODO(team) markers in the constitution rather than invented as rules: automated verification (the lint baseline is red at 930 pre-existing errors, CI runs no quality gate, and there is no test runner), Storybook coverage (32 stories for 48 component folders, and build-storybook is broken), and the undocumented components/ vs lib/ criterion.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
mariana-caetano
left a comment
mariana-caetano
left a comment
There was a problem hiding this comment.
Seria interessante adicionar uma seção no README.md do repositório explicando que esse repo. usa SDD, algo como contribuir com um novo componente. Por exemplo:
### Spec-Driven Development
This repository uses Spec-Driven Development (SDD Lite). Before implementing a new component, create a specification that describes:
- The problem and expected outcome.
- The component's public API.
- Architectural decisions.
- Acceptance criteria.
- Localization and Storybook requirements.
- The impact on consuming repositories.
The workflow is:
1. ...
2. ...
...| @@ -0,0 +1,91 @@ | |||
| # Pre-review checklist | |||
|
|
|||
There was a problem hiding this comment.
O que acha de incluirmos uma seção sobre acessibilidade? Posso pensar em alguns itens para o checklist. Por exemplo:
[ ] Relevant accessibility states and interactions are covered in Storybook.
[ ] Interactive elements have accessible names.
There was a problem hiding this comment.
(cf. .agents/skills/specification/references/template.md › "Non-Functional Requirements: … accessibility constraints")
+1 to @mariana-caetano's comment. The spec template asks for accessibility constraints, but nothing downstream verifies them — the checklist has none, and Storybook's a11y addon is not in .storybook/main.ts. A minimal section that fits the existing "Every box maps to a Quality Gate" framing:
## Accessibility
- [ ] Interactive elements are reachable and operable by keyboard, with visible focus.
- [ ] Images and icon-only controls have localized `alt` / `aria-label` (see Localization).
- [ ] Text and UI colors meet WCAG AA contrast at the `.storybook/preview.ts` viewports.
- [ ] Stories were checked with the Storybook `a11y` addon (or: the addon is not installed — flag in the PR).Adding @storybook/addon-a11y would be a separate, infrastructure-only PR per the "either behavior or infrastructure" rule.
🤖 AI-generated
mariananantua
left a comment
mariananantua
left a comment
There was a problem hiding this comment.
Per the rule this PR introduces (Step 6 / checklist: "Docs, tooling, governance only → release-no"), this PR should carry release-no, not release-patch. As labeled, it will cut a patch release with no change in dist/. Worth fixing before merge so the first governance PR follows its own rule.
| ```tsx | ||
| import { Box, Flex, Text } from '@vtex/brand-ui' | ||
|
|
||
| import type { ContributorsType } from 'lib/contributors' |
There was a problem hiding this comment.
This example contradicts the layering rule stated in Step 0 ("A components/* file MUST NOT import from lib/*"), the constitution's Architectural Boundaries table, and the checklist ("Nothing in src/components/ imports from src/lib/"). The constitution only tolerates type-only imports for the utils → lib exception.
Either (a) generalize the tolerance — "type-only imports across layer boundaries are tolerated; value imports are not" — in the constitution table, SKILL Step 0 and the checklist, or (b) change the example so Author takes a locally-defined props shape. I'd lean to (a), since Author today really does import that type.
🤖 AI-generated
| - Run `yarn storybook` and look at the component across the declared viewports — | ||
| 360px, 640px, 832px, 1024px, 1280px, 1920px, 2560px — before requesting review. | ||
|
|
||
| ## Step 6 — Export from the barrel |
There was a problem hiding this comment.
(also references/checklist.md › "Public API & release", and specs/README.md › "Public API impact")
The set of release labels differs across files:
- constitution (Principle VII) and AGENTS.md:
release-no | release-auto | release-patch | release-minor | release-major - this table and the checklist: 4 labels, no
release-auto specs/README.md: 3 labels, norelease-no
Suggest making the constitution the canonical list and having the other three reference it (or at least match it). If release-auto is intentionally discouraged for humans, say so explicitly instead of omitting it.
🤖 AI-generated
| hand-roll what the design system provides. | ||
| - **Alias imports**, not relative traversal: `components/tag`, `lib/contributors`, | ||
| `utils/typings/types` (`baseUrl: src`). Only `./styles`, `./functions`, and | ||
| `./<Component>.types` stay relative. |
There was a problem hiding this comment.
Three files state this rule three different ways:
- here:
./styles,./functions,./<Component>.types - AGENTS.md › Coding conventions › Imports, and constitution Principle II: only
./stylesand./functions - meanwhile stories import
./index, and private sibling subcomponents (header/dropdown-menu.tsx,feedback-modal/modal.tsx) are necessarily relative too.
Suggest one formulation everywhere: "Imports within the component's own folder are relative; everything outside it uses baseUrl: src aliases." Then the enumerated list becomes unnecessary.
🤖 AI-generated
| Rationale: The catalogs and the `LibraryContext` locale already exist and are used by | ||
| ten-plus components. Bypassing them ships English into the Brazilian and Spanish portals. | ||
|
|
||
| ### VII. The Built Artifact Ships With the Source |
There was a problem hiding this comment.
("Conventional Commits (feat:, fix:, chore:, styles: …)" — same in SKILL Step 8 and AGENTS.md › Commits)
styles: is not a Conventional Commits type — the spec uses style:. If styles: reflects actual history in this repo, fine, but let's confirm. Also note that neither type triggers a version bump in standard-version, so it's worth saying which types actually drive the release (feat → minor, fix → patch, BREAKING CHANGE → major), since the label is what really decides.
🤖 AI-generated
| --- | ||
| name: component-authoring | ||
| description: Create, change, or review a component in the @vtexdocs/components library. Use when the user mentions adding a component, editing a component, a new story, styles.ts, the src/index.ts barrel, brand-ui SxStyleProp, localizing a string, or asks for component best practices, conventions, or a component review in this repository. | ||
| license: MIT |
There was a problem hiding this comment.
Can we confirm license: MIT matches the repository's own license? If the repo has no LICENSE file or uses a different one, the skill frontmatter shouldn't claim MIT.
🤖 AI-generated
| - **NEVER** bulk-format the repository or run `eslint --fix` across `src/`. | ||
| - **NEVER** change `.github/workflows/` as a side effect of a component change. | ||
| - **NEVER** mix infrastructure and behavior in one pull request. | ||
| - `AGENTS.md` and `CLAUDE.md` are byte-identical mirrors. Edit both, or neither. |
There was a problem hiding this comment.
A 178-line byte-identical copy guarded only by a comment will drift. Claude Code resolves @path imports in CLAUDE.md, so this file can be a single line:
@AGENTS.mdIf you prefer to keep a full copy for tools that don't support imports, a symlink (ln -s AGENTS.md CLAUDE.md) achieves the same with zero maintenance. Either way the "edit both" guardrail can go.
🤖 AI-generated
| @@ -0,0 +1,178 @@ | |||
| # Agent guide — `@vtexdocs/components` | |||
There was a problem hiding this comment.
A 178-line byte-identical copy guarded only by a comment will drift. Claude Code resolves @path imports in CLAUDE.md, so this file can be a single line:
@AGENTS.mdIf you prefer to keep a full copy for tools that don't support imports, a symlink (ln -s AGENTS.md CLAUDE.md) achieves the same with zero maintenance. Either way the "edit both" guardrail can go.
🤖 AI-generated
| suppression MUST carry a comment justifying it and MUST be raised in the PR. | ||
| - Formatting is `@vtex/prettier-config` via `prettier/prettier: error`. `yarn lint` MUST | ||
| report **zero errors in the files a change touches**. | ||
| - The repository-wide baseline is **not** clean: `yarn lint` currently reports 930 errors |
There was a problem hiding this comment.
(same figures in Principle II "48 component folders", "42 of them today"; Principle III "44 styles.ts files"; Principle V TODO "17 of 48"; AGENTS.md › Build & run; component-authoring/SKILL.md Step 7 and Anti-patterns; references/checklist.md › Type safety & lint)
These counts are accurate today and wrong after the next merged PR, and they are repeated in six places, so they will go stale unevenly. Suggest keeping exact figures only inside the TODO(team) comments (where they are a dated snapshot of the decision to be made) and rewriting the normative text without numbers — e.g. "the repository-wide lint baseline is red; the gate is zero errors in the files you touch". Where a number is useful, give the command that produces it (yarn lint 2>&1 | grep -c 'error') instead of the value.
🤖 AI-generated
|
|
||
| Rules: | ||
|
|
||
| - `title: 'Example/<ComponentName>'`, `tags: ['autodocs']` (required — `docs.autodocs` |
There was a problem hiding this comment.
(also constitution Principle V and references/checklist.md › Storybook)
Example/ is the placeholder prefix from the Storybook boilerplate, not a taxonomy. Codifying it as a MUST means every future story is required to keep a placeholder. Since this PR is the moment conventions get written down, suggest either deciding a real hierarchy now (Components/<Name>, Lib/<Name>, Icons/<Name> maps directly onto the layering in Step 0) or downgrading this to "match the existing Example/ prefix" with a TODO(team) to pick a taxonomy before the story backfill in Principle V's TODO.
🤖 AI-generated
| @@ -0,0 +1,91 @@ | |||
| # Pre-review checklist | |||
|
|
|||
There was a problem hiding this comment.
(cf. .agents/skills/specification/references/template.md › "Non-Functional Requirements: … accessibility constraints")
+1 to @mariana-caetano's comment. The spec template asks for accessibility constraints, but nothing downstream verifies them — the checklist has none, and Storybook's a11y addon is not in .storybook/main.ts. A minimal section that fits the existing "Every box maps to a Quality Gate" framing:
## Accessibility
- [ ] Interactive elements are reachable and operable by keyboard, with visible focus.
- [ ] Images and icon-only controls have localized `alt` / `aria-label` (see Localization).
- [ ] Text and UI colors meet WCAG AA contrast at the `.storybook/preview.ts` viewports.
- [ ] Stories were checked with the Storybook `a11y` addon (or: the addon is not installed — flag in the PR).Adding @storybook/addon-a11y would be a separate, infrastructure-only PR per the "either behavior or infrastructure" rule.
🤖 AI-generated
3 participants
Adopts the VTEX SDD Lite model in this repository. Documentation, governance, and agent tooling only — no component source, build config, or workflow was touched.
Three open decisions are recorded as TODO(team) markers in the constitution rather than invented as rules: automated verification (the lint baseline is red at 930 pre-existing errors, CI runs no quality gate, and there is no test runner), Storybook coverage (32 stories for 48 component folders, and build-storybook is broken), and the undocumented components/ vs lib/ criterion.