Skip to content

docs: add base: composition and deprecated customized/ to glossary - #5868

Merged
ralphbean merged 3 commits into
mainfrom
docs/glossary-base-composition
Aug 3, 2026
Merged

docs: add base: composition and deprecated customized/ to glossary#5868
ralphbean merged 3 commits into
mainfrom
docs/glossary-base-composition

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Add a base: composition glossary entry (ADR 0045) — the mechanism for inheriting and overriding an upstream harness.
  • Add a customized/ directory glossary entry, marked deprecated, noting it's replaced by base: composition (ADR 0064).

Test plan

  • pre-commit run (markdown link lint, gitlint) passes on the commit.

base: composition (ADR 0045) is used across multiple ADRs and guides
but wasn't in the glossary. Add it, and add the customized/ directory
overlay it replaced, marked deprecated per ADR 0064.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean requested a review from a team as a code owner August 3, 2026 18:09
@ralphbean ralphbean added the fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs label Aug 3, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Docs: add glossary entries for base: composition and deprecated customized/ overlay

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Document base: composition as the preferred harness customization mechanism (ADR 0045).
• Add a deprecated glossary entry for customized/ overlays and point to migration/ADR 0064.
• Cross-link glossary entries to relevant ADRs and customization guides.
High-Level Assessment

The approach is appropriate for a docs-only change: define the new base: term where readers look for it (glossary) and explicitly mark the replaced mechanism (customized/) as deprecated with pointers to the governing ADRs and migration path.

Files changed (1) +10 / -0

Documentation (1) +10 / -0
glossary.mdAdd glossary entries for 'base:' composition and deprecated 'customized/' directory +10/-0

Add glossary entries for 'base:' composition and deprecated 'customized/' directory

• Introduces a new glossary definition for 'base:' composition, describing override/merge behavior and linking to ADR 0045/0064 and the customization guide. Adds a deprecated 'customized/' directory entry explaining the old overlay mechanism, its limitations, and what supersedes it, with ADR references and a migration command pointer.

docs/glossary.md

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:10 PM UTC · Completed 6:21 PM UTC
Commit: 01654f0 · View workflow run →

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Site preview

Preview: https://6e159b28-site.fullsend-ai.workers.dev

Commit: 48f2fcd22e9c22c66c63b26fbe43ec8e30054552

@qodo-code-review

qodo-code-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider


Remediation recommended

1. Wrong merge semantics ✓ Resolved 🐞 Bug ≡ Correctness
Description
The new glossary entry says list fields like providers are merged with deduplication/child-wins
conflict resolution, but base: composition actually concatenates providers (no dedup). This
misdocuments harness behavior and can cause users to unknowingly introduce duplicate providers or
rely on conflict behavior that doesn’t exist.
Code

docs/glossary.md[35]

+The mechanism for customizing an agent's harness: a thin harness file sets `base:` to a local path or URL pointing at an upstream harness, then declares only the fields that differ. Scalars override the base value; list fields like `skills` and `providers` merge (deduplicated by basename/name, child wins on conflict); `env` and `runner_env` merge as maps. Replaces the deprecated [`customized/` directory](#customized-directory) overlay, which required copying and maintaining an entire upstream YAML file to change a single field.
Relevance

●●● Strong

