Files
stem-launcher/doc/review-claude-2026-07-14.md

151 lines
5.9 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Senior review Claude — 2026-07-14
Zakres recenzji:
- `stem-launcher`: plan architektury, kontenery, agenci i migracja nazw;
- `rv32i-hazard3-student-env`: plan ewaluacji, Containerfile/Dockerfile,
Compose, integracja launchera i specyfikacja trzech obrazów;
- zgodność planu z bieżącym `rvctl.py` i `workspace.json`.
## Werdykt
Claude wydał akceptację warunkową. Zaakceptował:
- dokładnie trzy kontenery robocze i wspólną warstwę UI;
- profile `native-amd64`, `hazard3-sim`, `rp2350` oraz targety wewnątrz
profilu;
- Git, klucze i tokeny wyłącznie na hoście;
- rootless Podmana bez socketu runtime i bez `--privileged`;
- capabilities i wynik `unsupported` zamiast warunków zaszytych w launcherze;
- migrację zgodności przed zmianą nazwy repo Gitea;
- `fast-rsp` jako obecny backend, `fast-dm` jako kierunek i JTAG jako gate;
- tę samą sesję tmux/Neovim dla ucznia i agenta.
## Findings i rozstrzygnięcia
### P1 — graf cache Containerfile
Problem: częsta konfiguracja UI znajdowała się w stage'u będącym przodkiem
ciężkich toolchainów. Jej zmiana przebudowałaby SDK i Verilator.
Rozstrzygnięcie: `dev-ui-base` zawiera przypięte binaria/dependencies UI, ale
nie `configs/` ani entrypointy. Toolchainowe stage'e dziedziczą z niego, a
konfiguracja jest kopiowana dopiero do finalnych stage'ów. Test cache obejmuje
zmianę entrypointu i konfiguracji UI.
### P1 — limit Unix socketów
Problem: pełne `thread/instance/container-id` w ścieżce workspace przekraczały
linuksowy limit `sun_path`.
Rozstrzygnięcie:
```text
$XDG_RUNTIME_DIR/stem/<thread-key>/<instance-key>/<cid12>/t.sock
$XDG_RUNTIME_DIR/stem/<thread-key>/<instance-key>/<cid12>/n.sock
```
Pełne wartości są w registry/labels. Budżet ścieżki wynosi 100 bajtów.
### P1 — `deploy hazard3-sim`
Problem: jeden dokument utożsamiał deploy z załadowaniem symulatora, pozostałe
nie definiowały tej operacji.
Rozstrzygnięcie: `deploy` dla `hazard3-sim` jest `unsupported`. `run` i `debug`
ładują artefakt. Deploy pozostaje operacją targetu sprzętowego.
### P1 — status `fast-dm`
Problem: niezaimplementowany backend był w jednym miejscu opisany jako
codzienny.
Rozstrzygnięcie: dokumenty jawnie mówią:
- teraz: `fast-rsp`;
- docelowo: `fast-dm`;
- okresowo: `fidelity-jtag`.
### P1 — USB RP2350
Problem: statyczne `--device` nie obsługuje hotplug i re-enumeracji BOOTSEL.
Rozstrzygnięcie: zewnętrzny debug probe jest stabilnym, domyślnym backendem.
Natywne USB targetu/picotool jest osobną capability i krótkotrwałą akcją z tego
samego obrazu; launcher ponownie rozwiązuje węzeł po każdej fazie. Build/test
offline nie wymagają probe.
### P2 — koszt testu 10 000 ticków
Problem: 10 000 × 150 000 daje 1,5 miliarda cykli RTL.
Rozstrzygnięcie: PR wykonuje co najmniej 100 ticków z produkcyjnym dzielnikiem,
a długi test schedulera 10 000 ticków działa z jawnie przyspieszonym zegarem.
Nightly/release otrzymuje oddzielny budżet testu produkcyjnego.
### P2 — pozostałe
- wymaganie `probe` przeniesiono z profilu na akcje `debug/deploy`;
- format instancji ujednolicono do `task<N>`;
- manifest otrzymał `default_target` per profil;
- `run rp2350` wymaga pasującego rekordu deploy albo jawnej zgody na istniejący
firmware;
- bramka przenośności rozróżnia wektory niezależne od ABI.
## Korekta jednej sugestii recenzenta
Claude zasugerował, że realny RP2350 może taktować `MTIME` inną częstotliwością
niż symulator. Jest to prawdziwe dla domyślnego trybu SIO, ale nie dla badanego
portu FreeRTOS. Lokalny
`FreeRTOS-Kernel/portable/ThirdParty/GCC/RP2350_RISC-V/port.c` wywołuje:
```c
riscv_timer_set_fullspeed(true);
uxTimerIncrementsForOneTick = clock_get_hz(clk_sys) / configTICK_RATE_HZ;
```
Przy 150 MHz i 1 kHz oba środowiska używają więc 150 000 zliczeń. Plan nadal
wymaga osobnych BSP i niezależnego wyliczania dzielnika, ponieważ tryb timera i
zegar mogą się zmienić.
## Wynik etapu projektu
Wszystkie findings P1 oraz zasadne P2 zostały naniesione do planu. Ten fragment
review odbył się przed implementacją i dlatego nie stanowił jeszcze dowodu
działania środowiska.
## Review implementacji
Po wdrożeniu Claude wykonał dwa pełne oraz zawężone przeglądy diffów. Wykryte
problemy i ich rozstrzygnięcia:
- logi GDB/JTAG/UART sugerowały `0.0.0.0`, mimo planowanego loopbacku — bind i
logi są teraz jednoznacznie `127.0.0.1`;
- Compose zawierał martwe zmienne MCP — Compose służy wyłącznie do lokalnego
buildu i niezarządzanej powłoki, a sesje MCP tworzy kanonicznie `stem`;
- zgodnościowy `tmux-container` omijał rootless runtime — deleguje teraz do
`stem shell`;
- `status/attach/stop/rm --instance` zależały od bieżącej domyślnej karty —
operują teraz po stabilnej nazwie instancji bez checkoutu karty;
- `attach` zakładał jedną błędną nazwę sesji — rozpoznaje sesje `stem-*`,
`rv32i-*`, `host-*` lub jedyną sesję z dedykowanego socketu;
- osierocony socket tmux kończył `attach` bez diagnostyki — zwracany jest teraz
komunikat nakazujący ponowne uruchomienie debug UI;
- rekord deployu pobierał commit z katalogu launchera — używa teraz
`git -C <repo-karty> rev-parse HEAD`.
Końcowe, zawężone review obu repozytoriów zakończyło się wynikiem
`NO_P0_P1`. Potwierdzone zostały także:
- build wszystkich trzech lokalnych profili z warstwami toolchainów w cache;
- 12/12 testów kontraktowych środowiska i 12/12 testów launchera;
- FreeRTOS Blink 1 kHz dla RP2350 RISC-V oraz ARM;
- wykonanie karty bare-metal w Hazard3;
- Termdebug i dashboard po lewej oraz źródło po prawej, w górnym pane tmuxa;
- działanie MCP dla tmuxa i Neovima w wybranym kontenerze;
- nasłuch symulatora GDB wyłącznie na loopbacku.
Poza bieżącym wdrożeniem pozostają jawnie oznaczone etapy badawcze:
`fast-dm` wykorzystujący Debug Module Hazard3 oraz BSP FreeRTOS dla symulatora
RTL. Nie są one przedstawiane jako gotowe.