---
name: Feedback
description: Corrections and preferences to apply consistently
type: feedback
---

## Squash Merges Only
Always use `git merge --squash` — never regular merge.
**Why:** Clean main history.

## Terse Responses
Don't summarize what was just done — user can read the diff.

## Ask Before Committing
Only commit when explicitly asked.

## Use Sub-Agents for Non-Blocking Work
Leverage background sub-agents for API calls, commits, builds, searches. Never block the main session.

## Always Verify Changes Before Declaring Done
After any code change: analyze → grep for related occurrences → provide test steps. Never skip verification.

## Use BaseController + BasePageView
Every migrated screen must use `BaseController` + `BasePageView` — NOT `GetxController` + `StatelessWidget`, NOT `VariantBaseController/View`.
**Why:** User corrected when search screen used GetxController.

## Commit Per Migration
Commit after every single screen migration for safe recovery.
**Why:** User said "small win by small win" and "commit after one, for safe recovery".

## Match Old Loading UX
Check old implementation's loading behavior before migrating. If old code had custom buildLoadingView shimmer, port it. If screen should be instant, set `pageState = PageState.success` immediately.
**Why:** User asked "why loading is different?" when search screen showed full-screen loading.

## Use colorPrint for Debug Traces
Use `colorPrint` (not raw `print`). Keep debug logs — they help trace issues later.
**Why:** User said "later one issue arise, it help to trace".

## No Background Magic
UX goal: auto fetch only on app start, then manual. No periodic polling, no WidgetsBindingObserver, no auto-reload modals.
**Why:** User's explicit design philosophy for the refactored landing page.

## Memory Files in Repo
Store memory files in `.claude/memory/` inside the repo (git-tracked), NOT in the local `~/.claude/projects/` directory.
**Why:** User corrected when files were saved locally instead of in the repo.

## Don't Revert Good Decisions
If an optimization is correct (like dynamic social controllers from AppConfigModel instead of 15 hardcoded ones), don't talk yourself out of it when questioned. Investigate whether it actually works before offering to revert.
**Why:** Dynamic social controllers were the right call — extensible, server-driven. User confirmed it worked.

## Track Commit Hashes When Deleting Old Files
Record the last commit hash that still contained the old files in session_status.md before deleting them.
**Why:** Safety net — if we deleted something important, we know which commit to recover from.
**How to apply:** Before deleting old files, note the current commit hash in session_status.md next to that screen.

## Study Existing Patterns Before Writing Code
When migrating to an established pattern (e.g., BaseCardFactory), read 2-3 existing implementations FIRST — directory structure, naming, class hierarchy — before writing a single line. Don't assume, verify.
**Why:** Space card migration was rewritten twice (wrong base class pattern, wrong directory naming) because the existing pattern wasn't studied thoroughly upfront.
**How to apply:** Before any card view migration, check an existing migrated card's full structure (factory, base, design_1/, models/).

## Debug at the Right Layer First
When data is wrong, add a definitive debug print at the data boundary (where data enters/leaves a system) immediately — don't speculate layer by layer.
**Why:** Hive contamination took 6 rounds to find because debug prints were added incrementally instead of at the data source boundary first.
**How to apply:** For data issues: print at the source (Hive/API), then at the consumer (card factory). Compare. The mismatch tells you where to look.

## Verify Dependencies Before Using Them
Before using a value (like `ApiConfig.instance.baseUrl`), check if it's initialized at the point of use. Read the class to see defaults and initialization timing.
**Why:** `noImageUrl` getter used `baseUrl` (empty until `rebuildBaseUrl()`) instead of `domain` (always has default). Needed a second fix round.

## No Domain Layer
This is an SDUI app — server drives the UI. No need for domain/ layer (entities, repositories, use cases). Business logic lives server-side. Already discussed and decided.
**Why:** SDUI pattern means app is a renderer, not a logic owner.

## Memory Files in Repo — Enforced
Always write to `.claude/memory/` inside the repo. NEVER write to `~/.claude/projects/`. This has been corrected multiple times.
**Why:** Repo-local memory is git-tracked and shared across machines. Local `~/.claude/` is per-machine and invisible to other sessions.
**How to apply:** Before any Write to a memory file, check the path starts with the project root, not `~/.claude/`.

## Refresh, Don't Interpret — Action-Layer Recovery Pattern
After an API failure, an action layer may **passively refresh** local entity state from the server (max one extra fetch — `_fetchFreshSpace`-style), but must **never interpret** a rejection as success. The API's verdict stands. The UI renders whatever's actually true via `current = state.data ?? entity` plus the snackbar.
**Why:** User explicitly rejected the "if 422 + 'already a member', auto-treat as success and navigate" pattern as "extra interpretation our self". They want UI to honor what the API says, not what we wish it said.
**How to apply:** When designing recovery for any action layer (`SpaceActions`, future `MemberActions`, etc.): the maximum extra step is one passive `_refreshEntity({slug, error})`-style helper that fetches fresh data and writes it into `state(data: fresh, error: <api error>)`. Never auto-navigate. Never override an API failure with a fake success state. Reference: `space_actions.dart` `_refreshEntity` for the canonical pattern.

