Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .harness/docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ The files at the root always describe the *current* attempt. `attempts/attempt-N
- The coordinator's output contract is stated directly, in `tests/test_coordinator_contract.py`, not as equality with a past implementation. That file is the standing home for what a run must write: `state.json`'s exact field set (read from `dataclasses.fields(RunState)` rather than typed out, so it cannot disagree with the definition it describes), each field's type, the statuses a run can end in (exercised by runs that reach them), the frozen `events.log` line pattern, the escalation summary's five parts, and the run directory's **required subset** of artifacts (derived from the loaded workflow's `outputs`, per the same read-it-off-the-definition rule as `archivable_artifacts`). It is not named for a story and is not one story's evidence — a later story that means to change one of these shapes edits it deliberately, which is the point.
- A differential test against a frozen implementation is an instrument with a shelf life, and it should be retired when the constraint it was built for has landed. story-011's six comparisons — same `events.log`, same `state.json`, same escalation summary, same run-directory contents as the pre-story-011 coordinator — were exactly right while the requirement was "adding `execution-history.json` changes nothing else". Once merged they asserted something nobody holds: that the coordinator's output may never differ from what one implementation produced on one day. story-012 (`retry-history.json`) and story-014 (`clean-clone-result.json`) each legitimately add an artifact and an event; story-014 escalated on nine failures, all of them this comparison, with the file in its `do_not_modify`. The instrument also decays independently — the historical coordinator is run against today's workflow, schemas and config, a pairing that grows more artificial until the old code cannot run at all. story-016 replaced the comparisons with the direct shape assertions above. Removing a test is not weakening it when every guarantee it carried is restated as a shape and shown to fail on violation; the corollary is that the restatement must land *before* the removal, and each new assertion must be demonstrated red. What survives in `tests/test_story_011_validation.py` is everything not resolved out of git history: the log-line-to-history correspondence, the ordering and retry-stream checks, the schema conformance of a run's history, and the prompt-scope assertion with its baseline resolution intact. No module under `tests/` loads a coordinator implementation out of git history any longer.
- Do not assert a run directory as an exact set. Every story that adds an artifact would fail an assertion about something else, which is the friction story-016 exists to remove; require the artifacts a completed run must produce and permit others. The schemas *inventory* (above) is the deliberate exception — there the point is that a new shape cannot appear unnoticed.
- The `HEAD`-baseline trap catches validation files too, not only differential tests of orchestration code. story-016's tester resolved its "before" copy of `tests/test_story_011_validation.py` as `git show HEAD:...`; in the working tree the diff was real, and in a clean clone with the story committed the baseline *was* the story's own file, so the removal assertion compared the file against itself. One test failed and two neighbouring diff assertions had gone silently vacuous — the more dangerous outcome. The fix is the same walk story-011 uses: `git log --format=%H -- <path>` newest-first, taking the first blob that still carries what the story removed, raising loudly when none does, plus a positive guard asserting the resolved baseline differs from the working tree and really contains it. Read the bullet above as applying to any test that resolves a baseline out of git, whatever it is a baseline of. The same trap has a quieter form: an assertion that a story left a path alone written as `git diff HEAD -- <path>` empty. That asks whether the working tree is dirty there, which is a question about whoever is working *now* — vacuously green once the story commits, and red for every later story that legitimately edits the path. `tests/test_story_009_validation.py` and `tests/test_story_010_validation.py` carried six of these and story-014 rewrote them to diff the story's own commit against its parent, resolving the commit by walking the marker file's history oldest-first for the first revision carrying the feature the story introduced, with a positive guard asserting the parent really predates the marker so the bound cannot silently resolve to something vacuous. While a story is still uncommitted the marker resolves to `None` and the working tree is the end bound, which is the original comparison.
- The `HEAD`-baseline trap catches validation files too, not only differential tests of orchestration code. story-016's tester resolved its "before" copy of `tests/test_story_011_validation.py` as `git show HEAD:...`; in the working tree the diff was real, and in a clean clone with the story committed the baseline *was* the story's own file, so the removal assertion compared the file against itself. One test failed and two neighbouring diff assertions had gone silently vacuous — the more dangerous outcome. The fix is the same walk story-011 uses: `git log --format=%H -- <path>` newest-first, taking the first blob that still carries what the story removed, raising loudly when none does, plus a positive guard asserting the resolved baseline differs from the working tree and really contains it. Read the bullet above as applying to any test that resolves a baseline out of git, whatever it is a baseline of. The same trap has a quieter form: an assertion that a story left a path alone written as `git diff HEAD -- <path>` empty. That asks whether the working tree is dirty there, which is a question about whoever is working *now* — vacuously green once the story commits, and red for every later story that legitimately edits the path. `tests/test_story_009_validation.py` and `tests/test_story_010_validation.py` carried six of these; story-014 rewrote them to diff the story's own commit against its parent, resolving the commit by walking a marker file's history for the first revision carrying the feature the story introduced, and story-015 replaced that with the shared resolution below. Five stories in total (007, 008, 009, 010 and 013) shipped the idiom, which is why the next two bullets exist: it is a standing pattern the harness permitted, not one story's lapse.
- **The baseline a per-story assertion compares against is resolved in exactly one place**: `story_commit_range(validation_file, repo=HARNESS_ROOT)` in `tests/conftest.py`, with `story_diff(paths, validation_file=..., diff_filter=..., options=...)` on top of it. A story's own run commit is the **oldest** commit that *added* that story's validation file (`git log --diff-filter=A`), and the baseline is that commit's parent. Oldest-addition is what makes a planning or hotfix commit on the same story — which *modifies* the file — unable to be mistaken for the run, and the pair of bounds is what keeps the comparison fixed as later stories accumulate: it survives a commit, a rebase and a squash, where a pinned SHA would not, and it needs no marker string to be chosen and kept true. While the story is in flight the file is uncommitted and the range degrades to HEAD against the working tree, which is the correct pre-story baseline then. When the file is in `HEAD` but no adding commit is visible (a shallow clone) or the adding commit is the root, it raises `NothingToCompareAgainst` rather than returning a baseline that makes the caller vacuous — the discrimination is a `git cat-file -e HEAD:<path>` probe, so the uncommitted fallback can never mask a truncated history. The `repo` parameter exists so the same code path can be exercised against a synthetic history in which the story *is* committed, the state the repository under test cannot be in while these tests decide whether it commits. No repaired file carries its own copy of the resolution.
- **`tests/test_baseline_honesty.py` is the mechanical check, and it is deliberately narrow.** An `ast` scan over every module found by globbing `tests/` flags a `subprocess.run`/`check_output` whose first positional argument is a list literal beginning with `"git"`, that targets *this* repository's root (`-C <REPO_ROOT|HARNESS_ROOT>` or a `cwd=` naming one), and that carries a HEAD-derived revision (`HEAD`, `HEAD:path`, `HEAD~1`, `HEAD^`, or an f-string with that literal prefix) or `status --porcelain`. That is the whole class it covers: an assertion can still be empty on both sides of an honest baseline, tautological, or aimed at the wrong subject, and no AST scan will say so. Say that where the check is defined rather than implying broader coverage — the module docstring does. Two consequences worth knowing before extending it: a git call written with neither `-C` nor `cwd` inherits the process working directory (the repository root, under pytest) and is **not** flagged, and a call assembled through `functools.partial` or a local helper is not seen either — which is by design, since that is how the throwaway-repository tests are written and they must stay unflagged and unrewritten. Exactly one module is exempt, stated as `EXEMPT_MODULES = ("conftest.py",)` rather than inferred: the one holding the shared resolution, the one place where comparing the working tree against HEAD is the right answer. Every per-story validation file is subject to the check, including the four repaired. The glob carries its companion non-empty assertion, per the corpus-test rule above, and the scanned and exempt sets are held to a partition of the glob.
- **The regression set is committed evidence, not a constructed fixture.** The four pre-repair files are recovered with `git show <baseline>:<path>` where the baseline comes from the *same* shared resolution applied to the check's own module — so no pinned SHA a rebase could invalidate — and story-013's instance is read from `.harness/runs-archive/story-013-vacuous-tests/pre-reset-test_story_013_validation.py`, which is read-only evidence and was not touched. All five are asserted flagged and the four repaired files asserted clean afterwards. The check also carries its own negative controls (the same command flagged against the repository root and ignored against a repository the test built), so "zero flags in the suite" means the idiom is gone rather than that the detector went blind.
- **Repair is not deletion, and an honest baseline that is always empty for a different reason is no improvement.** Each repaired assertion kept its subject, its output options and its strictness; the only authorized change was narrowing the two tests named `test_no_committed_story_artifact_was_edited` to `--diff-filter=MD`, and that one is load-bearing rather than cosmetic — each story's own artifact was *added* in its own run commit, so without it both assertions repair straight into red, and an addition was never an edit. Every repaired subject is then shown failing by violating it against a synthetic history in which the story is committed and its run commit edits the path it claims to have left alone. The narrowed pair is shown still catching a modified *and* a deleted artifact, so narrowed is demonstrably not weakened.
- **Prompt guidance was added for the wider class but is not the enforcement.** `prompts/tester.md` now requires an absence assertion to carry a negative control demonstrating it can fail, distinguishing that class from positive assertions which fail loudly on their own; `prompts/verifier.md` makes an absence assertion offered as evidence without such a demonstration a finding. Both are prose in the role layer — there is no contract file to inject, and this is role guidance rather than schema content, per the injection bullet above. Presence is confirmed by rendering the templates through `orchestration/context_assembler.py`, not by reading them. Treat the prose as the general reminder and the mechanical check as the thing that actually holds: a written rule in an injected document was already tried here and failed five times.
- The verifier stands in the one environment where the story's own commit does not exist. `_complete` runs `git add -A` and commits *after* the documenter and after every check the workflow performs, so the state the code ships in is created after the last thing that could object to it, and until story-014 nothing ran the suite there. Three failures across two stories lived in that gap and every one reported green: story-011's differential test resolved its baseline as `git show HEAD:…` (419 passed uncommitted, 394 passed and 25 errors committed), and story-013 shipped a test asserting `git status --porcelain -- tests/` lists the tester's new files — true mid-run, false once `_complete` commits them — on a run that reported 460/460 and passed verification first time. This is not verifier leniency and a stricter verifier would not close it: the suite genuinely passes where the verifier is standing. The bug was the absent check, not a missed one.
- The clean-clone check is unconditional for every story that reaches a passing verifier. It is deliberately not gated on a heuristic about what the story touched — "the story edited tests that read git" is exactly the kind of predicate that fails on the next unforeseen environment difference. It does not replace CI, which remains the final word; it moves discovery earlier, so a story is not reported complete and committed before the failure is known.
- A clone, not a tree copy. A copy would carry `.venv/` and `.harness/runs/`, and it would not have the story as a *commit* — which is the whole point, since a test resolving a baseline as `git show HEAD:…` only misbehaves once the story *is* `HEAD`. Cloning from the local filesystem path also makes "no network access" true by construction rather than by observation, and letting `.gitignore` do the filtering is what makes the clone's contents the same set `_complete`'s `git add -A` would commit, without a second exclusion list to keep in sync. The target repository is never mutated: no commit, no branch, no index change, no stash.
Expand Down
24 changes: 24 additions & 0 deletions prompts/tester.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@ Do not:

New tests belong in tests/ and become permanent repository assets.

An assertion that claims an absence needs a negative control. A positive
assertion — that something exists, or behaves a particular way — fails
loudly on its own the moment the behavior is missing, so writing it is
enough. An absence assertion is different: that a path was not changed,
that a name does not appear, that a list is empty, that no violation was
found. It passes when the property holds and it passes just as happily when
the test is looking in the wrong place, when the subject has been resolved
to something that cannot differ, or when the check itself has stopped
seeing anything. Green tells you nothing about which of those happened.

So for every absence you assert, also demonstrate that it can fail:
construct the violation the assertion is meant to catch — against a
throwaway repository, a modified copy of the input, or a stripped
rendering — and assert that the same check reports it. Write the control
beside the assertion it protects, and say in the test what it is
controlling for. An absence assertion with no demonstration of failure is
not validation; it is a claim about what you happened to observe.

Baselines resolved out of git are the recurring instance of this. Do not
resolve one as `HEAD` or as the working tree against the repository root:
the coordinator commits the working tree at the end of a successful run, so
those comparisons go vacuously green the moment the story commits. Use the
shared resolution in `tests/conftest.py`.

When you finish, write these files to the run directory at {{run_dir}}:

test-results.json, the structured outcome of the validation you ran. It
Expand Down
9 changes: 9 additions & 0 deletions prompts/verifier.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ Do not:
Uncertainty is not failure. If evidence is missing, say what is missing
rather than inventing a failure.

A passing test is evidence only if it could have failed. An assertion that
claims an absence — that a path was not changed, that a name does not
appear, that a list is empty, that no violation was found — passes equally
when the property holds and when the check has stopped looking at anything.
An absence assertion presented as evidence without a demonstration that it
can fail is a finding: say which assertion, and what a violation of it
would have to look like for the test to notice. A positive assertion needs
no such control, because it fails on its own when the behavior is missing.

When you finish, write these files to the run directory at {{run_dir}}:

verification-result.json, your verdict and the evidence behind it. The
Expand Down
Loading
Loading