docs: add applied agent-eval-tools doc - #5518
Conversation
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
1 similar comment
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
PR Summary by QodoDocs: cross-reference problem docs in roadmap and add applied harness-eval note
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1.
|
Site previewPreview: https://6abbaaaa-site.fullsend-ai.workers.dev Commit: |
rh-hemartin
left a comment
There was a problem hiding this comment.
Remove the "related problem docs" from the roadmap, the convention does not exist as far as I can see and I don't want to start that convention.
|
I agree with the document, but I do think that you need to refer to them more specfically as "agent-driven tools", for example here: I would go with |
|
addressed all notes @rh-hemartin :
|
waynesun09
left a comment
There was a problem hiding this comment.
[MEDIUM] PR title/description still describe the reverted roadmap cross-reference work
The PR title ("docs: cross-reference problem docs in roadmap, add applied harness-eval doc") and the body's "Roadmap cross-references" section describe adding "Related problem docs" links to two docs/roadmap.md sections. That work was added in commit 4b51271 but fully reverted in the final commit d030fa8 after reviewer feedback ("the convention does not exist... I don't want to start that convention"), confirmed by the author's own follow-up comment that the "Related problem docs" lines were removed from the roadmap. git diff main...pr for docs/roadmap.md is now empty — the final diff touches only docs/problems/applied/README.md and docs/problems/applied/harness-eval/README.md. Nobody updated the PR title/body to match.
Concrete downstream impact: .goreleaser.yml's changelog exclude filter drops ^docs(\(.*\))?: commits, so this docs-only title won't leak into the public release-notes changelog. However, this repo's squash-merge title default is "commit or PR title" — since this PR has 3 commits, GitHub will default the squash commit's title to this (stale) PR title, permanently misdescribing the change in git log, even though the squash body would default to the accurate concatenated individual commit messages.
Suggestion: Edit the PR title (e.g., "docs: add applied harness-eval doc") and delete the body's "Roadmap cross-references" section before merging, so the merged commit accurately reflects that only the applied/harness-eval addition shipped.
Review-only pass; not self-assigning or requesting changes.
|
fixed many things in the latest push. PR title updated to "docs: add applied agent-eval-tools doc" and body rewritten to match. @waynesun09 thank you for the review |
waynesun09
left a comment
There was a problem hiding this comment.
Review-only pass (follow-up); not self-assigning or requesting changes.
One additional candidate finding (cross-linking testing-agents.md's harness-eval-lab reference) was skipped as already covered by the earlier review thread's suggestions.
waynesun09
left a comment
There was a problem hiding this comment.
[HIGH] DCO check fails on final commit — missing required Signed-off-by trailer
The branch tip, commit 5194d42f ("fix(docs): address review feedback on agent-eval-tools doc"), is failing the DCO check (gh pr checks 5518 shows DCO as fail; the check run's conclusion is action_required). The commit's author/committer is a human account, not a bot, and its message ends only with an AI-assistant attribution trailer — no Signed-off-by: line. All four earlier commits on this branch end with a proper Signed-off-by: trailer.
CONTRIBUTING.md and AGENTS.md both state that human-driven agent sessions should sign off — the human directing the session is the one certifying the DCO — with the bot-identity exemption applying only to fully autonomous agent commits (author type Bot). This is a human-authored, assistant-assisted commit, so per the repo's own written policy it needs sign-off, and currently lacks it. This also currently falsifies this PR's own checklist line "Commits are signed off (DCO)".
Caveat: the repo's active branch ruleset's required_status_checks list doesn't explicitly name DCO, so I can't confirm with certainty this technically blocks the GitHub merge button. Regardless, it's a real, currently-failing check that contradicts repo policy.
Suggestion: Since this commit is the branch tip, git commit --amend -s --no-edit and force-push (no rebase needed) so it carries a Signed-off-by: trailer like every other commit on the branch.
Review-only pass; not self-assigning or requesting changes.
|
Pushing back on the directory rename ("agent-eval-tools" to something else): the disambiguation parenthetical on first mention already distinguishes harness-eval from agent-eval-harness in prose. Renaming the directory would break the existing index entry in applied/README.md and any external links. The directory name describes the doc's topic, not the tool itself. @waynesun09 except for that i fixed all findings:
|
…al doc - Add "Related problem docs" links in the Security hardening and Testing roadmap sections pointing to relevant problem documents - Add docs/problems/applied/harness-eval/ covering how fullsend's problem areas manifest in agent setup evaluation tools Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
…index Address review feedback: - Link audit-log-integrity to section anchor in security-threat-model.md - Use relative markdown links in applied/harness-eval doc - Add harness-eval entry to applied/README.md index Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
Address rh-hemartin feedback: - Remove "Related problem docs" lines from roadmap (convention doesn't exist) - Rename applied doc to "Agent-Driven Evaluation Tools" Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
…scape Address waynesun09 review: - Rename directory from harness-eval/ to agent-eval-tools/ to avoid collision with fullsend's own "harness" terminology - Fix "anti-jailbreak patterns" to "jailbreak patterns" - Fix "phantom MCP detection" to describe the capability without asserting an unverified term - Soften "must handle" to "may need to support" for multi-assistant - Add Technology landscape section per applied-doc template - Name and link harness-eval explicitly as the concrete example Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
- Add disambiguation between harness-eval and agent-eval-harness (ADR-0051) - Fix MCP claim to describe harness-eval as analyzing MCP configs, not integrating via MCP - Correct runtime claim to reflect fullsend's pluggable runtime.Runtime interface with Claude Code as the current production default Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
- Rename title from "Agent-Driven" to "Agent Setup" to accurately reflect that the core mechanism is deterministic static analysis - Add harness-eval as a concrete fullsend consumer (citing .fullsend/config.yaml and bot-authored PR fullsend-ai#8) - Replace "pre-commit hooks" with "Tekton tasks" per actual install docs - Convert plain-text ADR-0051 and runtimes.md to proper relative links - Match index entry in applied/README.md to new title Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
67bf8d1 to
b9b4323
Compare
waynesun09
left a comment
There was a problem hiding this comment.
Review-only pass; not self-assigning or requesting changes.
|
On "Technology landscape generalizes from a single tool" - the section describes how this category of tools works, with harness-eval as the named example. The purpose of the applied doc is to map fullsend problems to this tooling context, not to survey the landscape. The specifics (CLI delivery, YAML/JSON parsing, AST analysis) are properties of the problem space, not just harness-eval. Adding a second tool name would be arbitrary since the tools i know that does something close to it are not relevant.. dont you think we can say harness-eval? @waynesun09 On "Doc structure doesn't satisfy applied/README.md checklist": acknowledged. The doc maps 4 of ~30 problem areas because those are the ones that meaningfully manifest differently in evaluation tooling. Adding coverage-for-coverage-sake would dilute the doc. Happy to add a "Why this is interesting" sentence if that resolves it, but restructuring to mirror konflux-ci's breadth doesn't match the scope of this contribution. |
|
I'm going to resolve everything and merge. |
Summary
Adds
docs/problems/applied/agent-eval-tools/README.mdcovering how fullsend's problem areas manifest in agent-driven tools that evaluate agent configurations (the "who watches the watchmen" angle). Follows the existing applied-problems pattern established byapplied/konflux-ci/.Names and links harness-eval as the concrete example, includes a Technology landscape section, and maps four problem areas (testing-agents, MCP config drift, tool call risk assessment, trustworthiness evidence) to the evaluation tooling context.
Testing
Checklist