## Don't Parse Server Message Strings for Control Flow
Never use `.contains('some english phrase')` on a server message to drive client behavior. Use HTTP status codes (4xx vs 5xx) or structured error fields (`error_code`).
**Why:** Brittle to wording, locale, backend changes. User flagged `_looksLikeAlreadyMember(message)` as fragile and demanded its removal.
**How to apply:** For "is this error type X?" decisions, check `e.response?.statusCode`. If a structured `error_code` field exists, use it. If neither is available, refresh state and let the data decide via the UI's natural rendering — don't parse prose.

## Confirm Before Cross-Cutting Edits on Open-Ended Suggestions
When the user makes an open-ended suggestion ("can we do X?", "I guess we could Y"), confirm the EXACT files/hooks before editing. Do NOT extrapolate "do X" to mean "do X in every applicable place".
**Why:** After "i guess we can clear all of these state on landing page?", I edited BOTH old and new landing controllers and used `handleUserLogoutEvent`. User said "you are doing wrong" — they meant ONE specific hook (`onInit` of the OLD `landing_controller.dart`) only. I had to revert and re-apply.
**How to apply:** Re-state the interpretation with exact file paths and exact hook/method names. Ask "is this what you meant?" before editing. Especially critical for: lifecycle hooks, controller bases, multi-controller wiring, importing across architectural boundaries (e.g., production code importing from `_example_*` folders).

## Card Action State — Cleared on Landing onInit (Old Controller Only)
`clearAllCardActionStates()` lives in `register_all_cards.dart` and is called from `landing_controller.dart` (old, in `presentation/features/landing_page/`) `onInit()`. NOT in `handleUserLogoutEvent`. NOT in `new_landing_controller.dart`.
**Why:** User explicitly chose `onInit` of the OLD landing controller as the single fresh-start lifecycle hook. It catches both logout-relogin AND login-without-logout (when LandingController is recreated). The user did not want both controllers wired — only the old one.
**How to apply:** When adding new action singletons (e.g., `MemberActions`, `FeedActions`), append their `clearAllStates()` call to `clearAllCardActionStates()` in `register_all_cards.dart`. Do NOT add new call sites in other controllers without explicit user direction.

## Button Stays Visible on Non-Retryable Errors
In design widgets, the dedicated error UI (`_buildErrorButton` etc.) renders **only** when `actionState.hasError && !actionState.hasData && actionState.error!.shouldRetry`. For non-retryable errors (auth, business, parse, unknown), keep the normal action button visible — the snackbar already communicates the failure.
**Why:** Replacing the button with red text strands the user with no way to interact (the original "button despire" bug). The snackbar carries the error message; the button doesn't need to vanish too.
**How to apply:** When writing a new card design widget, copy the conditional pattern from `space_card_views_new/design_1/vertical.dart`:
```dart
if (actionState.isLoading)
  _buildLoadingButton()
else if (actionState.hasError && !actionState.hasData && actionState.error!.shouldRetry)
  _buildErrorButton(actionState.error!)
else
  _buildButton(current),
```

## Never Download PNG/Image Attachments From LazyTasks
Do NOT download PNG (or any image) attachments from LazyTasks under any circumstances — not via curl, not via WebFetch, not via a sub-agent. Skip them entirely.
**Why:** Empirically, downloading a LazyTask image attachment breaks the session — Claude gets so badly lost it stops responding altogether. This has happened repeatedly; treat it as a hard "do not touch" rule, not a preference.
**How to apply:** When `tasks/show/${TASK_ID}` returns an `attachments` array containing images, ignore them. Do not list URLs in a way that might tempt a follow-up download. If task context clearly requires a screenshot, ask the user to paste/attach it directly in chat instead — chat-attached images are safe; LazyTask-hosted ones are not.

## Dart 3 `||` in Switch Cases IS a Valid Pattern
`case 'A' || 'B':` in a Dart 3 switch case IS a logical-or pattern that matches either constant. Verified empirically on Dart SDK 3.11.4. Do NOT claim this is a fall-through bug — it's not.
**Why:** A draft proposal claimed this was a runtime bug. Empirical test (`fvm dart run` on a minimal repro) showed both constants matched correctly. Don't repeat the false claim in reviews.
**How to apply:** When reviewing switch cases that use `||`, verify behavior empirically before flagging as a bug. The Dart 3 pattern syntax is valid.