lobo/zfin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

update AGENTS.md

Browse source
This commit is contained in:
Emil Lerch 2026-06-02 12:49:58 -07:00
parent d7a86cd639
commit b7b492e494
Signed by: lobo
GPG key ID: A7B62D657EF764F8
1 changed files with 84 additions and 303 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

387
AGENTS.md
View file

@ -1,45 +1,13 @@
# AGENTS.md
This file is zfin's source-of-truth for assistant agents. It
holds project-specific architecture, conventions, and gotchas
plus the universal hard rules that apply on top.
Read the ABSOLUTE PROHIBITIONS section first.
## ⛔ ABSOLUTE PROHIBITIONS — READ FIRST ⛔
### Zig 0.16.0 reference — read the release notes
This codebase is on Zig 0.16.0. The 0.16 release was a major
I/O-as-an-interface refactor that reshaped the standard library.
Before making non-trivial changes (especially anything touching
`std.Io`, `std.fs`, `std.process`, `std.http`, `std.Thread`,
allocators, or `std.time`), **read the release notes** at:
https://ziglang.org/download/0.16.0/release-notes.html
Key migrations that bit us repeatedly during the 0.16 upgrade and
will bite future work too:
- `std.io` → `std.Io` (namespace rename, no deprecation alias).
- `std.fs.cwd()` → `std.Io.Dir.cwd()`. All file ops take `io`.
- `std.process.Child.init(argv, alloc)` → `std.process.spawn(io, .{...})`
or `std.process.run(gpa, io, .{...})`.
- `std.time.timestamp()` → `std.Io.Timestamp.now(io, .real).toSeconds()`.
- `std.Thread.sleep(ns)` → `std.Io.sleep(io, duration, clock)`.
- `pub fn main` gains a `std.process.Init` parameter ("Juicy Main");
provides pre-built `gpa`, `io`, `arena`, `environ_map`.
- `std.heap.GeneralPurposeAllocator` → `std.heap.DebugAllocator`.
- `std.heap.ThreadSafeAllocator` removed; `ArenaAllocator` is now
lock-free thread-safe, `DebugAllocator` is thread-safe by default.
- `std.mem.trimRight`/`trimLeft` → `trimEnd`/`trimStart`.
- `std.mem.indexOf*` → `find*` (deprecation aliases still present,
so old names work but warn).
- `std.testing.refAllDeclsRecursive` removed. Only `refAllDecls`
remains. We use the bare `std.testing.refAllDecls(@This())` in
`src/main.zig`'s test block — it sema-touches every top-level decl,
which transitively pulls in every imported file's `test` blocks.
No local reimplementation is needed. See "Test discovery" below.
- `std.fs.File.readToEndAlloc(alloc, N)` → two-step:
`file.reader(io, &.{})` then `.interface.allocRemaining(alloc, .limited(N))`.
For anything not on this list, **read the release notes first.**
The notes are long but they're organized by section; searching for
the specific symbol you're migrating is fast.
### `io` vs `today` / `now_s` — design rule
@ -203,143 +171,15 @@ Add tests in the same file. Money helpers belong next to
the other tests in `Money.zig`; date helpers belong next to
`yearsBetween`'s tests.
### NEVER invoke ripgrep. EVER.
**Do not run `rg` in the Bash tool.** Not for open-ended search, not for
counting matches, not for "just this one quick check", not ever. Running
ripgrep on this machine hammers the filesystem badly enough to degrade the
whole system — this is a recurring, reproducible problem, not a hunch.
**Use instead:**
- **Grep tool** (built-in) for content search. It handles regex, file
globs, and output shaping without spawning `rg`.
- **Glob tool** (built-in) for finding files by name pattern.
- **Read tool** for reading files (with `offset`/`limit` for large ones).
- Plain `grep` via the Bash tool is acceptable when the built-in Grep
tool can't express what you need — but prefer the built-in first.
**If you catch yourself typing `rg` in a Bash command:** stop, delete it,
use the Grep tool instead. The fact that `rg` is faster in the abstract
does NOT matter here. This machine's filesystem + ripgrep's parallelism
is a bad combination, full stop.
**This applies to every variant:** `rg`, `ripgrep`, piping through
`rg`, backgrounded `rg`, `rg --files`, etc. All banned.
### NEVER delete or modify build caches. EVER.
**This means:**
- **NEVER** run `rm -rf .zig-cache` or `rm -rf .zig-cache/*` or any variant.
- **NEVER** run `rm -rf ~/.cache/zig` or touch anything under `~/.cache/zig/`.
- **NEVER** touch `~/.cache/zls/` or any other tool cache.
- **NEVER** suggest deleting the cache as a "fix" — it is not a fix, it is
damage. Deleting `.zig-cache` while ZLS or another `zig build` is running
creates a corrupt state where the build runner's expected cache entry
(`.zig-cache/o/<hash>/build`) references a path that no longer exists,
producing the error `failed to spawn build runner .zig-cache/o/<hash>/build:
FileNotFound`. Recovering from this on the affected machine requires
killing every concurrent `zig` process (including ZLS) and waiting for
filesystem state to re-stabilize — it is NOT a simple retry.
- **NEVER** touch the cache "just to force a rebuild". Zig's cache is
content-addressed. It does not get stale in a way that deletion fixes.
If a build result looks wrong, the bug is in the source, not the cache.
Use `touch src/somefile.zig` if you truly need to invalidate one file's
cache line. Do not nuke the whole directory.
- **NEVER** suggest a "different cache directory" as a workaround without
an explicit, specific reason and explicit user approval. `--cache-dir`
and `--global-cache-dir` flags exist; they are not toys.
**If a test result seems wrong or cached incorrectly:** the answer is
ALWAYS to investigate the source code or build graph, not to delete
cache. See "Test discovery" below — 99% of the time the "cached wrong
result" is actually a test discovery problem, not a cache problem.
**If you find yourself typing `rm -rf` anywhere near a cache path: STOP.
Ask the user instead.**
### NEVER run destructive git operations without explicit permission.
- No `git reset --hard`, `git clean -fdx`, `git push --force`, `git checkout .`
on files with uncommitted work, unless the user asks for that specific
operation by name.
### NEVER run `git stash`. EVER.
- **`git stash` is banned outright.** Not `git stash push`, not `git stash --keep-index`,
not `git stash pop`, not `git stash apply`, not even read-only `git stash list`.
No variant. No "just to test something." No "I'll pop it right back."
- The reason: `git stash pop` can conflict on overlapping lines and leave
unresolved conflict markers in the working tree. The recovery requires
hand-resolving the markers, which trashes whatever curated index state
the user had in flight. This has bitten the user before and it's bitten
them again because of me. The rule is absolute now.
- If you think you need `git stash` to verify something (e.g. "does the
staged-only state build cleanly in isolation?"), the answer is: **don't
verify it that way.** Either:
- Read `git diff --cached` and reason about whether the staged hunks
are coherent on their own, OR
- Ask the user to verify after they commit, OR
- If verification is critical, ask the user to do the stash themselves
with their tools — but recommend against it because of this rule.
- **There is no exception clause for `git stash`.** Not even "the user
said it's OK this once" — the previous "one-time exception" for git
staging operations is what led to the stash incident. Direct exceptions
for `git add -p` / `git restore --staged` for staging-management remain
permitted with explicit user approval, but `git stash` is permanently
off-limits regardless of consent.
### NEVER run `git add`, `git commit`, or `git push`. EVER.
- **The user commits. You do not.** Do not stage files. Do not create commits.
Do not amend commits. Do not push. Do not suggest running these commands
yourself "to save a step". This includes `git add -p`, `git add .`,
`git add <file>`, `git commit -m ...`, `git commit --amend`, `git push`,
and any `gh pr create` that would auto-stage or auto-commit.
- If you are tempted to run any of these because "the work is done and it
seems logical to commit" — STOP. The user has a review-and-commit workflow.
Your job ends at a clean working tree with the changes ready to review.
- The ONLY exception is when the user says, verbatim in the current turn,
"commit this" / "make a commit" / "push it" / similar direct imperative.
Do not extrapolate from earlier intent, a plan that mentioned milestones,
or any indirect signal. If in doubt, ask — don't commit.
- When a milestone plan says "STOP POINT — user reviews and commits": you
stop. You do not commit. You do not prepare a commit. You hand off the
working tree and wait.
### NEVER write to `/tmp`. Use `$(cwd)/.tmp` instead.
- **`/tmp` is off-limits for any temporary file the assistant creates.**
Not for scratch scripts, not for redirected stdout, not for `cp`
backups, not for "just for a second" experiments. The user's workflow
treats `/tmp` as their own space; assistant-created files there
pollute it across sessions and hide from `git status`.
- Use `$(cwd)/.tmp/` (a `.tmp` directory at the repository root) for
every transient file. `mkdir -p .tmp` if it doesn't exist;
`.gitignore` it (or rely on a project-level `.gitignore` rule that
catches it). Files in `.tmp` show up in `ls` next to the work,
surface in `git status` if the ignore rule is wrong, and get
cleaned up at the end of the task with a single `rm -rf .tmp/*`.
- This applies to:
- Bash redirections: `cmd > .tmp/out` not `cmd > /tmp/out`.
- Test fixture scratch files.
- `cp file /tmp/file.bak` for "I'll restore in a sec" backups —
use `cp file .tmp/file.bak` instead.
- Heredoc-created throwaway scripts to test a Zig snippet
(`zig run /tmp/foo.zig` → `zig run .tmp/foo.zig`).
- The ONE exception: temp files created by the system / by other
tools (mise installations, zig's own cache symlinks, etc.) that
legitimately use `/tmp`. Don't try to redirect those.
### Documentation-file conventions
- **`TODO.md` holds only what's still open.** Git already tracks what was
done and when. Do NOT add "DONE" markers, completion status, strikethrough,
or "shipped in …" blurbs to TODO entries — just delete the section when
the work is finished. If follow-up work came out of the finished task,
add it as a new top-level section; don't leave the parent entry around
as a historical wrapper.
- **`TODO.md` holds only what's still open.** Git already tracks
what was done and when. Do NOT add "DONE" markers, completion
status, strikethrough, or "shipped in ..." blurbs to TODO
entries. Just delete the section when the work is finished. If
follow-up work came out of the finished task, add it as a new
top-level section; don't leave the parent entry around as a
historical wrapper.
- **`REPORT.md` is untracked on purpose.** It's a personal workflow doc
living in the repo root only until it moves to the user's portfolio
directory. Edit it freely when asked; don't treat it as part of the
@ -347,140 +187,84 @@ Ask the user instead.**
### Lint warnings — there are no "pre-existing" warnings
**zlint warnings get fixed, period.** Do NOT excuse a warning by
saying it was "pre-existing in the file" or "inherited from a copy"
or "the same style as elsewhere." That's how lint debt accumulates
to the point where the pre-commit hook (`zlint --deny-warnings`)
becomes a tripwire that everyone routes around.
**Lint warnings get fixed, period.** Do NOT excuse a warning by
saying it was "pre-existing in the file" or "inherited from a
copy" or "the same style as elsewhere." That's how lint debt
accumulates to the point where the deny-warnings flag on the
pre-commit hook becomes a tripwire that everyone routes around.
The rule:
1. **Before any commit, run zlint on every file you touched in
the change.** If zlint reports any warnings on those files,
fix them in that change. There is no "I didn't introduce it,
not my problem" — once you've touched the file, the warnings
are yours.
1. **Before any commit, run zlint on every file you touched
in the change.** If zlint reports any warnings on those
files, fix them in that change. There is no "I didn't
introduce it, not my problem." Once you've touched the file,
the warnings are yours.
2. **If you find pre-existing warnings in a file you didn't
intend to touch (e.g. you ran zlint across the whole tree
for a sanity check), fix them in a SEPARATE commit and call
that out to the user explicitly so they can keep the commits
clean.** Do not silently bundle drive-by lint fixes into a
feature commit; the diff becomes harder to review and the
lint-debt origin gets buried.
intend to touch** (e.g. you ran zlint across the whole
tree for a sanity check), fix them in a SEPARATE commit and
call that out to the user explicitly so they can keep the
commits clean. Do not silently bundle drive-by lint fixes
into a feature commit.
3. **Common warning kinds and the right fix:**
- `suppressed-errors` (`catch {}` / `catch "fallback"`):
replace with `try` (propagate to the caller). If the call
genuinely cannot propagate (e.g. an stderr write inside an
error-reporting path where the secondary error doesn't
matter), use `catch |err| std.log.debug(...)` or rewrite to
not need the catch. The `catch {}` form is almost never the
right answer.
- `unsafe-undefined`: add a `// SAFETY: <reason>` comment on
the line with `undefined` explaining why it's safe (e.g.
"buffer immediately overwritten by bufPrint below").
- `unused-decls`: delete the decl. Don't leave dead imports
or constants around "in case."
3. **Never report a lint result by saying "0 errors, N warnings,
but they're all pre-existing."** Either fix the warnings in
this change OR report "0 errors, 0 warnings on the files I
touched. The wider tree has N warnings I haven't addressed
in this change; flagging for follow-up" — and only after
you've confirmed by file that none of the wider-tree warnings
are in files you modified.
4. **Never report a lint result by saying "0 errors, N warnings,
but they're all pre-existing."** Either:
- Fix the warnings in this change (preferred when N is small
or the file is yours), OR
- Report "0 errors, 0 warnings on the files I touched. The
wider tree has N warnings I haven't addressed in this
change; flagging for follow-up" — and only after you've
confirmed by file that none of the wider-tree warnings
are in files you modified.
zfin uses `zlint --deny-warnings` on the pre-commit hook. The
common zlint warning kinds and the right fix:
The `--deny-warnings` flag is on the pre-commit hook for a
reason: every warning is a real signal that the codebase asked
the linter to flag and someone hasn't dealt with. Treat them as
errors at write-time, not as background noise to ignore.
- `suppressed-errors` (`catch {}` / `catch "fallback"`):
replace with `try` (propagate to the caller). If the call
genuinely cannot propagate (e.g. an stderr write inside an
error-reporting path where the secondary error doesn't
matter), use `catch |err| std.log.debug(...)` or rewrite to
not need the catch. The `catch {}` form is almost never the
right answer.
- `unsafe-undefined`: add a `// SAFETY: <reason>` comment on
the line with `undefined` explaining why it's safe (e.g.
"buffer immediately overwritten by bufPrint below").
- `unused-decls`: delete the decl. Don't leave dead imports
or constants around "in case."
### Em-dash usage — ASK FIRST
If you're about to write an em-dash (`—`) anywhere (code, tests, doc
comments, commit messages, AGENTS.md prose), **stop and check whether a
regular ASCII hyphen (`-`) would do.** Most of the time it would. Em-dashes
look nice in prose but they create real problems:
- **In tabular output / TUI cells**, `—` is 3 bytes / 1 display column.
Zig's `{s:>N}` formatter pads by byte count, so any column containing
em-dashes will be 2 visual columns short per em-dash. We've fixed this
bug at least twice; don't reintroduce it. If you genuinely need a
multibyte sentinel for "no data", use `fmt.padRightToCols` /
`fmt.centerDash` (display-column-aware) — or just hard-code the cell
as a literal const string when the cell width is fixed and the dash
position is static (no point computing what you already know). Add
an alignment test that compares the multibyte row's `displayCols`
against an ASCII row.
- **In code identifiers and string literals**, em-dashes look fine in your
editor and break grep on the user's machine when they're searching with
ASCII `-`. If a future grep for "expected-return" or "as-of" silently
misses your "expected—return" string, that's a bug surface.
- **In commit messages and prose docs**, em-dashes are an AI tell. The
user reads commit messages and won't appreciate the codebase looking
like it was written by ChatGPT.
**Rule of thumb:**
- Use `-` (ASCII hyphen) for: ranges (`1-5y`), compound modifiers
(`forecast-vs-actual`), CLI flag names (`--return-backtest`), code
identifiers, and string concatenation in tables.
- Use `—` (em-dash) only when you're displaying it to the user as a
meaningful sentinel (e.g. "no data" cell in a table), AND you've handled
the display-column padding correctly.
- If you find yourself reaching for `—` in prose for a parenthetical
aside, **switch to a regular dash, comma, or parens.** Em-dashes have
become a stylistic AI tic and the user has explicitly asked to keep
them out unless they earn their place.
When in doubt, **ask**. A one-line "I'm about to use `—` here for X, OK?"
is much cheaper than reverting after the user notices.
### Errors carry information — never throw it away
When you catch an error, the caller's first question is **"why did
this fail?"** A user-facing error message that says only
When you catch an error, the caller's first question is **"why
did this fail?"** A user-facing error message that says only
`"FetchFailed"` or `"could not parse portfolio file"` is failing
the user — they have to read source code to figure out what
happened. Three habits cause this:
**(1) Bare `catch {}` and `catch return error.X`.** The error
value carries the upstream's classification (`RateLimited`,
`Unauthorized`, `NotFound`, `ParseError`, etc.). Discarding it
reduces N distinct conditions to one opaque "something went
wrong." Fix: capture as `catch |err|` and at minimum include
`@errorName(err)` (or `{t}` format) in any user-visible message.
Better: re-raise a more specific error variant from the caller's
error set so the next layer up can decide what to do.
**(1) Bare `catch {}` and `catch return error.X`.** Capture as
`catch |err|` and at minimum include `@errorName(err)` (or `{t}`
format) in any user-visible message. Better: re-raise a more
specific error variant from the caller's error set.
**(2) Generic `error.X` collapse points in service-layer code.**
**(2) Generic `DataError.FetchFailed` collapse in service.zig.**
When `service.zig:getX` does `provider.fetchX(...) catch return
DataError.FetchFailed`, the provider's specific error
(`RateLimited` vs `Unauthorized` vs `NotFound`) is permanently
erased. Fix: either expand the service-layer error set
(`DataError`) to mirror the provider distinctions, OR pass the
inner error through via `catch |err| { log.warn(...
@errorName(err) ...); return ...; }` so the stderr path tells
the user something useful even if the typed return value is
collapsed.
erased. Fix: either expand `DataError` to mirror the provider
distinctions, OR pass the inner error through via `catch |err| {
log.warn(... @errorName(err) ...); return ...; }` so stderr
tells the user something useful even when the typed return value
is collapsed.
**(3) User-facing stderr messages that don't say what the actual
error name was.** A CLI command that catches an error MUST print
`@errorName(err)` (or a deliberate human translation per error
kind) in the stderr message. `"Error: Failed to fetch data for
symbol"` is unhelpful. `"Error fetching AAPL: RateLimited (free
tier 5/min — wait 60s)"` is helpful. The error name alone is
enough for the user to look it up; the human translation is a
nice-to-have on top.
**(3) User-facing stderr messages must name the error.** A CLI
command that catches an error MUST print `@errorName(err)` (or a
deliberate human translation per error kind) in the stderr
message. `"Error: Failed to fetch data for symbol"` is
unhelpful. `"Error fetching AAPL: RateLimited (free tier 5/min,
wait 60s)"` is helpful.
**The grep test:** if you're catching an error in user-facing
code, the message should mention either `@errorName` / `{t}` or
reference a specific error variant by name. `cli.stderrPrint`
calls with hardcoded "Error: failed to ..." strings and no error
context fail this test.
**The grep test:** `cli.stderrPrint` calls with hardcoded "Error:
failed to ..." strings and no error context fail this test.
### NEVER put PII in tests, fixtures, comments, or docs
@ -879,26 +663,27 @@ shape with example signatures.
### `anytype` is almost never the right answer — pause and ask first
Empirically, every time `anytype` looked necessary in this codebase
it turned out not to be. Concrete-typed parameters always worked
once we actually tried them. **Before adding any new `anytype`
parameter, stop and reconsider.** The discussion that surfaces "no,
this can be `*App` after all" is short and worth having every time.
Empirically, every time `anytype` looked necessary in this
codebase it turned out not to be. Concrete-typed parameters
always worked once we actually tried them. **Before adding any
new `anytype` parameter, stop and reconsider.** The discussion
that surfaces "no, this can be `*App` after all" is short and
worth having every time.
Common reasons people reach for `anytype` and what to do instead:
- **"I want to avoid a circular import."** Test the assumption.
Zig resolves `a.zig ↔ b.zig` cycles fine in most cases — file
structs are evaluated lazily, and the cycle only fails if
evaluation actually loops. Just write the concrete type and run
`zig build`. If it fails, the answer is usually to extract the
shared type to a third file, not to weaken the contract with
`anytype`. (See: the `tab_framework.zig` ↔ `tui.zig` cycle —
caught me once; turned out Zig handled it cleanly.)
evaluation actually loops. Just write the concrete type and
run `zig build`. If it fails, the answer is usually to extract
the shared type to a third file, not to weaken the contract
with `anytype`. (See: the `tab_framework.zig` ↔ `tui.zig` cycle
— caught me once; turned out Zig handled it cleanly.)
- **"This function is genuinely polymorphic over many types."**
In Zig, the right shape for runtime polymorphism is usually
`*anyopaque` + an explicit cast at the boundary, paired with
a vtable struct of fn pointers. That's harder to abuse than
`*anyopaque` + an explicit cast at the boundary, paired with a
vtable struct of fn pointers. That's harder to abuse than
`anytype` and the cast site documents the type contract
unambiguously. Compile-time polymorphism over a known set of
types is better expressed with `comptime T: type` parameters.
@ -910,8 +695,8 @@ Common reasons people reach for `anytype` and what to do instead:
the method.
- **"It's a test helper that takes any struct."** This is the
one case where `anytype` is sometimes OK — generic test
utilities like `std.testing.expectEqual`. But check whether
a concrete type would do.
utilities like `std.testing.expectEqual`. But check whether a
concrete type would do.
The real cost of `anytype` is that it punches a hole in the type
contract: the compiler can't catch wrong-shape arguments at the
@ -977,7 +762,3 @@ command.
| [SRF](https://git.lerch.org/lobo/srf) | Cache file format, portfolio/watchlist parsing, serialization |
| [libvaxis](https://github.com/rockorager/libvaxis) (v0.6.0) | Terminal UI rendering |
| [z2d](https://github.com/vancluever/z2d) (v0.11.0) | Pixel chart rendering (Kitty graphics protocol) |
## Build system rules
- **Never use `addAnonymousImport`** in `build.zig`. Always use `b.addModule()` + `addImport()`. Anonymous imports cause "file belongs to multiple modules" errors and make dependency wiring opaque.