Notes for AI agents and human contributors working on Sitewire-Web. Keep this file short and load-bearing — anything that's just useful background lives in code comments, PRs, or other docs.
Commits should be small, reviewable chunks of code.
Commit messages should be a single line of imperative text, like "Add foo to enable bar".
Only humans get attribution in commit messages -- NEVER add Claude as a co-author.
Every commit must be authored AND committed as Liv Carman <olivia.carman@gmail.com> so it
attributes to her GitHub account. Do not edit git config to achieve this. Instead set the
identity per commit:
GIT_COMMITTER_NAME="Liv Carman" GIT_COMMITTER_EMAIL="olivia.carman@gmail.com" \
git commit --author="Liv Carman <olivia.carman@gmail.com>" -m "..."
After committing, verify with git log -1 --format='%an <%ae> / %cn <%ce>' that both the author
and committer are olivia.carman@gmail.com.
Comments should be brief and explain the "why" not the "what" unless the "what" is confusing (and if the "What" is confusing, that's a code smell).
Do not write backwards-looking comments. A comment's job is to explain why the code IS the way it is, not why it ISN'T some other way. Phrases like "Previously...", "Earlier...", "An earlier X was overly broad", "Unlike before...", "This used to..." all anchor the reader to a version of the code that doesn't exist anywhere they can see. The git log and the PR / commit message are where that history lives.
Allowed exceptions, narrow: a comment may reference prior behavior when the reader has no other way to understand the current code — e.g., the rationale for a non-obvious migration step where the old shape constrains the new one, or a workaround for a third-party-system quirk that doesn't show up in this repo. If you're tempted to write one of these, first ask whether the code itself can be made self-evident; reach for backwards-looking prose only when it can't.
This rule applies even more strictly when the "earlier" behavior was on an unshipped branch (yours or someone else's): in that case, there is no "before" — the buggy version never existed in any deployed environment, so naming it in a comment just confuses future readers.
Do not write comments documenting the absence of code. "No X here because Y", "We intentionally don't Z" and similar prose explain why something ISN'T there — a reader who isn't carrying the PR history in their head has no reason to wonder about a callback / branch / method that doesn't exist, so the comment just plants the question it then has to answer. If the absence is genuinely surprising, that's usually a sign the code is wrong; if it's not surprising, the comment is noise. Same anti-pattern as backwards-looking comments, sneakier shape: it references an imagined alternative version of the file rather than a past one. Document why the present code IS the way it is, in the place where the present code lives (e.g., on the method that owns the responsibility, not in a void where the alternative would have gone).
Do not reference review/conversation artifacts in comments. Phrases like "the open question", "per the review comment", "as discussed", "addresses the feedback", "TODO from PR #123" point at a thread, ticket, or chat that lives outside the code — a reader looking at the file has no way to resolve what they name. The same goes for naming a reviewer or an LLM's finding. State the actual fact directly: instead of "the open question: a forged kid must not starve a refetch", write "a forged kid must not starve a refetch for a real rotation". The reason the constraint exists belongs in the comment; the place you first heard it argued does not.
Before reporting a task complete, run the relevant linters and test suites for the code you touched. Don't claim "tests pass" without having actually run them, and don't skip a tier because the change "feels small" — type errors and lint violations from one-line changes block PRs the same as bigger ones.
- Ruby:
bundle exec standardrb(auto-fix with--fix) for any.rbchange. - Ruby specs:
bundle exec rspec <path>for the touched spec(s);bundle exec rspecfor broader changes that span services / models. - JS/TS:
yarn testruns the full Jest suite. Run it for any.ts/.tsxchange, not just files with co-located tests — a type tweak can break unrelated suites. - TypeScript types: any new prop / type field needs a clean
tsc(or a passingyarn test, since ts-jest type-checks on compile). - Formatting: two formatters split the front-end code and must not be crossed —
Prettier owns
app/javascript/react, while Biome owns the paths listed in.prettierignore(app/javascript/{helpers,controllers},react/components, thereact/views/ChangeOrderviews, andapp/assets/stylesheets/**/*.css). CI runsyarn format:check, which runs both — so check both before declaring done.yarn formatauto-fixes both; narrow it withyarn format:react(Prettier only) oryarn format:fix:biome(Biome only). Never reformat a Biome-owned file with Prettier or vice-versa — they disagree on tabs vs spaces and semicolons and will fight.
If yarn test prints a Warning: An update to <Component> inside a test was not wrapped in act(...), treat it as a real failure and fix it before moving on. Do
not:
- Suppress it with
jest.spyOn(console, "error"). - Defer it with a TODO / "we'll fix it later".
- Dismiss it because the test still passes — it almost always points at a state update happening outside the test's awaited control flow, which is the same shape of bug that produces the worst kind of cross-test flake (passes in isolation, fails in CI under load).
The fix is usually to wrap the triggering call in await act(async () => { ... })
so React flushes the re-render before the next assertion. If a polling /
timer-driven component is involved, also reset timers + mocks in an afterEach
so a still-resolving promise from one test can't bleed into the next.
While a branch is unmerged and unreleased, editing migrations in place is fine — drop the
affected tables, clear schema_migrations rows, and re-run db:migrate to make the live DB
match the edited code. Useful during rapid iteration on a new feature.
Once a branch ships, schema changes are additive migrations only. Never edit a historical migration that's been applied in any deployed environment. To change something already in the schema:
- New column?
add_columnmigration. - Column type / nullability?
change_columnmigration (or thechange_column_*helpers). - Drop a column?
remove_columnmigration. Reversible. - Rename? Often two migrations (add new, backfill, remove old) to avoid downtime.
This applies to db/migrate/, db/access_logs_migrate/, and db/ach_authorizations_migrate/.
The structure dump (db/structure.sql) is regenerated by db:migrate; don't hand-edit it once
deployed either.
Always commit db/structure.sql alongside the migration that produced it — but only the
hunks your migration caused. Running db:migrate regenerates the full dump from current DB
state, which often includes pre-existing drift (other devs' migrations applied locally but not
yet committed, pg_dump \restrict nonces, view-rewrite cosmetic changes). Before staging,
diff the file and revert any hunk that isn't yours — typically you keep only the new
schema_migrations row and any actual schema lines the migration adds. git add -p is the
right tool here. The same rule applies to db/access_logs_structure.sql and
db/ach_authorizations_structure.sql.
Flipper flags live in the database (the flipper-active_record adapter), so a new flag a deploy
depends on must be created by a data migration — never assume it will be toggled by hand in the UI.
For every new Flipper.enabled?(:some_flag, ...) you introduce, add a migration that registers the
flag, default off, following the existing convention (e.g. db/migrate/*_add_*_feature_flag.rb):
class AddSomeFlagFeatureFlag < ActiveRecord::Migration[7.1]
FLAG = :some_flag
def up
Flipper.add(FLAG)
Flipper.disable(FLAG)
end
def down
Flipper.remove(FLAG)
end
endRamp/rollback is then done through the Flipper UI / console (Flipper.enable_percentage_of_actors,
Flipper.enable_actor, Flipper.disable); the migration only guarantees the flag exists and starts
off. Removing a flag from the code gets the mirror-image remove migration. The schema_migrations
hunk these produce in db/structure.sql follows the same commit-only-your-hunks rule above.