feat(editor): restore reading scroll position on reload (#266) #267

agent_coder · 2026-06-30T11:44:01+03:00

agent_coder commented

2026-06-30 11:44:01 +03:00

Summary

Сохранение и восстановление позиции чтения (scroll) при перезагрузке/повторном открытии страницы. closes #266

Новый хук apps/client/src/features/editor/hooks/use-scroll-position.ts — useScrollPosition(pageId):
- сейв window.scrollY в sessionStorage (ключ gitmost:scroll-position:<pageId>) по throttled scroll (250мс, leading-edge), pagehide, visibilitychange→hidden и в cleanup (SPA-навигация);
- синхронный захват ранее сохранённого значения в initialTargetRef в теле рендера — ДО регистрации обработчиков, чтобы свежий 0 при загрузке не затёр цель;
- восстановление один раз: уступает якорю #hash, поллит высоту документа и скроллит к сохранённому Y, как только контент дорос, либо по таймауту 5с скроллит к максимально достижимой позиции (clamp);
- весь доступ к Storage в try/catch — приватный режим/квота не валят страницу;
- in-flight poll хранится в ref и отменяется в cleanup — быстрая SPA-навигация не проскроллит следующую страницу.
Подключение в page-editor.tsx: хук + эффект if (!showStatic && editor) restoreScrollPosition() сразу после перехода в live-контент. Существующая якорная логика (onCreate/handleScrollTo) не тронута — механизмы взаимоисключающие (#hash).

Хранилище — sessionStorage (MVP per issue): переживает F5 в той же вкладке, чистится на закрытии, само-ограничивается. localStorage с LRU — опциональный follow-up.

How verified

На стенде (apps/client): vitest run use-scroll-position.test.ts — 7/7; tsc --noEmit — 0; eslint (новый хук + тест) — 0.

Тесты (нон-вакуумные, fake timers): (a) throttled-сейв пишет scrollY под ключ pageId; (b) #hash → no-op (контент УЖЕ высокий, так что без hash-гарда scrollTo сработал бы — тест реально проверяет приоритет якоря); (c) ничего/≤0 сохранено → no-op; (d) скроллит к Y когда контент дорос; (d2) clamp к max после таймаута; (e) исключение Storage не пробрасывается; (f) poll отменяется при unmount (без cleanup'а проскроллил бы следующую страницу — тест валится без фикса).

Внутренний ревью (отдельный субагент) поймал 2 реальных пункта — оба исправлены: (1) poll-таймер не отменялся при unmount → видимый неверный скролл новой страницы (теперь ref + cleanup); (2) тест (b) был вакуумным → переписал на fake timers + высокий scrollHeight. Остальные замечания (#3-#5) — некритичные, приняты как ок для MVP.

Checklist

позиция восстанавливается после F5; #hash сохраняет приоритет
нет протекания позиции между страницами; восстановление одноразовое, без дёрганья
ошибки Storage не валят страницу; комментарии на английском
вне scope ничего не менялось (новый хук + подключение в page-editor)

## Summary Сохранение и восстановление позиции чтения (scroll) при перезагрузке/повторном открытии страницы. closes #266 - Новый хук `apps/client/src/features/editor/hooks/use-scroll-position.ts` — `useScrollPosition(pageId)`: - **сейв** `window.scrollY` в `sessionStorage` (ключ `gitmost:scroll-position:<pageId>`) по throttled `scroll` (250мс, leading-edge), `pagehide`, `visibilitychange→hidden` и в cleanup (SPA-навигация); - **синхронный захват** ранее сохранённого значения в `initialTargetRef` в теле рендера — ДО регистрации обработчиков, чтобы свежий `0` при загрузке не затёр цель; - **восстановление** один раз: уступает якорю `#hash`, поллит высоту документа и скроллит к сохранённому Y, как только контент дорос, либо по таймауту 5с скроллит к максимально достижимой позиции (clamp); - весь доступ к `Storage` в try/catch — приватный режим/квота не валят страницу; - **in-flight poll хранится в ref и отменяется в cleanup** — быстрая SPA-навигация не проскроллит следующую страницу. - Подключение в `page-editor.tsx`: хук + эффект `if (!showStatic && editor) restoreScrollPosition()` сразу после перехода в live-контент. Существующая якорная логика (`onCreate`/`handleScrollTo`) не тронута — механизмы взаимоисключающие (`#hash`). Хранилище — `sessionStorage` (MVP per issue): переживает F5 в той же вкладке, чистится на закрытии, само-ограничивается. `localStorage` с LRU — опциональный follow-up. ## How verified На стенде (apps/client): `vitest run use-scroll-position.test.ts` — **7/7**; `tsc --noEmit` — 0; `eslint` (новый хук + тест) — 0. Тесты (нон-вакуумные, fake timers): (a) throttled-сейв пишет scrollY под ключ pageId; (b) `#hash` → no-op (контент УЖЕ высокий, так что без hash-гарда scrollTo сработал бы — тест реально проверяет приоритет якоря); (c) ничего/≤0 сохранено → no-op; (d) скроллит к Y когда контент дорос; (d2) clamp к max после таймаута; (e) исключение Storage не пробрасывается; (f) poll отменяется при unmount (без cleanup'а проскроллил бы следующую страницу — тест валится без фикса). Внутренний ревью (отдельный субагент) поймал 2 реальных пункта — **оба исправлены**: (1) poll-таймер не отменялся при unmount → видимый неверный скролл новой страницы (теперь ref + cleanup); (2) тест `(b)` был вакуумным → переписал на fake timers + высокий scrollHeight. Остальные замечания (#3-#5) — некритичные, приняты как ок для MVP. ## Checklist - [x] позиция восстанавливается после F5; `#hash` сохраняет приоритет - [x] нет протекания позиции между страницами; восстановление одноразовое, без дёрганья - [x] ошибки Storage не валят страницу; комментарии на английском - [x] вне scope ничего не менялось (новый хук + подключение в page-editor)

agent_coder added 1 commit 2026-06-30 11:44:02 +03:00

feat(editor): restore reading scroll position on reload (#266 ) b601c78c21

Adds useScrollPosition(pageId): saves window.scrollY to sessionStorage
(key gitmost:scroll-position:<pageId>) on throttled scroll / pagehide /
visibilitychange / cleanup, capturing the previously-saved value
synchronously at mount before any handler can overwrite it with the fresh 0.
restoreScrollPosition() (wired in page-editor.tsx to fire once the live
content is laid out, !showStatic && editor) yields to a #hash anchor, then
polls the document height and scrolls to the saved Y once the content is
tall enough, with a 5s timeout clamped to the max reachable position. All
storage access is try/caught so a disabled/quota'd Storage never breaks the
page. The in-flight restore poll is held in a ref and cancelled on unmount,
so a fast SPA navigation can't scroll the next page. closes #266

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

agent_coder added the review/needs label 2026-06-30 11:44:03 +03:00

agent_reviewer commented

2026-06-30 11:55:40 +03:00

Ревью b601c78c2 — раунд 1, Full tier (9 аспектов вкл. COHERENCE). Фича #266: восстановление позиции чтения (scroll) при reload/reopen.

Вердикт: CHANGES (3 пункта — все low/medium: тест-покрытие самого тонкого инварианта хука, одно нарушение задокументированного правила AGENTS.md, и документирование скрытой кросс-файловой связки). Сама фича корректна и проверена по коду многими аспектами. Отвечай по id.

Что проверено и ЧИСТО:

Навигация page→page НЕ ломается (coherence+stability): page.tsx:103 рендерит <MemoizedFullEditor key={page.id} pageId={page.id}> — key ремаунтит всё поддерево PageEditor на каждую страницу, рефы (initialTargetRef/hasRestoredRef) свежие, рендер-боди перечитывает Y новой страницы. (Это же — основа пункта F3 ниже.)
Контейнер скролла — window (coherence): Mantine v7 AppShell.Main без overflow-override → скроллит документ, window.scrollY ненулевой, фича не no-op.
Два механизма скролла не дерутся (coherence+regressions+architecture): и useEditorScroll.handleScrollTo (anchor по #hash), и restoreScrollPosition гейтятся на одном предикате window.location.hash и взаимоисключающи; handleScrollTo no-op без хеша. Глобального scroll/pagehide/visibilitychange слушателя в клиенте больше нет (грэп чист) — коллизий нет, слушатели снимаются на ремаунте.
Гонки/утечки/замыкания чисты (stability): pollTimerRef — стабильный per-instance реф, cleanup отменяет in-flight setTimeout(tryRestore) (JS однопоточен, окна между clearTimeout и ремаунтом нет — нет скролла чужой страницы); throttleTimer и pollTimerRef оба чистятся; save() на unmount пишет СТАРЫЙ pageId (корректно); clamp Math.min(targetY, Math.max(maxScroll,0)) верен для коротких/растущих страниц.
Security (LGTM): client-only sessionStorage, pageId — серверный id в КЛЮЧЕ (не sink), значение — оффсет (не чувствительно), Number.isFinite-гард + clamp, нет network/DOM/eval-стоков.
Тест не-вакуозен (test-coverage): гоняет РЕАЛЬНЫЙ хук (renderHook+jsdom+fake timers+реальный sessionStorage), покрывает poll-until-tall, 5s-таймаут-фоллбек, cleanup-отменяет-poll, hash-yield — ассерты падают при сломе. Комментарии все верны (documentation). Размещение/именование/консты — по конвенциям.

Что сделать

F1 [test coverage] Покрыть синхронный initial-capture (анти-clobber) и restore-at-most-once — два непокрытых инварианта самого хука — apps/client/src/features/editor/hooks/use-scroll-position.ts:64-71,127-128
Сердце хука — синхронный захват сохранённого Y в initialTargetRef в теле рендера ДО регистрации хендлеров, чтобы свежий scroll@0 при загрузке не затёр цель. Этот инвариант (ради которого весь трюк и существует) НЕ покрыт: существующий restore-тест захватывает 500, но не шлёт затирающий scroll@0, поэтому регрессия, переносящая захват в эффект (после регистрации хендлеров), молча убила бы фичу и тест бы НЕ упал. Аналогично не покрыт гард hasRestoredRef (restore не более раза) — а wiring-эффект [showStatic, editor, restoreScrollPosition] может перезапуститься, и сломанный гард дважды дёрнул бы scrollTo, выдернув читателя. Это самые тонкие/рискованные пути изменения. (Severity medium — риск-взвешенно: покрыт дешёвый storage-хелпер и poll, но не анти-clobber-ядро.)
Fix: добавить кейс — setItem(key,"500"); renderHook; диспатчить scroll при scrollY=0 и проверить, что storage стал "0"; вызвать restoreScrollPosition; вырастить scrollHeight; проверить scrollTo({top:500}) (цель пережила затирание). И второй: после успешного restore вызвать restoreScrollPosition() повторно — scrollTo вызван ровно раз.

F2 [conventions] Не глотать ошибки sessionStorage молча — логировать (правило AGENTS.md) — apps/client/src/features/editor/hooks/use-scroll-position.ts:28,36
AGENTS.md («Conventions») задаёт жёсткое правило: «Errors must never be swallowed… Every caught error MUST be logged in full to the console/logger.» Оба catch (readStorage возвращает null, writeStorage — пустой // Silently ignore) глотают молча. User-facing нотификация для best-effort-скролла была бы излишней, но консоль-лог — обязателен по правилу. (Severity low, но это нарушение задокументированного правила, не вкусовщина.)
Fix: catch (err) { console.warn("[useScrollPosition] sessionStorage read failed", err); return null; } (и аналогично в writeStorage).

F3 [stability] Задокументировать жёсткую зависимость от remount-per-pageId — apps/client/src/features/editor/hooks/use-scroll-position.ts:56-62
Хук КОРРЕКТЕН только потому, что PageEditor ремаунтится на каждую страницу (key={page.id}, apps/client/src/pages/page/page.tsx:103): initialTargetRef/hasRestoredRef латчатся per-mount и НЕ сбрасываются на смену pageId (эффект-то на [pageId] перезапускается, рефы — нет). Если этот key когда-нибудь уберут при рефакторинге, restore молча сломается на 2-й странице (рефы держат старое). Связка невидима — она в другом файле. Два аспекта (stability+test-coverage) указали на неё независимо. (Severity low, латентно — живого бага нет.)
Fix: комментарий у объявления рефов (56-62), явно фиксирующий, что хук требует remount-per-pageId через key={page.id} в page.tsx, и что без него рефы надо сбрасывать на смену pageId. (Поведенческий само-сброс рефов имеет свою тонкость с порядком render/effect — поэтому достаточно явной фиксации контракта; опционально dev-only console.warn при детекте смены pageId без ремаунта.)

⛔ DROP — кодеру НЕ делать (калибровка)

[below-threshold] low/med [simplification] заменить initialTargetRef+рендер-гард на useState(() => readStorage(pageId) ?? 0) — :56-71: идиоматичнее и короче, но вкусовщина (оба валидны), не load-bearing, и трогает тот же код, что F3 — не мешаю.
[below-threshold] low/low [conventions] Mantine useThrottledCallback/useDocumentVisibility вместо ручного throttle/visibility — :82-101: сам аспект признал acceptable (state-возвращающие хуки форсили бы ре-рендеры в императивном listener-эффекте).
[below-threshold] low [test-coverage] тест на pageId-смену-без-ремаунта — key={page.id} структурно исключает сценарий в приложении; контракт фиксируется F3-комментарием, а не тестом несуществующего пути.

Объективные проверки: vitest сам прогнать не могу (нет vitest/@testing-library/react в node_modules окружения ревью); тест НЕЗАВИСИМО верифицирован не-вакуозным против реального хука. После F1 прогони pnpm --filter client vitest run use-scroll-position.

Маркер reviewed_head обновлён на b601c78c2. После правок верни review/needs.

Ревью **b601c78c2** — раунд 1, Full tier (9 аспектов вкл. COHERENCE). Фича #266: восстановление позиции чтения (scroll) при reload/reopen. **Вердикт: CHANGES (3 пункта — все low/medium: тест-покрытие самого тонкого инварианта хука, одно нарушение задокументированного правила AGENTS.md, и документирование скрытой кросс-файловой связки).** Сама фича корректна и проверена по коду многими аспектами. Отвечай по id. Что проверено и ЧИСТО: - **Навигация page→page НЕ ломается** (coherence+stability): `page.tsx:103` рендерит `<MemoizedFullEditor key={page.id} pageId={page.id}>` — `key` ремаунтит всё поддерево `PageEditor` на каждую страницу, рефы (`initialTargetRef`/`hasRestoredRef`) свежие, рендер-боди перечитывает Y новой страницы. (Это же — основа пункта F3 ниже.) - **Контейнер скролла — window** (coherence): Mantine v7 `AppShell.Main` без `overflow`-override → скроллит документ, `window.scrollY` ненулевой, фича не no-op. - **Два механизма скролла не дерутся** (coherence+regressions+architecture): и `useEditorScroll.handleScrollTo` (anchor по `#hash`), и `restoreScrollPosition` гейтятся на одном предикате `window.location.hash` и взаимоисключающи; `handleScrollTo` no-op без хеша. Глобального `scroll`/`pagehide`/`visibilitychange` слушателя в клиенте больше нет (грэп чист) — коллизий нет, слушатели снимаются на ремаунте. - **Гонки/утечки/замыкания чисты** (stability): `pollTimerRef` — стабильный per-instance реф, cleanup отменяет in-flight `setTimeout(tryRestore)` (JS однопоточен, окна между clearTimeout и ремаунтом нет — нет скролла чужой страницы); `throttleTimer` и `pollTimerRef` оба чистятся; `save()` на unmount пишет СТАРЫЙ pageId (корректно); clamp `Math.min(targetY, Math.max(maxScroll,0))` верен для коротких/растущих страниц. - **Security** (LGTM): client-only sessionStorage, `pageId` — серверный id в КЛЮЧЕ (не sink), значение — оффсет (не чувствительно), `Number.isFinite`-гард + clamp, нет network/DOM/eval-стоков. - **Тест не-вакуозен** (test-coverage): гоняет РЕАЛЬНЫЙ хук (`renderHook`+jsdom+fake timers+реальный sessionStorage), покрывает poll-until-tall, 5s-таймаут-фоллбек, cleanup-отменяет-poll, hash-yield — ассерты падают при сломе. Комментарии все верны (documentation). Размещение/именование/консты — по конвенциям. ### Что сделать **F1 [test coverage] Покрыть синхронный initial-capture (анти-clobber) и restore-at-most-once — два непокрытых инварианта самого хука** — `apps/client/src/features/editor/hooks/use-scroll-position.ts:64-71,127-128` Сердце хука — синхронный захват сохранённого Y в `initialTargetRef` в теле рендера ДО регистрации хендлеров, чтобы свежий `scroll`@0 при загрузке не затёр цель. Этот инвариант (ради которого весь трюк и существует) НЕ покрыт: существующий restore-тест захватывает 500, но не шлёт затирающий `scroll`@0, поэтому регрессия, переносящая захват в эффект (после регистрации хендлеров), молча убила бы фичу и тест бы НЕ упал. Аналогично не покрыт гард `hasRestoredRef` (restore не более раза) — а wiring-эффект `[showStatic, editor, restoreScrollPosition]` может перезапуститься, и сломанный гард дважды дёрнул бы `scrollTo`, выдернув читателя. Это самые тонкие/рискованные пути изменения. (Severity medium — риск-взвешенно: покрыт дешёвый storage-хелпер и poll, но не анти-clobber-ядро.) Fix: добавить кейс — `setItem(key,"500")`; `renderHook`; диспатчить `scroll` при `scrollY=0` и проверить, что storage стал `"0"`; вызвать `restoreScrollPosition`; вырастить `scrollHeight`; проверить `scrollTo({top:500})` (цель пережила затирание). И второй: после успешного restore вызвать `restoreScrollPosition()` повторно — `scrollTo` вызван ровно раз. **F2 [conventions] Не глотать ошибки sessionStorage молча — логировать (правило AGENTS.md)** — `apps/client/src/features/editor/hooks/use-scroll-position.ts:28,36` AGENTS.md («Conventions») задаёт жёсткое правило: «Errors must never be swallowed… Every caught error MUST be logged in full to the console/logger.» Оба `catch` (`readStorage` возвращает `null`, `writeStorage` — пустой `// Silently ignore`) глотают молча. User-facing нотификация для best-effort-скролла была бы излишней, но консоль-лог — обязателен по правилу. (Severity low, но это нарушение задокументированного правила, не вкусовщина.) Fix: `catch (err) { console.warn("[useScrollPosition] sessionStorage read failed", err); return null; }` (и аналогично в `writeStorage`). **F3 [stability] Задокументировать жёсткую зависимость от remount-per-pageId** — `apps/client/src/features/editor/hooks/use-scroll-position.ts:56-62` Хук КОРРЕКТЕН только потому, что `PageEditor` ремаунтится на каждую страницу (`key={page.id}`, `apps/client/src/pages/page/page.tsx:103`): `initialTargetRef`/`hasRestoredRef` латчатся per-mount и НЕ сбрасываются на смену `pageId` (эффект-то на `[pageId]` перезапускается, рефы — нет). Если этот `key` когда-нибудь уберут при рефакторинге, restore молча сломается на 2-й странице (рефы держат старое). Связка невидима — она в другом файле. Два аспекта (stability+test-coverage) указали на неё независимо. (Severity low, латентно — живого бага нет.) Fix: комментарий у объявления рефов (56-62), явно фиксирующий, что хук требует remount-per-pageId через `key={page.id}` в page.tsx, и что без него рефы надо сбрасывать на смену pageId. (Поведенческий само-сброс рефов имеет свою тонкость с порядком render/effect — поэтому достаточно явной фиксации контракта; опционально dev-only `console.warn` при детекте смены pageId без ремаунта.) --- ### ⛔ DROP — кодеру НЕ делать (калибровка) - `[below-threshold]` `low/med` **[simplification]** заменить `initialTargetRef`+рендер-гард на `useState(() => readStorage(pageId) ?? 0)` — `:56-71`: идиоматичнее и короче, но вкусовщина (оба валидны), не load-bearing, и трогает тот же код, что F3 — не мешаю. - `[below-threshold]` `low/low` **[conventions]** Mantine `useThrottledCallback`/`useDocumentVisibility` вместо ручного throttle/visibility — `:82-101`: сам аспект признал acceptable (state-возвращающие хуки форсили бы ре-рендеры в императивном listener-эффекте). - `[below-threshold]` `low` **[test-coverage]** тест на pageId-смену-без-ремаунта — `key={page.id}` структурно исключает сценарий в приложении; контракт фиксируется F3-комментарием, а не тестом несуществующего пути. Объективные проверки: vitest сам прогнать не могу (нет `vitest`/`@testing-library/react` в node_modules окружения ревью); тест НЕЗАВИСИМО верифицирован не-вакуозным против реального хука. После F1 прогони `pnpm --filter client vitest run use-scroll-position`. Маркер `reviewed_head` обновлён на `b601c78c2`. После правок верни `review/needs`.

agent_reviewer added review/changes-requested and removed review/needs labels 2026-06-30 11:55:40 +03:00

agent_coder added 1 commit 2026-06-30 12:14:02 +03:00

test/refactor(#266 ): cover anti-clobber capture + once-guard; log storage errors 30cdd65b92

Review round 1 on the scroll-position feature:
- F1: add two tests for the hook's subtlest invariants — (a2) the restore
  target is captured synchronously at mount and survives a fresh scroll@0
  overwriting storage on load (a regression moving the capture into an effect
  would now fail); (a3) restore runs at most once per mount even when called
  again (the wiring effect can re-run).
- F2: log instead of silently swallowing sessionStorage errors in
  readStorage/writeStorage (AGENTS.md "errors must never be swallowed" rule);
  no user notification since a missed scroll restore is not actionable.
- F3: document the hard dependency on PageEditor remounting per page
  (key={page.id}) at the refs declaration — the per-mount refs are not reset
  on an in-place pageId change, so removing that key would break restore on
  the 2nd page.

vitest 9/9, tsc 0, eslint 0.

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

agent_coder commented

2026-06-30 12:14:23 +03:00

F1: fixed — добавил два теста на тончайшие инварианты хука:

(a2) анти-clobber синхронного захвата: сохранил 500, диспатчу scroll@scrollY=0 (storage становится "0"), затем restoreScrollPosition при высоком scrollHeight → scrollTo({top:500}). Цель пережила затирание; если перенести захват в эффект (после регистрации хендлеров) — тест упадёт.
(a3) restore-at-most-once: после успешного restore зову restoreScrollPosition() повторно → scrollTo вызван ровно 1 раз (сломанный hasRestoredRef дёрнул бы дважды).

F2: fixed — оба catch (readStorage/writeStorage) теперь логируют console.warn("[useScrollPosition] … failed", err) вместо молчаливого глотания (правило AGENTS.md «errors must never be swallowed»). User-facing нотификацию не делаю — пропущенное восстановление скролла не actionable, но ошибка логируется.

F3: fixed — добавил комментарий-контракт у объявления рефов: хук корректен только при remount-per-pageId (key={page.id} в page.tsx); рефы латчатся per-mount и НЕ сбрасываются на in-place смену pageId, поэтому при удалении этого key restore молча сломается на 2-й странице (тогда рефы надо сбрасывать на смену pageId).

Проверки: vitest — 9/9; tsc — 0; eslint — 0 (console.warn разрешён). Возвращаю review/needs.

F1: fixed — добавил два теста на тончайшие инварианты хука: - **(a2) анти-clobber синхронного захвата:** сохранил 500, диспатчу `scroll`@`scrollY=0` (storage становится `"0"`), затем `restoreScrollPosition` при высоком `scrollHeight` → `scrollTo({top:500})`. Цель пережила затирание; если перенести захват в эффект (после регистрации хендлеров) — тест упадёт. - **(a3) restore-at-most-once:** после успешного restore зову `restoreScrollPosition()` повторно → `scrollTo` вызван ровно 1 раз (сломанный `hasRestoredRef` дёрнул бы дважды). F2: fixed — оба `catch` (`readStorage`/`writeStorage`) теперь логируют `console.warn("[useScrollPosition] … failed", err)` вместо молчаливого глотания (правило AGENTS.md «errors must never be swallowed»). User-facing нотификацию не делаю — пропущенное восстановление скролла не actionable, но ошибка логируется. F3: fixed — добавил комментарий-контракт у объявления рефов: хук корректен только при remount-per-pageId (`key={page.id}` в `page.tsx`); рефы латчатся per-mount и НЕ сбрасываются на in-place смену `pageId`, поэтому при удалении этого `key` restore молча сломается на 2-й странице (тогда рефы надо сбрасывать на смену pageId). Проверки: `vitest` — **9/9**; `tsc` — 0; `eslint` — 0 (`console.warn` разрешён). Возвращаю `review/needs`.

agent_coder added review/needs and removed review/changes-requested labels 2026-06-30 12:14:23 +03:00

agent_reviewer commented

2026-06-30 12:28:31 +03:00

Ревью 30cdd65b9 — раунд 2 (правки F1/F2/F3), спотлайт на трёх правках + проверка отсутствия регрессий.

Вердикт: PASS. Все три пункта закрыты и сверены ПО КОДУ. Готово к мержу.

F1 [test coverage] — закрыт, оба теста не-вакуозны.
- (a2) анти-clobber: сохраняет 500, диспатчит scroll@scrollY=0, ассертит, что storage СТАЛ "0" (затирание реально произошло), затем при высоком scrollHeight restoreScrollPosition → scrollTo({top:500}). Цель пережила затирание — пинит ровно синхронный mount-захват: перенос захвата в эффект (после регистрации хендлеров) прочитал бы затёртый 0 и тест упал бы.
- (a3) restore-once: после успешного restore повторный restoreScrollPosition() → scrollTo вызван ровно 1 раз. Сломанный hasRestoredRef дёрнул бы дважды (двойной скролл, выдёргивающий читателя) — поймано.
F2 [conventions] — закрыт. Оба catch (readStorage/writeStorage) логируют console.warn("[useScrollPosition] … failed", err) с комментарием со ссылкой на правило AGENTS.md «errors must never be swallowed». User-facing нотификация осознанно не делается (пропущенное восстановление не actionable) — это и есть верный объём правила.
F3 [stability] — закрыт. У объявления рефов добавлен CONTRACT-комментарий: хук корректен только при remount-per-pageId через key={page.id} в page.tsx; рефы латчатся per-mount и не сбрасываются на in-place смену pageId; при удалении key restore молча сломается на 2-й странице (тогда сбрасывать рефы на смену pageId). Скрытая кросс-файловая связка теперь зафиксирована в коде.

Дельта — только тесты (+45), логирование в catch и комментарий (+20 в хуке); логика restore не менялась, регрессий нет. Остальная часть PR верифицирована чистой в раунде 1 (ремаунт по key, window-контейнер, anchor/restore взаимоисключающи, гонок/утечек нет, security чисто).

Объективные проверки: vitest сам прогнать не могу (нет vitest/@testing-library/react в окружении ревью); базис PASS — оба новых теста НЕЗАВИСИМО верифицированы не-вакуозными против реального хука (форсят затирание/повторный вызов, ассерты падают при сломе инварианта), F2/F3 — лог+комментарий (тест не нужен), кодер отчитался о прогоне vitest.

Маркер reviewed_head обновлён на 30cdd65b9.

Ревью **30cdd65b9** — раунд 2 (правки F1/F2/F3), спотлайт на трёх правках + проверка отсутствия регрессий. **Вердикт: PASS.** Все три пункта закрыты и сверены ПО КОДУ. Готово к мержу. - **F1 [test coverage] — закрыт, оба теста не-вакуозны.** - `(a2)` анти-clobber: сохраняет 500, диспатчит `scroll`@`scrollY=0`, ассертит, что storage СТАЛ `"0"` (затирание реально произошло), затем при высоком `scrollHeight` `restoreScrollPosition` → `scrollTo({top:500})`. Цель пережила затирание — пинит ровно синхронный mount-захват: перенос захвата в эффект (после регистрации хендлеров) прочитал бы затёртый 0 и тест упал бы. - `(a3)` restore-once: после успешного restore повторный `restoreScrollPosition()` → `scrollTo` вызван ровно 1 раз. Сломанный `hasRestoredRef` дёрнул бы дважды (двойной скролл, выдёргивающий читателя) — поймано. - **F2 [conventions] — закрыт.** Оба `catch` (`readStorage`/`writeStorage`) логируют `console.warn("[useScrollPosition] … failed", err)` с комментарием со ссылкой на правило AGENTS.md «errors must never be swallowed». User-facing нотификация осознанно не делается (пропущенное восстановление не actionable) — это и есть верный объём правила. - **F3 [stability] — закрыт.** У объявления рефов добавлен CONTRACT-комментарий: хук корректен только при remount-per-pageId через `key={page.id}` в `page.tsx`; рефы латчатся per-mount и не сбрасываются на in-place смену pageId; при удалении `key` restore молча сломается на 2-й странице (тогда сбрасывать рефы на смену pageId). Скрытая кросс-файловая связка теперь зафиксирована в коде. Дельта — только тесты (+45), логирование в catch и комментарий (+20 в хуке); логика restore не менялась, регрессий нет. Остальная часть PR верифицирована чистой в раунде 1 (ремаунт по key, window-контейнер, anchor/restore взаимоисключающи, гонок/утечек нет, security чисто). Объективные проверки: vitest сам прогнать не могу (нет `vitest`/`@testing-library/react` в окружении ревью); базис PASS — оба новых теста НЕЗАВИСИМО верифицированы не-вакуозными против реального хука (форсят затирание/повторный вызов, ассерты падают при сломе инварианта), F2/F3 — лог+комментарий (тест не нужен), кодер отчитался о прогоне vitest. Маркер `reviewed_head` обновлён на `30cdd65b9`.

agent_reviewer added review/approved and removed review/needs labels 2026-06-30 12:28:31 +03:00

vvzvlad merged commit 38f9a7938a into develop

2026-06-30 19:59:51 +03:00

vvzvlad referenced this issue from a commit

2026-06-30 19:59:53 +03:00

Merge pull request 'feat(editor): restore reading scroll position on reload (#266)' (#267) from feat/266-scroll-position into develop

agent_reviewer referenced this pull request

2026-07-01 01:29:53 +03:00

feat(comment): hover tooltip with comment text over comment marks (#268) #271

Sign in to join this conversation.