External/5chan
1
0
Fork 0
mirror of https://github.com/bitsocialnet/5chan.git synced 2026-08-03 07:41:04 +02:00
Code Issues Packages Projects Releases Wiki Activity
Files
0493492f55b626d64eb1ba39ce98a272a5c7fd32
5chan/docs/agent-playbooks/known-surprises.md
T
Tommaso CasaburiandGitHub 0493492f55 fix(react-doctor): correct test exclusion + React-Compiler lint policy + state-sync fix (#1155) 
* fix(react-doctor): correctly exclude test files from scoring

The intended test-file ignore in react-doctor.config.json was never
applied: react-doctor's config precedence reads the "reactDoctor" key
in package.json (which had no ignore), shadowing the config file. On
top of that, react-doctor 0.4.0's ignore.files matcher is broken — any
non-empty value collapses scan scope and drops real product files, not
just tests.

Consolidate to a single canonical doctor.config.json using
ignore.overrides (which works correctly): only test files are excluded
while all product code is still scored. Remove the shadowing
package.json key and the dead react-doctor.config.json.

Product-code baseline is 55 (92 errors, 515 warnings, 118 files).

* chore(react-doctor): add long-running task tracking for score effort

* refactor(react): remove compiler-redundant memoization in verified files

Delete manual useMemo/useCallback/memo that the React Compiler already
handles, in 7 files validated to be behavior-preserving (factories are
pure functions of compiler-trackable reactive inputs). Kept memos whose
factories read external mutable DOM/theme state with load-bearing deps
(e.g. use-reply-height-estimates metrics). Also hoists a regex and reads
a localStorage value once.

Note: this is code-quality cleanup; react-doctor's score is error-
weighted, so warning cleanup like this does not move the score. See
docs/agent-runs/react-doctor-score/progress.md.

* fix(react-doctor): adopt React-Compiler lint policy + fix one state-sync bug

react-doctor's score is dominated by React-Compiler optimizability
diagnostics that flag intentional patterns (the latest-ref idiom) and
current compiler limitations (try/finally, throw-in-try/catch the
compiler can't lower yet), not bugs. Rewriting that working code to
satisfy them would degrade it.

- Replace doctor.config.json with a documented doctor.config.jsonc that
  does not enforce the react-hooks-js (React Compiler) rules or
  react-compiler-no-manual-memoization. All real code-quality, a11y, and
  performance rules stay enforced.
- Fix one genuine state-sync bug: use-now-seconds refreshed 'now' via a
  synchronous setState inside an effect (an extra render with a stale
  value); move it to a render-time prev-prop comparison (React's
  adjust-during-render pattern), behavior-equivalent.

Score 54 (broken config) -> 63. type-check/lint/1051 tests pass; browser
smoke confirms timestamps render with no re-render regression. The
remaining no-adjust-state-on-prop-change diagnostics are real bugs but
entangled with legitimate side effects (navigate/ref-cancel/async) in
critical flows; left for careful follow-up.

* chore(react-doctor): remove the vanity score badge, keep PR-diff review

The single 0-100 react-doctor score mostly reflects React-Compiler
optimizability and isn't a meaningful health grade to display (see
docs/agent-runs/react-doctor-score). Remove the README badge and its now-
dead generation infra (CI write/upload/publish steps + the
write-react-doctor-badge.mjs script + doctor:badge package script).

Kept: react-doctor's actual value -- the PR step that runs
'yarn doctor --diff <base> --annotations' on pull requests touching React
files, surfacing newly-introduced issues inline. Coverage badge untouched.

* docs(react-doctor): document why the score is not a target to chase

Record the reasoning so future agents/contributors don't re-attempt to
grind the react-doctor score: it overwhelmingly reflects React-Compiler
optimizability (most 'errors' flag intentional patterns and current
compiler limitations, not bugs) and saturates on the fraction of clean
files, so ~63 is the honest ceiling and 90 only comes from disabling the
linter.

- Add a known-surprises entry with the full reasoning + mitigation.
- Reframe the AGENTS.md react-doctor verification line: it's a PR-diff
  reviewer for newly-introduced issues, not an aggregate score to raise.
2026-06-05 22:21:30 +07:00

140 lines
11 KiB
Markdown
Raw Blame History

# Known Surprises
This file tracks repository-specific confusion points that caused agent mistakes.
## Entry Criteria
Add an entry only if all are true:
- It is specific to this repository (not generic advice).
- It is likely to recur for future agents.
- It has a concrete mitigation that can be followed.
If uncertain, ask the developer before adding an entry.
## Entry Template
```md
### [Short title]
- **Date:** YYYY-MM-DD
- **Observed by:** agent name or contributor
- **Context:** where/when it happened
- **What was surprising:** concrete unexpected behavior
- **Impact:** what went wrong or could go wrong
- **Mitigation:** exact step future agents should take
- **Status:** confirmed | superseded
```
## Entries
### react-doctor score reflects React-Compiler coverage, not code health — do not chase it
- **Date:** 2026-06-05
- **Observed by:** Tommaso + Claude
- **Context:** Trying to raise the `yarn doctor` (react-doctor) score to 90 (PR #1155).
- **What was surprising:** The score is overwhelmingly driven by React-Compiler *optimizability* diagnostics, not code quality. Most of the ~92 "errors" are the `react-hooks-js` plugin flagging valid, idiomatic code the React Compiler (v1.0) cannot optimize *yet* — `refs` (the deliberate latest-ref idiom for a stable callback) and `todo` (`try/finally` and throw-in-`try/catch` the compiler can't lower). The score also saturates on the *fraction of files with zero diagnostics*: removing 150 warnings moved it +1; suppressing all 76 compiler-bailout errors reached only 63; only suppressing essentially every rule reaches 90.
- **Impact:** Agents/contributors can burn large effort (and risk real regressions) "fixing" the score by rewriting correct code into compiler-friendly-but-worse shapes, or by suppressing rules until the badge is meaningless. ~63 is the honest, no-regression ceiling.
- **Mitigation:** Do NOT treat the aggregate react-doctor score as a target to grind up (the README badge was removed for this reason). Use react-doctor as a PR-diff reviewer — `yarn doctor --diff <base> --annotations`, already wired in `.github/workflows/ci.yml` — to catch *newly introduced* issues. `doctor.config.jsonc` deliberately does not enforce the `react-hooks-js` rules or `react-compiler-no-manual-memoization` (intentional patterns / current compiler limits). Only fix genuine bugs (e.g. clean `no-adjust-state-on-prop-change` cases). Full reasoning: `docs/agent-runs/react-doctor-score/`.
- **Status:** confirmed
### Portless 0.11 reuses legacy proxy state unless the launcher forces HTTPS
- **Date:** 2026-04-28
- **Observed by:** Tommaso + Codex
- **Context:** Upgrading the normal `yarn start` flow from the old `http://5chan.localhost:1355` proxy URL to `https://5chan.localhost`.
- **What was surprising:** Even with `portless@0.11.1` installed, Portless reused the existing `~/.portless/proxy.port = 1355` HTTP proxy and printed the legacy `:1355` URL.
- **Impact:** Updating package versions and docs is not enough; `yarn start` can still advertise and use the old URL when a contributor has legacy Portless state running.
- **Mitigation:** Keep `scripts/start-dev.js` explicitly starting the Portless HTTPS proxy on port `443` before registering the app route, so the runtime flow migrates away from persisted `1355` state instead of inheriting it.
- **Status:** confirmed
### Android release signing still uses legacy keystore names
- **Date:** 2026-04-23
- **Observed by:** Codex
- **Context:** Cutting the `v0.8.0` release after the core terminology rename changed release workflow signing references.
- **What was surprising:** The tracked Android release keystore is still `android/plebbit.keystore`, and the available GitHub secret is still `PLEBBIT_REACT_KEYSTORE_PASSWORD`; changing only the workflow references to `bitsocial.keystore` / `BITSOCIAL_KEYSTORE_PASSWORD` made the Android release artifact fail at signing.
- **Impact:** Tag releases can pass the Android build and then fail before publishing because `apksigner` cannot find the keystore or password secret.
- **Mitigation:** Until the keystore migration is done, keep release APK signing pointed at `android/plebbit.keystore` and `PLEBBIT_REACT_KEYSTORE_PASSWORD`. If renaming, migrate the tracked file, workflow path, and GitHub secret in one change and verify with a tag release dry run or full release.
- **Status:** confirmed
### 5chan consumes a pinned hooks tarball instead of using the local hooks repo
- **Date:** 2026-04-15
- **Observed by:** Codex
- **Context:** Debugging strict `{name, publicKey}` community refs after upstream fixes landed in `bitsocial-react-hooks`
- **What was surprising:** 5chan does not consume the nearby `/Users/Tommaso/Desktop/bitsocial/bitsocial-react-hooks` checkout by default; `package.json` installs a pinned GitHub tarball of `@bitsocialnet/bitsocial-react-hooks`.
- **Impact:** Agents can wrongly assume local hooks source changes are already active in 5chan, or debug the wrong package build when the app is really running a tarball revision from GitHub.
- **Mitigation:** Before debugging hooks behavior from 5chan, check `package.json` to see whether the app points at a tarball commit or a local path. If you need fresh hooks behavior, update the tarball commit or temporarily switch 5chan to a local path intentionally.
- **Status:** confirmed
### Hooks source commits can land before the generated tarball payload
- **Date:** 2026-04-18
- **Observed by:** Codex
- **Context:** 5chan CI failed after `expandTimeWindow` landed in `bitsocial-react-hooks` because the app was pinned to the feature source commit.
- **What was surprising:** `bitsocial-react-hooks` uses `dist/` as its published entrypoint, and the repo's CI writes that generated payload in a follow-up `chore(ci): update dist and coverage badge` commit after the source commit lands on `master`.
- **Impact:** Pinning 5chan to the feature source SHA can install a tarball whose runtime and typings still omit the new API, causing downstream type errors even though the hooks repo's source and CI look green.
- **Mitigation:** When updating 5chan to a new hooks change, verify whether hooks `master` has a newer follow-up `chore(ci): update dist and coverage badge` commit and pin 5chan to that dist-synced SHA rather than the source-only SHA.
- **Status:** confirmed
### Portless breaks Windows installs
- **Date:** 2026-03-04
- **Observed by:** Codex
- **Context:** GitHub Actions `Test Windows` dependency install on `windows-2022`
- **What was surprising:** `portless@0.5.2` is a local dev-only tool, but keeping it in `devDependencies` makes `yarn install` fail on Windows because the package declares `win32` unsupported.
- **Impact:** Windows CI fails before build steps run, even though the app does not need `portless` there.
- **Mitigation:** Keep `portless` in `optionalDependencies` and make `yarn start` fall back to direct `vite` startup when `portless` is unavailable.
- **Status:** confirmed
### Electron RPC uses direct pkc-js imports
- **Date:** 2026-03-07
- **Observed by:** Codex
- **Context:** The desktop bootstrap now imports `@pkcprotocol/pkc-js/rpc` directly from `electron/start-pkc-rpc.js`.
- **What was surprising:** Most app data access still goes through `@bitsocialnet/bitsocial-react-hooks`, but the Electron-local RPC bootstrap is intentionally a direct `pkc-js` integration.
- **Impact:** Agents may try to route Electron RPC back through hooks, or reintroduce the legacy protocol package name while fixing dependency/tooling warnings.
- **Mitigation:** Keep Electron RPC on direct `@pkcprotocol/pkc-js` imports. If `knip` flags `electron/start-pkc-rpc.js`, audit the actual dependency graph before adding ignores or legacy packages.
- **Status:** confirmed
### Electron packaging can ship a broken `better-sqlite3` binary
- **Date:** 2026-03-17
- **Observed by:** Codex
- **Context:** Investigating the `v0.7.1` macOS arm64 DMG after the app showed a live IPFS node but never loaded boards or comments.
- **What was surprising:** The packaged app can start IPFS successfully while `electron/start-pkc-rpc.js` loops forever because `/Applications/5chan.app/.../better_sqlite3.node` was built for plain Node 22 (`NODE_MODULE_VERSION 127`) instead of Electron 36 (`NODE_MODULE_VERSION 135`).
- **Impact:** The local RPC server on `ws://localhost:9138` never starts, so the desktop app cannot load boards, posts, or comments even though node stats look healthy.
- **Mitigation:** Before any Electron package/build job, run `yarn electron:prepare-package` so `better-sqlite3` is rebuilt for Electron and immediately verified via `ELECTRON_RUN_AS_NODE=1 electron`.
- **Status:** confirmed
### Plain Vite fallback used to hard-fail on port 1355
- **Date:** 2026-03-30
- **Observed by:** Codex
- **Context:** Running `PORTLESS=0 yarn start` while another local service already owned port `3000`
- **What was surprising:** The non-Portless dev fallback forced Vite onto `5chan.localhost` with `--strictPort`, so the fallback path could fail immediately even though the main Portless flow is collision-safe.
- **Impact:** Contributors could lose the fallback dev path or interrupt their startup flow when `3000` was already busy.
- **Mitigation:** Keep the fallback behind `scripts/start-dev.js`, which now probes from `3000` upward and starts Vite on the next free port instead of exiting.
- **Status:** confirmed
### Fixed Portless app names collide across 5chan worktrees
- **Date:** 2026-03-30
- **Observed by:** Codex
- **Context:** Starting `yarn start` in one 5chan worktree while another 5chan worktree was already serving through Portless
- **What was surprising:** Using the literal Portless app name `5chan` in every worktree makes the route itself collide, even when the backing ports are different, so the second process fails with `"5chan.localhost" is already registered`.
- **Impact:** Parallel 5chan branches can block each other even though Portless is meant to let them coexist safely.
- **Mitigation:** Keep Portless startup behind `scripts/start-dev.js`, which now uses a branch-scoped `*.5chan.localhost` route outside the canonical case and automatically increments a `-2`, `-3`, ... suffix when that branch-scoped route is already occupied.
- **Status:** confirmed
### Toolchain model names are not interchangeable
- **Date:** 2026-04-08
- **Observed by:** contributor + Codex
- **Context:** Reviewing repo-managed agent configs under `.codex/agents`, `.cursor/agents`, and `.claude/agents`
- **What was surprising:** `composer-2` is only available for Cursor in this repo, while Codex agents using `gpt-5.3-codex` or `gpt-5.3-codex-spark` perform poorly enough that they should not be configured by default.
- **Impact:** Agents can silently inherit invalid or weak model settings, leading to broken subagent runs or degraded implementation quality.
- **Mitigation:** Keep `.cursor` agent configs on Cursor-supported models only, never use `composer-2` in `.claude`, and standardize `.codex/agents/*.toml` on `gpt-5.4` unless a contributor explicitly requests an override.
- **Status:** confirmed
Reference in New Issue View Git Blame Copy Permalink