Team often accepts fixes for docs/ADR statements that misdescribe implemented behavior/merge
semantics (e.g., PRs #2582, #5328).

PR-#2582
PR-#5328

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The glossary claims providers are deduplicated/child-wins on conflict, but ADR 0045 and the merge
implementation both define providers as simple concatenation with no deduplication.

docs/glossary.md[33-36]
docs/ADRs/0045-forge-portable-harness-schema.md[357-368]
internal/harness/compose.go[518-529]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`docs/glossary.md` claims list fields like `skills` and `providers` merge with deduplication and “child wins on conflict”. That is not correct for `providers` (and some other list fields): the implemented/documented behavior is concatenation (base + child) without deduplication.

## Issue Context
The glossary entry is intended to summarize the real `base:` composition semantics (ADR 0045 + implementation). Right now it overgeneralizes the `skills` dedup behavior to `providers`.

## Fix Focus Areas
- docs/glossary.md[33-36]

### Suggested wording direction
Adjust the sentence to reflect field-specific behavior, e.g.:
- Scalars: child overrides base
- `skills`: merged with dedup by basename (child overrides)
- `plugins`/`providers`/`api_servers`: concatenated (base + child)
- `host_files`: last-writer-wins by `dest`
- `env` + `runner_env`: merged maps (child keys win)
(Exact scope can match ADR 0045; keep it concise.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread docs/glossary.md Outdated
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Looks good to me

Previous run

Looks good to me

Previous run (2)

Review

Findings

Low

  • [technical accuracy] docs/glossary.md:35 — The base: Composition entry groups skills and providers as having the same merge semantics ("list fields like skills and providers merge (deduplicated by basename/name, child wins on conflict)"). ADR 0045 distinguishes them: skills merge with basename deduplication at composition time, but providers are concatenated (base + child) with deduplication happening at runtime (dedupResolvedProviders), not during harness composition. The glossary conflates two stages into one.
    Remediation: Either describe providers separately as "concatenated; deduplicated by name at runtime," or reword to clarify the description covers effective behavior rather than the composition step alone.

  • [heading-formatting] docs/glossary.md:33 — Heading uses backtick code formatting (### \base:` Composition`). All existing glossary entries use plain text headings without code formatting.
    Remediation: Use plain text (e.g., "Base Composition") to match the established pattern.

  • [heading-formatting] docs/glossary.md:55 — Heading uses backtick code formatting (### \customized/` Directory`), inconsistent with established plain-text heading pattern.
    Remediation: Use plain text (e.g., "Customized Directory") to match the established pattern.

  • [stale-reference] docs/agents/topics/default-vs-custom.md:73 — Table entry presents customized/skills/ as a current extension point without noting deprecation (ADR 0064). The linked guide already has deprecation notices, mitigating reader impact.
    Remediation: Add a deprecation marker to the table entry.


Labels: PR modifies documentation glossary under docs/

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/docs User-facing documentation labels Aug 3, 2026

@waynesun09 waynesun09 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.

Approving — no new findings from review squad beyond what's already flagged. Note the existing qodo/fullsend-ai-review threads on docs/glossary.md's providers merge-semantics claim (concatenation vs. dedup, per ADR 0045) and heading-formatting nits are still open; author should address before merge, not gating approval on them.

Match ADR 0045: providers/plugins/api_servers concatenate rather than
dedup like skills. Also drop backtick formatting from headings to
match the rest of the glossary.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:17 PM UTC · Completed 7:27 PM UTC
Commit: c1e2ca8 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ralphbean
ralphbean added this pull request to the merge queue Aug 3, 2026

@waynesun09 waynesun09 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.

Automated review pass — 1 finding on the Base Composition glossary entry (env/runner_env merge semantics vs ADR 0055), distinct from the already-resolved provider/skills merge-semantics and heading-formatting comments on this PR.

Comment thread docs/glossary.md Outdated
@ralphbean
ralphbean removed this pull request from the merge queue due to a manual request Aug 3, 2026
Base Composition entry treated env and runner_env as the same
map-merge mechanism. env is actually a distinct EnvConfig struct
with independent runner/sandbox sub-maps, and runner_env is
deprecated in favor of env.runner.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:33 PM UTC · Completed 7:41 PM UTC
Commit: 48f2fcd · View workflow run →

@ralphbean
ralphbean added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 7a432de Aug 3, 2026
16 checks passed
@ralphbean
ralphbean deleted the docs/glossary-base-composition branch August 3, 2026 20:16
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:18 PM UTC · Completed 8:38 PM UTC
Commit: 48f2fcd · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5868 — glossary entries for base composition and customized directory

What happened

PR #5868 was a human-authored docs PR by ralphbean adding two glossary entries: one for base: composition (ADR 0045) and one for the deprecated customized/ directory (ADR 0064). It changed a single file (docs/glossary.md, +10 lines) and went through 3 commits before merging.

Timeline:

  1. 18:09 — PR opened with initial commit 01654f0.
  2. 18:10–18:12 — Both qodo-code-review and fullsend-ai-review independently flagged incorrect merge semantics: the glossary described providers as deduplicated when ADR 0045 specifies concatenation. fullsend-ai-review also flagged two heading-formatting inconsistencies (backtick code in headings vs. plain-text convention).
  3. 18:21 — fullsend-ai-review approved with 4 low-severity findings.
  4. 19:13 — waynesun09 (human) approved, noting the open review threads should be addressed before merge.
  5. 19:17 — Author pushed c1e2ca8 fixing provider merge semantics and heading style.
  6. 19:28 — waynesun09 posted a new finding: the env/runner_env description was inaccurate. env is an EnvConfig struct with independent runner/sandbox sub-maps (per ADR 0055), not a flat map merged identically to runner_env. Additionally, runner_env is deprecated in favor of env.runner, and ADR 0055 was not cited.
  7. 19:31 — Author pushed 48f2fcd fixing the env description and citing ADR 0055.
  8. 19:41 — fullsend-ai-review re-approved the final state.
  9. 20:16 — PR merged.

Three review agent runs executed (IDs 30840032802, 30845101251, 30846190669), all completing successfully. No code or fix agent runs were involved — this was a human-authored PR.

What went well

  • Both bot reviewers independently caught the provider merge semantics issue on the first pass — good redundancy.
  • fullsend-ai-review correctly identified heading-formatting inconsistencies against the established glossary convention.
  • The review pipeline responded efficiently: 3 runs averaging ~10 minutes each, total cost ~$7.25 across all runs.
  • The author engaged constructively with all findings and pushed fixes promptly.

What the human caught that the agent missed

waynesun09 identified that the glossary's env and runner_env description was structurally inaccurate — a finding the review agent missed across two runs despite seeing the relevant evidence.

In the second review run (30845101251), the correctness subagent (Opus) investigated the env claim thoroughly: it grepped Go code for yaml:"env,", found the EnvConfig struct, and read the code comment that literally said "Replaces runner_env (ADR 0055)." The agent recognized that env is a struct containing maps, not a flat map, and noted this was "slightly imprecise." However, it concluded the description was "close enough for a glossary definition" and did not file a finding. Critically, the agent never read ADR 0055 despite seeing it referenced in the code comment — an ADR that would have confirmed runner_env is deprecated and provided the precise semantics.

Root cause

Two compounding gaps in the correctness subagent's behavior:

  1. Reference-following gap: The agent's search strategy was anchored to ADRs already cited in the PR text (0035, 0045, 0064). When it discovered an ADR reference in code during its own investigation (ADR 0055 in the EnvConfig comment), it did not follow it. Had it read ADR 0055, it would have found clear evidence that runner_env is deprecated and env has a specific struct-based merge model.
  2. Threshold calibration: The correctness subagent's guidance says technical documentation has "correctness surface area" and must not be short-circuited, but it does not instruct the agent to report structural inaccuracies in reference documentation. The agent applied a lenient "close enough" standard to a glossary entry whose purpose is to precisely describe merge semantics.

Evidence for existing issues

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/docs User-facing documentation fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants