Skip to content

fix(#1839): add technical doc accuracy to correctness sub-agent - #1840

Merged
ralphbean merged 2 commits into
mainfrom
agent/1839-correctness-technical-docs
Jun 4, 2026
Merged

fix(#1839): add technical doc accuracy to correctness sub-agent#1840
ralphbean merged 2 commits into
mainfrom
agent/1839-correctness-technical-docs

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

The correctness sub-agent was declaring "zero correctness surface area" on documentation-only PRs, even when those documents contained implementation plans with verifiable technical claims (algorithm descriptions, pseudocode, CLI flag semantics, API behavior claims). Human reviewers on PR #1804 found 9 confirmed technical accuracy issues that the bot missed.

Changes:

  • Updated the correctness sub-agent definition to own technical
    accuracy in implementation plans and design documents, with
    specific evaluation guidance for algorithm logic, API/library
    behavior claims, design document alignment, internal
    consistency, and edge case correctness.
  • Updated SKILL.md section 3b to classify docs/plans/ files and
    technical documentation as having correctness surface area,
    ensuring the correctness sub-agent is dispatched for such PRs.
  • Added an implementation plan example to the dispatch table.

Note: make lint could not run (sandbox Go toolchain permission error unrelated to these markdown-only changes). Pre-commit encountered the same infrastructure error (exit code 3). The post-script runs authoritative pre-commit on the runner.


Closes #1839

Post-script verification

  • Branch is not main/master (agent/1839-correctness-technical-docs)
  • Secret scan passed (gitleaks — 1088f9b74b9ed046b902bf25e6ce4204339c99ee..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Site preview

Preview: https://55e523de-site.fullsend-ai.workers.dev

Commit: 3bdd71beedb35ec5646cff00a9fa344e9983adcc

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [internal-consistency] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md:28 — The list of content types defining "correctness surface area" in correctness.md differs from the list in SKILL.md section 3b. correctness.md includes "type specifications" (absent from SKILL.md) and uses "CLI flag semantics" instead of SKILL.md's "CLI flag specifications." Since SKILL.md drives the orchestrator's triage classification and correctness.md drives the sub-agent's self-assessment, a mismatch could cause the orchestrator to not classify a PR as having correctness surface area (e.g., a doc containing only type specifications) while the sub-agent would consider it in-scope, or vice versa. Align the two lists — either add "type specifications" to the SKILL.md bullet and standardize on one term for CLI flags, or remove the extra term from correctness.md.

Low

  • [scope-creep] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md:35 — The new "Design document alignment" criterion ("If the plan references a design document or ADR, are the claims consistent with the referenced source? Flag contradictions.") overlaps with the intent-coherence sub-agent's ownership of "whether the design fits the project's documented architecture (CLAUDE.md, ADRs, AGENTS.md)." The distinction exists (correctness = factual accuracy of claims about ADRs; intent-coherence = architectural direction alignment) but is not made explicit. Consider clarifying the boundary in both sub-agent definitions.

  • [pattern-inconsistency] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:253 — The new "Implementation plan" row is inserted at the top of the dispatch examples table, but existing rows follow a simple-to-complex progression (typo fix → bug fix → new endpoint → refactor → CI → migration). Consider reordering to maintain this progression or rephrasing to match the action-based naming pattern (e.g., "Technical documentation update").

Info

  • [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:253 — The "Implementation plan" dispatch example omits the security sub-agent. Implementation plans describing auth flows, secret handling, or data access patterns would contain security-relevant technical claims worth reviewing. The table shows examples rather than exhaustive rules, but a note about security dispatch for security-adjacent implementation plans could be useful.
Previous run

Review

Findings

Medium

  • [edge-case] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:196 — The category table in section 3a was not updated with documentation-accuracy categories. The correctness sub-agent now owns technical accuracy in implementation plans, but the re-review routing table only lists code-oriented categories (logic-error, nil-deref, off-by-one, etc.). New categories the sub-agent may produce (e.g., algorithm-error, api-claim-incorrect, design-inconsistency) will only route correctly via the fallback rule ("to correctness as a fallback"). If the fallback rule is ever changed, these findings would be misrouted during re-reviews.
    Remediation: Add documentation-accuracy categories (e.g., algorithm-error, api-claim-incorrect, design-inconsistency, edge-case-gap) to the correctness row of the category table in section 3a.

  • [incomplete-doc] docs/problems/code-review.md:55 — The Correctness agent section in the code-review problem doc describes the sub-agent's scope (logic errors, edge cases, test adequacy, split-payload attacks) but does not mention the new responsibility for technical documentation with correctness surface area. This doc is now stale relative to the expanded correctness.md definition.
    Remediation: Add a bullet noting that technical documentation with correctness surface area (algorithm logic, API behavior claims, design documents under docs/plans/) is also reviewed by the correctness sub-agent.

Low

  • [pattern-inconsistency] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:215 — The new classification bullet uses an em dash (—) as an explanatory aside before the arrow (→), a pattern not used in the other bullets in this list. Minor stylistic inconsistency; the em dash serves a legitimate clarification purpose.

  • [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:214 — The new classification criterion introduces content-based detection patterns ("algorithm descriptions, pseudocode, data structure definitions") but existing dispatch triage in section 3b is primarily domain-based (file paths, changed symbols). It is unclear whether the orchestrator should inspect file contents or rely solely on the docs/plans/ path prefix. See also: [edge-case] finding at SKILL.md:196.

  • [code-organization] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:253 — The new dispatch example "Implementation plan in docs/" is inserted at the top of the table. The existing table appears roughly ordered by complexity. Consider placing it after "Typo fix in README" since both are documentation-focused.

  • [pattern-inconsistency] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md:14 — The Own: section now mixes parenthetical clarifying questions with a declarative addition, slightly breaking parallel structure.

Info

  • [architectural-conflict] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:13 — Pre-existing ADR-0018 deviation. The skill already documents this as an "approved temporary exception." Not introduced by this PR.

  • [api-contract] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md:22 — The guidance to "cross-check against known behavior" for API/library claims relies on model training knowledge, which has a cutoff date. For claims about internal APIs, tool-based verification against repo source would be more reliable.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 3, 2026
Comment thread internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md Outdated
Comment thread internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md Outdated
Comment thread internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md Outdated
@ralphbean

Copy link
Copy Markdown
Member

/fs-fix

fullsend-ai-coder Bot pushed a commit that referenced this pull request Jun 3, 2026
Remove `docs/plans/` and `docs/` path references from SKILL.md dispatch
rules and correctness.md guidance. The pr-review skill is deployed to
repos with varying directory structures and cannot depend on
fullsend-specific paths.

Addresses review feedback on #1840

Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 3 review findings from ralphbean. Removed repo-specific path references (docs/plans/, docs/) from the generic pr-review skill's SKILL.md and correctness sub-agent definition.

Fixed (3):

  1. SKILL.md classification bullet references docs/plans/ path (internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md): Removed 'files under docs/plans/,' from the correctness classification bullet, keeping only the content-based detection criteria
  2. SKILL.md dispatch table references docs/ path (internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md): Changed 'Implementation plan in docs/' to 'Implementation plan' in the dispatch examples table
  3. correctness.md references docs/plans/ path (internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md): Applied reviewer's suggestion: changed 'Files under docs/plans/, and any' to 'Any' to remove path specificity

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 3, 2026
@ifireball ifireball assigned ggallen and unassigned ggallen Jun 4, 2026
@ralphbean

Copy link
Copy Markdown
Member

/fs-fix fix merge conflicts with latest main.

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the docs/plans/ specificity is gone, guidance looks solid.

fullsend-code added 2 commits June 4, 2026 13:30
The correctness sub-agent was declaring "zero correctness surface
area" on documentation-only PRs, even when those documents
contained implementation plans with verifiable technical claims
(algorithm descriptions, pseudocode, CLI flag semantics, API
behavior claims). Human reviewers on PR #1804 found 9 confirmed
technical accuracy issues that the bot missed.

Changes:
- Updated the correctness sub-agent definition to own technical
  accuracy in implementation plans and design documents, with
  specific evaluation guidance for algorithm logic, API/library
  behavior claims, design document alignment, internal
  consistency, and edge case correctness.
- Updated SKILL.md section 3b to classify docs/plans/ files and
  technical documentation as having correctness surface area,
  ensuring the correctness sub-agent is dispatched for such PRs.
- Added an implementation plan example to the dispatch table.

Note: make lint could not run (sandbox Go toolchain permission
error unrelated to these markdown-only changes). Pre-commit
encountered the same infrastructure error (exit code 3). The
post-script runs authoritative pre-commit on the runner.

Closes #1839

Signed-off-by: fullsend-code <fullsend-code@users.noreply.github.com>
Remove `docs/plans/` and `docs/` path references from SKILL.md dispatch
rules and correctness.md guidance. The pr-review skill is deployed to
repos with varying directory structures and cannot depend on
fullsend-specific paths.

Addresses review feedback on #1840

Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
@ralphbean
ralphbean force-pushed the agent/1839-correctness-technical-docs branch from c6c6702 to 3bdd71b Compare June 4, 2026 17:31
@ralphbean
ralphbean added this pull request to the merge queue Jun 4, 2026
Merged via the queue into main with commit 5816463 Jun 4, 2026
7 of 9 checks passed
@ralphbean
ralphbean deleted the agent/1839-correctness-technical-docs branch June 4, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review correctness sub-agent should evaluate technical accuracy in implementation plans

2 participants