Add agent instructions for Jamulus - #3785
Conversation
|
|
I agree. I'm happy for AI to show me how to do something, but then I re-implement the technique myself. I wouldn't want to include code that I don't understand how or why it works! |
|
Yes sure. But I still think that if the code is understood and well written it's fine. But yes, we need to update CONTRIBUTING.md |
|
My experience has been that an agent should author this kind of guidance, and the agent should be asked to evolve the guidance when an agent makes an error and then recovers from the error with or without human intervention ("a teachable moment"). I say something like, "You made an error while fixing/making this, and then we recovered from the error. Let's add guidance to AGENTS.md that would help prevent this kind of error next time." I asked Fable to review this PR against my own CLAUDE.md (basically same function as AGENTS.md). This is primarily because a model like Fable can produce great guidance that a lesser model like Sonnet can follow. Its overall view is: good foundation, accurate throughout, worth merging after additions. Here are 5 observations it provided:
My view on this: This is some pretty technical, fundamental stuff, and I think I know why it mentions this. I had added a mechanism to my servers to keep out known attack vectors (ASNs and a few IP ranges), but my lookup was synchronous, and every 20 seconds, the whole server would freeze for 100ms, for everyone. People wouldn't stay. :) If it was easy-breezy for me to introduce that problem, perhaps it's basic guidance for any agent to avoid making such a blunder.
My view: Maybe this guidance gets too far into the weeds in some ways, or maybe not. When your project has non-negotiable architectural laws, it's good to include them in tose terms. I started with a "CONSTITUTION" in the earliest comments of a script file, with numbered LAWS. The location (first comments, or AGENTS.md) assures these always get read, and fundamentally unwise changes just don't happen (because they'd break the LAW).
My view: I agree. Spell out what it should do.
My view: Perhaps so. And notice that AGENTS.md can and probably should link through to CONTRIBUTING.md and any other pre-existing guidance. Agents will investigate these hyperlinked guidance Markdown files, but AGENTS.md (or CLAUDE.md) are considered the start points. So we'd link from AGENTS.md to COMPILING.md, CONTRIBUTING.md, SECURITY.md, and I guess README.md. this is exactly the kind of upgrade an agent can perform for us, and if we add AGENTS.md, I can ask Fable for an agent-ready makeover of these files, with AGENTS.md as the root file.
Maybe pick out the fixes or ideas you find most compelling for this PR and merge it, and we'll start evolving all of our guidance Markdown content over time, especially when we see a real-world error that could have been avoided by a little tip in AGENTS.md. Regarding the larger discussion, I am also concerned that it's now easy to produce PRs, but it's ultimately a blessing, even if it means we need vigilant gatekeeping by humans. My fork of Jamulus has a CLAUDE.md that might interest you. One thing I notice is all those architecture pitfalls. I've found different library requirements for deployment to Ubuntu 22 vs. 24 vs. 26, on ARM vs. x86, as well as gotchas when on very small (half-gb) RAM scenarios. |
This was done with open code and corrected by hand. I instructed it to be brief also as I wanted to make sure that small (local) models can also benefit from it. |
|
This YouTube video explains why small models running locally aren't suited to agentic programming tasks, but AGENTS.md is typically loaded into context before every interaction with an agent while using any model, so concision is always in style. |
|
Qwen3 27b works for easy tasks. Maybe that counts as medium sized. Will check the video soon. |
Let me put it this way, none of my local machines is capable of running a local LLM (well, maybe above 2b). Anything in the Jamulus source directory that is going to mislead Cursor or VSCode running against cloud instances is going to have negative impacts - and reducing the scope of what the LLM considers as potential context is as good as misleading it. |
|
Agree. Also I believe context must be as concise and information dense as possible. |
Let's be as concise and dense as possible, but go as deep as we must to prevent common errors and pitfalls.
LLMs will "bridge the gap" and "fill in the blanks" which is fascinating, but the goal of AGENTS.md is to prevent speculations that just lead to failures and problems. I'd like to see a culture here where people submit Markdown PRs for agent use that (concisely and densely) would have prevented the pitfalls they experienced when agentically programming this repo. |
my little rant: |
|
Agree. ARCHITECTURE.md or something comparable should exist - also for humans - in docs. Comparable to the protocol. |
Only if it's clear that the contributor doesn't have understanding of the code. If they know what it does I'm definitely fine with it. |
We will only find out once the PR is open and time has been spent with the code. This should be mitigated and I don't know how other than ban PRs written by LLMs completely. |
|
A robust application of LLM tactics would give you the time protection you want. A submitter with little understanding of the code might drop the hot feature of the century. Today it'll show up alongside 5 loser PRs. Historically we implicitly thought of the submitter as the first reviewer, and that part isn't true anymore. We maintain the gate where the approver needs to really believe in the change regardless of the submitter reputation or apparent understanding. And we know that's not enough, so we insist on two approvers. If we had robust AGENTS, COMPILING, PROTOCOL, and ARCHITECTURE Markdown, the hot feature of the century would show up, and the other 5 wouldn't. Or maybe 1 of those 5 would be redirected by the agent into something worthwhile. So you only need to consider 2 PRs rather than 6. So your thoughtful comments here wouldn't have been necessary, because the agent would have nudged me here before I submitted anything. It's much easier to submit a PR now, and this has led to PRs of dubious merit. AI with robust coding guidance would radically improve the quality of each PR. AI triggered the new problem we've been seeing, but it's also the mitigation you're hoping for. |
I don't think we should accept any generated code, unless we generate it ourselves from a given set of rules. When there is no author we don't need one at all so a corresponding commit makes clear it wasn't contributed by a human. |
|
For the port TODO: d46a77a is one commit behind — the final version after all the review rounds in #3789 is 22 lines: https://github.com/mcfnord/jamulus/blob/59a167fb0d9d12af5f547a16d9c2ddbbf7fbddbe/AGENTS.md It has the granularity requirement as hard rules at the top (one PR per new file / independent fix / typo; every PR a standalone option reviewable in minutes), the renumbering rule removed, and everything else as links. Two of the linked files exist only if their PRs merge — drop the docs/ARCHITECTURE.md entry unless #3791 lands and the docs/DEPLOY.md entry unless #3790 lands. Take any or all of it. |
|
To give some commercial perspective: the company I work for expects us to:
Now, for myself, I will state you get sloppy. I've been working on areas of the system at work where I have no prior in depth knowledge (but I generally know my way around). However:
Even with limited context, getting a semi-decent code review adds value. The main problem is it all costs money... On my other hobby projects, given I simply can't justify investing in capable hardware to run AI locally, I can't afford to use more than the occasional bit of low grade assistance. And the difference in performance between the models I use at work and those I use at home is huge. So I do think using AI has great merit. But it doesn't take responsibility away from the person raising the PR to know what they're doing. |
|
I pay about $90/mo for my Jamulus hobby, broken into 3 similar cost buckets: LLMs for user content (welcome messages and essays about recent events), agentic programming, and about 30 servers. Understanding costs seems to be an important skill as software engineering moves from low-capital to a higher-capital activity. The good news is that frontier models can be found at reasonable prices, perhaps just for now. Fable is available within the fixed $20/mo subscription for a few more days. There's continued talk about price spikes due to ending subsidies designed to attract userbase, but I haven't seen a spike trend yet. I'm hearing the Codex has price performance that exceeds Fable, so I plan to try it. Some say we should tokenmax now because it might not be affordable later. This PR discussion sent me off on a whirlwind experiment using Fable within GitHub in a group setting. I was curious what would happen. Observations:
I describe this experience as dizzying. So many claims, all carefully and thoughtfully cross-linked, readable as individual chunks, but overwhelming as a whole. And much if it appears under my name, kind of leaving me as a party to a whirlwind of interlocking proposals and claims. When I review it, I have the impression that it's helpful. It's navigating and managing the complexity, and humans are jumping in as hoped. LLM technology can be confident about counter-productive nonsense. So can people, but LLMs can produce a lot more of it. An unskilled operator can choose the wrong model and get a bunch of hogwash that sounds right. For this experiment, I used Fable at High effort, and that's a level where the product seems to meet the bar for submissions in this repo. Expanding AGENTS.md guidance around PR granularity and code formatting will improve results. Going forward, I will consider doing this again, but will also do my best to double-check everything it does, and monitor its successes and failures. Subjectively it feels like progress, but I'm not sure that's the right word for what it is. It would be bad if it was just the illusion of progress. It would also be bad if it was 50-50 right and wrong work. But if it's 80% helpful, that's as good as most humans. I have a personal project where I ask the agent to improve code that is just barely within my mental grasp. The algorithm involves a lot of statistics and real-time data. When I barely understand the code changes it recommends, that's when I barely consent, and frequently just beg it to make the tiniest step in that direction, paired with diagnostics that will confirm or deny its hypothesis over time. If I can't check its work, at least I can ask it to check its work. In this experiment--making issues and PRs reviewed by this group--since other people are involved, I see how excessive activity could overwhelm the humans, and that's why it's interesting how Fable took pains to navigate the politics as well as the source code. I also had to remain the human-in-the-loop, or feel I should, even though I didn't modify anything it said, only clipped away some comments that didn't seem necessary. I think everyone will need to develop their own personal philosophies about when to turn this technology loose, and when a human needs to gate automated work. |
This comment was marked as outdated.
This comment was marked as outdated.
CONTRIBUTING.md: point agent-assisted contributors at AGENTS.md. - Contributors remain accountable for every line of AI-generated code. Co-Authored-By: jrd <gh26.x.jrd@xoxy.net> Co-Authored-By: Peter L Jones <pljones@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Revised and "merged". Needs re-review. |
|
@mcfnord I gave some more context and a merge prompt to my opencode install. Please recheck for conciseness. |
| ## Qt / portability | ||
|
|
||
| - Minimum Qt: **5.12.2**. Qt 6.x.y recommended. Guard newer APIs with `#if QT_VERSION >= QT_VERSION_CHECK(...)`. | ||
| - C++11 (C++17 on Android for Oboe's `std::timed_mutex`). |
There was a problem hiding this comment.
| - C++11 (C++17 on Android for Oboe's `std::timed_mutex`). | |
| - C++11 (C++17 on Android). |
| - C++11 (C++17 on Android for Oboe's `std::timed_mutex`). | ||
| - iOS builds require Qt 5.15 or later (Qt 6 is buggy for iOS). | ||
| - Preserve platform support. Don't break Android/iOS builds even if unofficial. | ||
| - Supported desktop: Windows 10+, macOS 10.10+, Ubuntu 20.04+/Debian 11+. |
There was a problem hiding this comment.
I think some high-level guidance for contributors is needed somewhere other than in the AutoBuild workflow itself. Whether that's here or somewhere else, though, is open to debate. Probably somewhere else?
There was a problem hiding this comment.
This is the right level of specificity in the right place (AGENTS.md, aka the Bible). The plusses provide a lot of future-proofing. I want every agent to remember these platform commitments every time it considers doing something to the code. Omitting this guidance is inviting agents to forget about a platform in a solution attempt, or to guess which platforms we support. If/when Qt 6 becomes viable, we will be happy to update AGENTS.md to say so.
There was a problem hiding this comment.
OK. Sounds reasonable.
Do we have an expanded version with the history behind each decision written down, I wonder? We should -- though that isn't for here.
|
MY LLM WROTE: @ann0see Rechecked as asked. First: I verified every factual claim in the file against Four cuts, in descending order of size. Together they take it from 79 lines / 506 words to 58 / 405 — 23% shorter with no fact removed, because every cut line is either duplicated inside the file or lives in a file this one already links to. 1. The Build block is a copy of
2. Line 54 (the clang-format rules list) earns nothing. 3. "Smallest change" is stated three times — line 5 (bold), line 65 ("Small changes preferred, every change tested"), line 71–72 ("One logical change per PR… Smallest change possible?"). "Tested" likewise appears at lines 36, 65 and 72. Repetition in a file that gets prepended to every prompt costs context on every single call. It's already stated best at line 5, bold, above the fold. Suggest merging 4. Inline "See X" duplicates the closing list. One accuracy nit, unrelated to length: line 55 says pre-3.12.1dev code is "GPL 2.0+ (dual-licensed as part of Jamulus)". I have the full trimmed version in one piece if you want it as a suggestion batch or a PR against |
|
Thanks. The macOS specific build was always missed by agents in open code -> must be there The make clang format is probably enough |
| Generally use: | ||
| ```bash | ||
| git submodule update --init # required: oboe (Android) | ||
| qmake && make # Linux (use qmake-qt5 on Fedora) |
There was a problem hiding this comment.
I usually run a make distclean (which is allowed to error if the Makefile's gone) to start completely afresh.
| ```bash | ||
| git submodule update --init # required: oboe (Android) | ||
| qmake && make # Linux (use qmake-qt5 on Fedora) | ||
| qmake "CONFIG+=headless serveronly" && make # headless server |
There was a problem hiding this comment.
I usually run make clean after I've finished working.
(One thing this means is I've noticed how messy the Android build leaves the working directory...)
There was a problem hiding this comment.
MacOS and iOS are also messy.
There was a problem hiding this comment.
OK, I guess this is another housekeeping task we need on a new PR.
I'm envisaging:
- checkout the git repo to a fresh directory
- new temporary step:
- store the directory listing
- run all the build steps
- new temporary step:
make clean(or platform-appropriate command)- store the directory listing again
- compare the two directory listings
Then pass the cruft listing to an LLM to raise a PR to fix the "make clean" scope.
| - **CI uses clang-format** (check `.github/workflows/coding-style-check.yml` for version). | ||
| - CI runs **shellcheck + shfmt** on `.sh` files; **pylint** (config: `.pylintrc`) on `.py` files in `tools/`. | ||
| - Rules: tabs→4 spaces, braces on own line, space inside `()` and around `if/for/while`, column limit 150, left pointer alignment (`int* p`). | ||
| - New files: AGPL 3.0+ license header. Pre-3.12.1dev code: GPL 2.0+ (dual-licensed as part of Jamulus). |
There was a problem hiding this comment.
GPL 2.0+ -> we've taken the "or later" and moved everything to GPL 3.0+ as of 3.12.1dev.
So you'd have to git reset --hard before that version to checkout older code.
Also the intent is stated that all new contributions are AGPL 3.0+, not only to new files. It's only the code that was there prior to the move to AGPL3.0+ that is now GPL3.0+.
| ## PR expectations | ||
|
|
||
| - Small changes preferred, every change tested | ||
| - Branch `autobuild/<name>` triggers CI builds on your fork. |
There was a problem hiding this comment.
(The "/" isn't actually needed by the regex, IIRC -- anything autobuild.*)
|
|
||
| - Small changes preferred, every change tested | ||
| - Branch `autobuild/<name>` triggers CI builds on your fork. | ||
| - Follow `.github/pull_request_template.md`. Include `CHANGELOG:` line with changelog description. For new deps/build changes, add `AUTOBUILD: Please build all targets`. |
There was a problem hiding this comment.
Might be worth stating which targets are in without this and which this adds. Presumably the occasional fix to actual code (not deps / build process) will need this.
| ## Before opening a PR | ||
|
|
||
| One logical change per PR — no unrelated cleanup or reformatting of untouched code. Discuss features in an issue before implementing. See `CONTRIBUTING.md`. | ||
| Builds? Tested? **Smallest change possible?** |
There was a problem hiding this comment.
Add Self-reviewed against "Priority order" above? maybe.
5 participants
Short description of changes
Adds agent instructions for opencode/claude code/... since we have an increasing amount of agent/AI PRs.
CHANGELOG: SKIP
Context: Fixes an issue?
No
Does this change need documentation? What needs to be documented and how?
This is documentation
Status of this Pull Request
Ready for review.
What is missing until this pull request can be merged?
Review for conciseness. @mcfnord had AI contributions - I assume that he knows if this prompt is good enough.
Checklist