Delete four documents that contradict the code #99
-118
@@ -1,118 +0,0 @@
|
||||
# Work log
|
||||
|
||||
Temporal memory: what happened and what's next. Structure lives in
|
||||
`CLAUDE.md`, operational instructions in `.pi/skills/yellowjacket-dev/`,
|
||||
measured discoveries in `.planning/NOTES.md`. Don't duplicate those here.
|
||||
|
||||
## Current state
|
||||
|
||||
Plan 005 (agent development harness) is **complete — all seven
|
||||
phases**. Everything from phase 1 onward is still **uncommitted**: one
|
||||
large but coherent working-tree diff, nothing pushed.
|
||||
|
||||
All four tiers verified green from a cold, cleaned state:
|
||||
`make ui-test` 313 passed, `make lint` 0 issues × 3 configurations,
|
||||
`make test` green × 3 passes, `make e2e` 19 passed. Both CI jobs
|
||||
verified green in a bare `ubuntu:24.04` container, including 19/19 on
|
||||
WebKit.
|
||||
|
||||
**Committed and pushed** as `5ca6cad` (the harness) + `ccacd67` (a CI
|
||||
fix), and **green on the real runner**: job `check` ~4 min, job `e2e`
|
||||
~3 min with 19/19 chromium *and* 19/19 webkit. One commit rather than
|
||||
seven because the working tree was the end state, not per-phase
|
||||
snapshots — `Makefile`, `CLAUDE.md` and `lefthook.yml` are touched by
|
||||
nearly every phase, so a split would have been fabricated history.
|
||||
|
||||
Still unverified, because no run has failed yet: the
|
||||
`actions/upload-artifact` step (`continue-on-error`, so it cannot mask
|
||||
a real failure) and whether pnpm honours `npm_config_store_dir` for
|
||||
store caching. Worth checking the next time a spec legitimately fails.
|
||||
|
||||
- [ ] `gitea_ci`'s `job_logs` returns 404 on Gitea 1.27.1 — the endpoint
|
||||
is not exposed. Logs come from the VPS instead: `zstdcat` the file
|
||||
under `gitea/actions_log/<owner>/<repo>/<xx>/<task_id>.log.zst`,
|
||||
and note `zstdcat` is not in the gitea container, so
|
||||
`docker cp` it out first. Job status is `action_run_job.status`
|
||||
(1 success, 2 failure, 4 skipped, 5 waiting, 6 running).
|
||||
Probably belongs in the `gitea` skill, not here.
|
||||
|
||||
Open items deliberately not fixed: WAV tags are write-only
|
||||
(`TestWAVTagsAreNotReadableYet`), `themeStore.loadFromBackend`'s failure
|
||||
handler cannot recover, `backend/playlist` has no CRUD suite.
|
||||
|
||||
## Log
|
||||
|
||||
### 2026-08-11 — cold skill run, then phase 7 (CI)
|
||||
|
||||
- **Followed the skill cold first**, as the last session asked. It
|
||||
works: app up from a wiped `.dev/`, an undocumented flow driven
|
||||
(queue panel + shuffle, asserted on `QueueModeChanged`), stopped —
|
||||
~1 minute, no dead ends. One real config bug: `outputDir` in
|
||||
`.playwright/cli.config.json` resolves against **cwd**, not the
|
||||
config file's directory (only `initScript` does that), so snapshots
|
||||
were landing above the repo and a *stale* one from the previous
|
||||
session answered `ls -t` instead. That cost a DOM walk to disprove a
|
||||
regression that did not exist. Four smaller doc gaps fixed
|
||||
(`sandbox-seed` already runs `testdata`; `ui-setup`/`e2e-setup` were
|
||||
undocumented prerequisites; `snapshot` prints a path; `dev-stop`
|
||||
leaves the browser open), plus `dev-headless.sh`'s own banner, which
|
||||
was suggesting the bare `window.go` call its next paragraph warns
|
||||
against.
|
||||
- **Built both CI jobs as container scripts before writing any YAML**,
|
||||
then transcribed the YAML back out and re-ran it to prove the
|
||||
transcription. Push-and-see is a bad loop on a self-hosted runner.
|
||||
- **It found a real bug immediately**: `make lint` omitted
|
||||
`webkit2_41` on all three passes, so it was linting configurations
|
||||
nothing builds. Invisible on Arch (which still ships
|
||||
`webkit2gtk-4.0.pc`), fatal on Ubuntu 24.04. Tag sets now match
|
||||
`make test`.
|
||||
- **Both open decisions settled by measurement**: ALSA `null` PCM for
|
||||
audio (no daemon; the elapsed clock really advances), dead-address
|
||||
stub for the explore artifact (and setting it for the *app* run, not
|
||||
just seeding, is worth 8x on suite wall clock). **WebKit is a
|
||||
required step** — it had never been run anywhere, so one throwaway
|
||||
container run replaced a coin flip with 19/19 at +11 s.
|
||||
|
||||
### 2026-08-10 — phase 6, pi affordances
|
||||
|
||||
- Added `.pi/skills/yellowjacket-dev/` as a directory rather than a flat
|
||||
file: only the description is always in context, so `SKILL.md` stays
|
||||
short enough that reading it whole is never a decision, and the deeper
|
||||
material sits in `references/{harness,fixtures,ui-tier,schema-change}.md`.
|
||||
- Settled the CLAUDE.md-vs-skill split **grammatically, not topically**,
|
||||
because a topical split is what rots — every new fact gets two
|
||||
plausible homes. Three docs, three tenses: NOTES.md is past
|
||||
(measured, dated, append-only), CLAUDE.md is present (what the system
|
||||
is), the skill is imperative (what to run). A new paragraph's tense
|
||||
decides where it goes.
|
||||
- The five gotchas (binding timeouts, first-run wizard, `pkill -f`,
|
||||
seeds-by-running, WebKit-is-CI-only) went **inline in SKILL.md**, not
|
||||
into a reference: you need them before the failure, not after.
|
||||
- Trimmed CLAUDE.md's "Fixtures and the headless harness" section by
|
||||
about half — the command sequences and gotchas it was carrying are now
|
||||
the skill's, and leaving both would have created exactly the duplicate
|
||||
description this repo has a standing rule against.
|
||||
- Added `make skill-check` / `scripts/skill-check.sh` + a pre-commit
|
||||
hook: every command in `.pi/**/*.md` must be a real `make` target, so
|
||||
the Makefile stays the source of truth for invocation and a renamed
|
||||
target fails a commit instead of misleading an agent later. Verified
|
||||
it fails (it caught its own not-yet-created target) and passes.
|
||||
- Added the `/e2e` prompt template: promoting a hand-driven
|
||||
`playwright-cli` session into a spec is a transcription with four
|
||||
fixed substitutions (refs → testids, sleeps → `waitForEvent`, raw
|
||||
`window.go` → `callBinding`, short fixture → `LONG_TRACK`), plus three
|
||||
runs — pass, pass again, pass after a DB restore — because the usual
|
||||
failure is a spec depending on state the hand-driving left behind.
|
||||
- One shell trap: under `set -euo pipefail`, `x="$(make -pqRr | …)"`
|
||||
fails the whole assignment, because `make -q` exits non-zero when a
|
||||
target is out of date and `pipefail` propagates it.
|
||||
|
||||
### Earlier
|
||||
|
||||
Phases 1–5 of plan 005: fixture generator and manifest, headless launch
|
||||
and seeds, the event bridge + `data-testid` pass + `backend/testctl` +
|
||||
`e2e/`, the Vitest component tier + `make bindings-check`, and the
|
||||
`events.Emit` wrapper with its in-process service-event tests. Recaps
|
||||
and the five "verified end to end" blocks are in
|
||||
`.planning/plans/active/005-agent-development-harness.md`; the lessons
|
||||
are in `.planning/NOTES.md`.
|
||||
@@ -106,5 +106,8 @@ make dev # run with hot-reload
|
||||
make build-prod # produce a release binary
|
||||
```
|
||||
|
||||
More detail for contributors lives in
|
||||
[`docs/dev/overview.md`](./docs/dev/overview.md) and [`CLAUDE.md`](./CLAUDE.md).
|
||||
More detail for contributors lives in [`CLAUDE.md`](./CLAUDE.md) — the
|
||||
architecture, the conventions and the reasons behind them. What is
|
||||
being worked on is [the issue
|
||||
tracker](https://git.ljones.me/yonlu/yellowjacket/issues); #73 is the
|
||||
roadmap.
|
||||
|
||||
@@ -1,194 +0,0 @@
|
||||
# Config Improvement Suggestions
|
||||
|
||||
Remaining suggestions for improving the configuration system in YellowJacket.
|
||||
|
||||
## 2. Thread Safety Concerns
|
||||
|
||||
The current `Config` struct lacks synchronization:
|
||||
- `Load()` and `Save()` can race with concurrent reads
|
||||
- `handleConfigUpdate()` in library mutates `l.conf.DirectoryPath` without locks
|
||||
|
||||
**Suggestion:** Add a `sync.RWMutex` to protect config access, especially if config is read during scans.
|
||||
|
||||
```go
|
||||
type Config struct {
|
||||
mu sync.RWMutex
|
||||
ctx context.Context
|
||||
logger *slog.Logger
|
||||
// ...
|
||||
}
|
||||
|
||||
func (c *Config) Load() error {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
## 3. Nil Safety in Validation
|
||||
|
||||
In `config.go`, validation only runs if `c.Library != nil`, but `handleConfigPost` dereferences `postedConfig.Library` without checking for nil:
|
||||
|
||||
```go
|
||||
if postedConfig.Library != nil {
|
||||
c.Library = postedConfig.Library
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
**Status:** Partially addressed in the event refactor, but consider adding explicit nil checks in `Validate()` as well.
|
||||
|
||||
## 4. Inconsistent Error Handling on HTTP Responses
|
||||
|
||||
In `httphandler.go:28-31`, `WriteHeader` is called *after* rendering the error template, which won't work as expected (headers must be set before writing body):
|
||||
|
||||
```go
|
||||
c.formSubmitError(err.Error()).Render(r.Context(), w)
|
||||
w.WriteHeader(http.StatusInternalServerError) // Too late!
|
||||
```
|
||||
|
||||
**Fix:** Set the status code before rendering:
|
||||
|
||||
```go
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
c.formSubmitError(err.Error()).Render(r.Context(), w)
|
||||
```
|
||||
|
||||
## 5. Make `scanWorkerCount` Configurable
|
||||
|
||||
There's a TODO at `library.go:289`:
|
||||
```go
|
||||
// TODO: make configurable via Config.
|
||||
var scanWorkerCount = goruntime.NumCPU()
|
||||
```
|
||||
|
||||
**Suggestion:** Add this to `library.Config`:
|
||||
|
||||
```go
|
||||
type Config struct {
|
||||
DirectoryPath Directory `form:"Directory" schema:"directory,required"`
|
||||
ScanWorkers int `form:"ScanWorkers" schema:"scan_workers"`
|
||||
}
|
||||
```
|
||||
|
||||
Then in `NewLibrary()` or `Scan()`:
|
||||
|
||||
```go
|
||||
workers := l.conf.ScanWorkers
|
||||
if workers <= 0 {
|
||||
workers = goruntime.NumCPU()
|
||||
}
|
||||
```
|
||||
|
||||
## 6. Consider Config Defaults
|
||||
|
||||
Currently if no config exists, an empty one is saved. Consider providing sensible defaults (e.g., common music directories like `~/Music`).
|
||||
|
||||
```go
|
||||
func (c *Config) setDefaults() {
|
||||
if c.Library == nil {
|
||||
c.Library = &library.Config{}
|
||||
}
|
||||
if c.Library.DirectoryPath == "" {
|
||||
// Try common music directories
|
||||
home, _ := os.UserHomeDir()
|
||||
musicDir := filepath.Join(home, "Music")
|
||||
if info, err := os.Stat(musicDir); err == nil && info.IsDir() {
|
||||
c.Library.DirectoryPath = library.Directory(musicDir)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## 7. Config Reload/Watch Capability
|
||||
|
||||
The config is only loaded at startup. Consider adding:
|
||||
- File watcher for external config changes (using `fsnotify`)
|
||||
- Explicit reload method callable from UI
|
||||
|
||||
```go
|
||||
func (c *Config) Watch() error {
|
||||
watcher, err := fsnotify.NewWatcher()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
go func() {
|
||||
for event := range watcher.Events {
|
||||
if event.Op&fsnotify.Write == fsnotify.Write {
|
||||
c.Load()
|
||||
// Emit event for listeners
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
return watcher.Add(c.filePath)
|
||||
}
|
||||
```
|
||||
|
||||
## 8. Validation Should Return Structured Errors
|
||||
|
||||
Currently validation returns combined errors. Consider returning a structured validation result that the UI can map to specific fields for better user feedback.
|
||||
|
||||
```go
|
||||
type ValidationError struct {
|
||||
Field string
|
||||
Message string
|
||||
}
|
||||
|
||||
type ValidationResult struct {
|
||||
Valid bool
|
||||
Errors []ValidationError
|
||||
}
|
||||
|
||||
func (c *Config) ValidateStructured() ValidationResult {
|
||||
var result ValidationResult
|
||||
result.Valid = true
|
||||
|
||||
if c.Library != nil {
|
||||
if err := c.Library.Validate(); err != nil {
|
||||
result.Valid = false
|
||||
result.Errors = append(result.Errors, ValidationError{
|
||||
Field: "Library.DirectoryPath",
|
||||
Message: err.Error(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
```
|
||||
|
||||
## 9. Use Standard Library for Config Paths
|
||||
|
||||
The path construction in `system/userdata.go` doesn't respect `$XDG_CONFIG_HOME` on Linux or use the standard Go `os.UserConfigDir()`.
|
||||
|
||||
**Current implementation:**
|
||||
```go
|
||||
case "linux":
|
||||
return fmt.Sprintf("/home/%s/%s/yellowjacket", username, unixSubdirs[dt]), nil
|
||||
```
|
||||
|
||||
**Suggested improvement:**
|
||||
```go
|
||||
func GetUserConfigDirPath() (string, error) {
|
||||
baseDir, err := os.UserConfigDir() // Respects XDG_CONFIG_HOME
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("could not get user config directory: %w", err)
|
||||
}
|
||||
|
||||
path := filepath.Join(baseDir, "yellowjacket")
|
||||
|
||||
if err := os.MkdirAll(path, 0o755); err != nil {
|
||||
return "", fmt.Errorf("could not create config directory: %w", err)
|
||||
}
|
||||
|
||||
return path, nil
|
||||
}
|
||||
```
|
||||
|
||||
This approach:
|
||||
- Respects `$XDG_CONFIG_HOME` on Linux
|
||||
- Uses proper macOS paths (`~/Library/Application Support`)
|
||||
- Uses `%AppData%` on Windows
|
||||
- Is more portable and follows platform conventions
|
||||
@@ -1,53 +0,0 @@
|
||||
# Development Overview
|
||||
|
||||
YellowJacket is a moderately complex application. This document gives an overview of how development of it works.
|
||||
|
||||
## Logical Breakdown
|
||||
|
||||
YellowJacket can be thought about in a heirarchy of logical modules and components. The borders of these logical sections are mostly represented in the code and directory structure as well.
|
||||
|
||||
- Frontend
|
||||
- UI Components (see [Lit](###lit-web-components))
|
||||
- Backend
|
||||
- App
|
||||
- Asset Handler
|
||||
- Logging
|
||||
- System
|
||||
- Player
|
||||
- Library
|
||||
- Config
|
||||
- Database
|
||||
- Queries (see [sqlc](###sqlc))
|
||||
|
||||
## Dependencies
|
||||
|
||||
YellowJacket uses many tools and libraries to provide its functionality.
|
||||
This section lists each of these dependencies and explains how they are used.
|
||||
|
||||
### [Wails](https://wails.io)
|
||||
|
||||
Used to create desktop apps with Go and web technologies.
|
||||
|
||||
### [SQLite](https://github.com/mattn/go-sqlite3?tab=readme-ov-file#go-sqlite3)
|
||||
|
||||
Used for local database.
|
||||
|
||||
### [sqlc](https://sqlc.dev/)
|
||||
|
||||
Used to generate Go code from SQL.
|
||||
|
||||
### [Templ](https://templ.guide/)
|
||||
|
||||
Used to generate HTML templates with Go code.
|
||||
|
||||
### [Beep](https://github.com/gopxl/beep?tab=readme-ov-file#beep)
|
||||
|
||||
Used for audio playback.
|
||||
|
||||
### [Lit Web Components](https://lit.dev/)
|
||||
|
||||
Used for dynamic/reactive frontend components.
|
||||
|
||||
### [HTMX](https://htmx.org/)
|
||||
|
||||
Used for requesting HTML fragments from the backend and rendering them on the frontend.
|
||||
-1648
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user