Move coordination files to .amplifier/, stop capturing raw tool output, unskip the test suite - #8
michaeljabbour wants to merge 3 commits into
Conversation
…context The coordination files no longer sit in the repo root. They move to `.amplifier/project-context/` — the hidden per-repo directory Amplifier already owns for local state, and the directory `@project:` already resolves to, so the files gain a mention namespace for free. Nothing breaks before migration: both hooks probe the hidden location first and fall back to a legacy top-level `project-context/` at each level of the walk. A new `context_dir` config knob (absolute, or relative to the repo root) overrides both, and scaffolding now targets the hidden directory. This repo's own files move in the same commit — `git mv` keeps their history, and `.gitignore` re-includes them under the existing `.amplifier/*` ignore, since they are this repo's cross-clone memory. The resolver is duplicated in hooks-project-context and hooks-memory-briefing on purpose: sibling tool-memory imports must be ImportError-guarded per BUNDLE_GUIDE, and a guarded import whose fallback resolves *differently* is worse than two copies that agree. Test monkeypatches of `_find_project_context_dir` take `*args` now that the resolver accepts `context_dir`.
…tput
Seeds are not a labelling nicety — they are the capture filter. Content
matching no attractor is never filed, so a seed like "module" or "error"
turns every file read and every shell result into a memory.
Measured over 7 days of real sessions with the word-level seeds:
8,330 drawers, 18.2 MB, 68% raw tool output. 4,190 of those were filed
as `architecture` because source code contains the word "module", and
1,976 as `blocker` because stderr contains "error". Thirty-three
`lesson_learned` drawers were competing with that for the briefing's
1,500-token budget.
Every seed set now requires phrasing that only shows up when someone is
stating a conclusion ("we decided", "root cause", "turns out") rather
than phrasing that appears inside the artifacts being worked on. The
three copies move together — context/memory-manifest.yaml, the in-code
DEFAULT_MANIFEST, and the capture hook's built-in table — and the
parity test now compares seeds, not just ids and importance bases.
New tests assert the property directly: verbatim samples of what the
old seeds captured (`--version` output, a returncode blob, a
line-numbered file read, a "Path not found" error) must classify as
None, and no seed may be a bare common word.
`pytest_collection_modifyitems` is called once per session with every collected item, whichever conftest defines it. The integration conftest's loop had no path filter, so outside the DTU container it marked the entire repo's unit suite as skipped: a bare `pytest` at the root reported "138 skipped" and looked green while running nothing. It now skips only items collected from tests/integration/. `pytest` at the root runs 130 tests and skips the 8 integration ones. Also documents the new coordination-file location and the `context_dir` knob in README, behaviors/memory.yaml, and CHANGELOG.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc7eb2c7a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Project Coordination Files | ||
|
|
||
| This bundle integrates the **project-context** coordination system. Every project using this bundle gets a `project-context/` directory containing structured files that persist memory across sessions, across clones, and across AI tools. | ||
| This bundle integrates the **project-context** coordination system: a `.amplifier/project-context/` directory of structured files that persist memory across sessions, across clones, and across AI tools. |
There was a problem hiding this comment.
After moving coordination files here, behaviors/memory.yaml still injects context/instructions.md, whose line 47 tells agents that the handoff is at project-context/HANDOFF.md. In migrated repositories that legacy directory is absent, so the runtime prompt gives agents conflicting paths and can make session-end updates target a nonexistent file; update that companion instruction to the hidden path as part of the move.
AGENTS.md reference: AGENTS.md:L37-L41
Useful? React with 👍 / 👎.
| ```bash | ||
| mkdir -p .amplifier && git mv project-context .amplifier/project-context | ||
| # if .gitignore ignores .amplifier/, re-include the coordination files: | ||
| # !.amplifier/project-context/ |
There was a problem hiding this comment.
When a target repository ignores .amplifier/ as a bare directory pattern, adding only this negation does not re-include the coordination files because Git does not traverse an excluded parent; git check-ignore continues to report .amplifier/project-context/HANDOFF.md as ignored. The repository's own .gitignore handles this by changing the parent rule to .amplifier/*, so the migration instructions should require that form (or equivalent ancestor re-inclusions), otherwise users can migrate successfully but never commit the cross-clone memory.
AGENTS.md reference: AGENTS.md:L14-L15
Useful? React with 👍 / 👎.
|
|
||
| This project uses a coordination file system in `project-context/`. | ||
| These files give you persistent memory across sessions. **Read them before starting any work.** | ||
| This project uses a coordination file system in `.amplifier/project-context/`. |
There was a problem hiding this comment.
Making this the canonical checked-in location leaves tests/integration/conftest.py:108-113 resetting only the removed top-level project-context/, so the autouse fixture becomes a no-op during the DTU event-wiring sessions run from the bundle clone. Those sessions can modify .amplifier/project-context/PROVENANCE.md, and the fixture's documented cross-test isolation no longer prevents later model-driven tests from being steered by those edits; update both the directory check and git checkout path.
AGENTS.md reference: AGENTS.md:L14-L15
Useful? React with 👍 / 👎.
1 participant
Three fixes found by measuring a week of real sessions across this machine's projects, plus one bug found while running the tests.
1. Coordination files move to
.amplifier/project-context/They no longer sit in the repo root. The hidden per-repo directory is the one Amplifier already owns for local state, and it is exactly what
@project:resolves to — so the files gain a mention namespace (@project:project-context/HANDOFF.md) for free.Backward compatible: both hooks probe the hidden location first and fall back to a legacy top-level
project-context/at each level of the upward walk, so unmigrated repos keep working. A newcontext_dirconfig knob (absolute, or relative to the repo root) overrides both. Scaffolding targets the hidden directory. Manifest resolution gained the new path ahead of the legacy one.This repo dogfoods it in the same commit:
git mvkeeps the history, and.gitignorere-includes the directory under the existing.amplifier/*ignore since these files are this repo's cross-clone memory.2. The session-start injection lists the files that exist
The instructions told agents, unconditionally, to read seven coordination files. In repos that only ever had Tier 1, that produced a steady stream of
Path not found— 66 in one measured week, 46 of them for a single missingAGENTS.md.The injected block now names the directory and its actual contents:
~30 tokens, and the scaffolded
AGENTS.mdnow frames Tier 2 as files to write rather than files to read.3. Capture seeds are phrases, not bare words
Seeds are the capture filter — content matching no attractor is never filed — so a seed like
"module"or"error"turns every file read and shell result into a memory. Measured over 7 days:architecturefired 4,190 times because source code contains the word "module";blocker1,976 times because stderr contains "error". Thirty-three real lessons were competing with that for the briefing's 1,500-token budget.All three copies move together (
context/memory-manifest.yaml, in-codeDEFAULT_MANIFEST, the capture hook's built-in table), and the parity test now compares seeds as well as ids and importance bases. New tests assert the property directly: verbatim samples of what the old seeds captured (--versionoutput, a returncode blob, a line-numbered file read, aPath not found) must classify asNone, and no seed may be a bare common word.4.
pytestat the root was skipping everythingFound while verifying the above.
pytest_collection_modifyitemsis called once per session with every collected item, whichever conftest defines it — and the integration conftest's loop had no path filter. Outside the DTU container it marked the entire repo's unit suite as skipped, so a barepytestreported "138 skipped" and looked green while running nothing.Verification
Known gap, not addressed here:
testpaths = ["tests"]means a rootpyteststill does not collectmodules/*/tests— the capture module's own suite has to be run explicitly.Migration for other repos
Or leave it — the legacy location is still discovered.
🤖 Generated with Claude Code