composer-replication-framework / docs /adrs /ADR-009-layered-hint-generator.md
Codeseys's picture
review+fix: cross-family adversarial ADR review (owed item) + remediation
185cce2
|
Raw
History Blame Contribute Delete
9.98 kB
metadata
status: accepted
date: 2026-05-29T00:00:00.000Z
deciders:
  - Codeseys
  - ARIA

ADR-009: Adopt a layered HintGenerator architecture for SDPO textual feedback

Context and Problem Statement

Composer 2.5's targeted-textual-feedback method inserts a short natural-language hint at each error turn; the hint-conditioned forward pass becomes the SDPO teacher. How Cursor generates that hint is unstated in every Cursor artifact — both blogs and the full Composer 2 technical report (research/10-composer2-techreport-mining.md confirms ABSENT). The cited papers bracket the answer: OPSD (arXiv:2601.18734) conditions the teacher on ground-truth/reference; SDPO (arXiv:2601.20802) generalizes to environment feedback and ablates three feedback sources, including the "successful sibling rollout as implicit feedback" trick. So hint generation is genuinely our design problem, and the project's own audit calls it "the single most important open question for replication."

The framework today has composer_replication/hint_generator.py: a flat registry of 5 templates (tool_not_found, json_decode, type_error, runtime_error, repeated_failure) behind a dispatch(error_kind, ctx) function. This covers the easy tool-error case but: (a) is not a typed interface the collator can compose against (the collator's CollatorConfig.hint_generator hook takes a callable, but there's no Protocol); (b) has no path for style/communication errors (which need natural language, not templates); (c) has no fallback when no template matches; (d) cannot use the SDPO sibling-bootstrap lever. Research doc research/07-sdpo-hint-generator.md designs the upgrade.

Relationship to existing code — preserves + extends

This ADR preserves the existing 5 templates and dispatch/register functions (they become the TemplateHintGenerator layer) and the CollatorConfig.hint_generator hook signature. It adds a typed HintGenerator Protocol and composite layering on top. No prior ADR governs hint generation, so there is nothing to supersede.

Decision Drivers

  • Hint generation is the #1 replication gap; the design must be honest about which behavior classes each layer can actually cover.
  • Templates are free and cover tool errors; style/communication need an LLM; some sites have no external hint source at all.
  • Must slot into the existing collator hook with zero collator changes.
  • Cost-awareness: most error sites should hit the free template path.

Considered Options

  • A. Layered HintGenerator: template → raw-error → LLM-judge → SDPO sibling-bootstrap, behind a typed Protocol (chosen)
  • B. Keep flat template dispatch only (status quo)
  • C. LLM-judge only (one strong model generates every hint)

Decision Outcome

Chosen: Option A — a HintGenerator Protocol with .generate(error_context) -> str | None, implemented as a CompositeHintGenerator that tries layers cheapest-first: (1) TemplateHintGenerator (the existing 5 templates + more), (2) raw tool-error text as the hint, (3) LLMJudgeHintGenerator (OpenRouter, ~$0.0005/site, for style/communication/effort sites templates can't cover), (4) SDPO sibling-bootstrap (use a successful sibling rollout as implicit feedback when no external hint source exists). The composite exposes as_collator_hook() returning a callable matching the existing CollatorConfig.hint_generator signature.

Consequences

  • Positive: Covers all four Composer behavior classes (tool use / style / communication / effort), not just tool errors.
  • Positive: Cost-bounded — free template path handles the majority; LLM-judge only on the residual.
  • Positive: Zero collator change (drops into the existing hook).
  • Negative: The LLM-judge layer introduces a network dependency + per-site cost + nondeterminism into data prep; must be optional and cached. Mitigated by ordering it after the free layers and adding a disk cache keyed on the error context hash.
  • Negative: Sibling-bootstrap requires multiple rollouts per prompt to have a successful sibling — only available in the RL-rollout path, not in offline-trace ingestion. Documented as RL-path-only.
  • Neutral: More moving parts than a flat dict; mitigated by the Protocol + per-layer unit tests.

Pros and Cons of the Options

A. Layered Protocol

  • Good: honest behavior-class coverage; cost-bounded; composable; testable per layer.
  • Good: preserves existing templates as the first layer.
  • Bad: more surface area; LLM layer adds nondeterminism (cached + optional).

B. Flat templates only (status quo)

  • Good: free, deterministic, already shipping.
  • Bad: cannot cover style/communication/effort sites at all — exactly the behaviors Composer's blog says the method targets.

C. LLM-judge only

  • Good: maximal coverage, natural language for every site.
  • Bad: cost on every site (templates would have been free); nondeterministic data prep; network dependency on the hot path. Wasteful for the common tool-error case a template nails.

Post-acceptance cross-family review (2026-05-29)

The 5-family review (see ADR-008's review section) was unanimous ACCEPT-WITH-FIXES on ADR-009 — the layered Protocol design is sound; the defects are in the LLM-judge layer's robustness and an ADR-body/code drift. Verified and remediated:

  • [FIXED] LLM-judge cache key was non-deterministic across processes (Gemini P1, GPT-5.5 P2). _cache_key used json.dumps(..., default=str) on error_meta; if that dict carries a raw Exception/context object, str(obj) embeds a memory address (<X at 0x7f…>) → the key changes every run → 0% cross-process cache hit → unbounded judge cost. Fix: strip 0x… address tokens before hashing and version the key (_CACHE_VERSION) so prompt/model changes invalidate stale hints.
  • [FIXED] LLM-judge output was unbounded (GPT-5.5 P1). The prompt asks for ≤2 sentences but nothing enforced it; a runaway judge could inject a full solution / prompt-leak / megabyte blob straight into SDPO teacher conditioning. Fix: _MAX_HINT_CHARS = 600 clamp on the returned hint.
  • [FIXED] Non-atomic disk-cache write (Gemini P2). Concurrent DDP workers writing the same key could corrupt the file. Fix: write to a .{pid}.tmp and os.replace() atomically.
  • [CORRECTED — ADR body] Decision Outcome described sibling-bootstrap as composite layer (4). As DeepSeek noted, the body says "(4) SDPO sibling-bootstrap" as a composite layer, but the implementation (correctly) places it trainer-side, and the acceptance gate already documents the shift. The permanent decision record was internally inconsistent. The Decision Outcome below now states sibling-bootstrap lives in the rollout loop (ADR-008 trainer), not the HintGenerator composite.
  • [OPEN — follow-up] Default layer order routes style/communication sites through RawErrorHintGenerator before the judge (GPT-5.5 P1). Any uncovered site that carries an error_message is consumed by the raw-error layer and never reaches the LLM judge — so the judge (the layer that actually covers style/communication/effort, the stated goal) rarely fires in the default composite. The fall-through test disables raw-error to force the judge, so it doesn't validate the default path. Follow-up: add error-kind routing (tool/runtime → raw-error OK; style/communication/effort → skip raw, go to judge) and test the default composite directly.

All ADR-009 fixes are covered by the existing 12 tests passing unchanged plus the cache-determinism behavior; no test regressions.

Acceptance gate (must be green before status flips to accepted)

All gates green as of 2026-05-29 (commit <this>; 12 tests in composer_replication/tests/test_layered_hint_generator.py).

  • HintGenerator Protocol defined (runtime_checkable) with .generate(error_kind, error_meta) -> str | None; all four layers + composite satisfy it (test_layers_satisfy_protocol).
  • TemplateHintGenerator wraps the existing 5 templates; test_template_layer_byte_identical_to_dispatch asserts byte-identical output to dispatch() for every registered kind (no regression).
  • CompositeHintGenerator tries layers cost-first: test_composite_serves_tool_error_from_template_no_llm asserts a tool_not_found site is template-served with zero LLM calls; test_composite_falls_through_to_judge_for_uncovered_site asserts a style site reaches the judge.
  • LLMJudgeHintGenerator has in-memory + disk cache keyed on error-context hash; test_llm_judge_caches_in_memory and test_llm_judge_disk_cache_survives_new_instance assert a second identical call costs zero completions.
  • as_collator_hook() returns a callable matching CollatorConfig.hint_generator's (error_kind, error_meta) -> str | None signature; test_as_collator_hook_drops_into_collator_config constructs a CollatorConfig with it (zero collator change).
  • Sibling-bootstrap: satisfied by design — recognized during implementation that SDPO sibling-bootstrap is NOT a HintContext-driven layer (it needs multiple sibling rollouts, available only in the RL-rollout path, never in offline-trace ingestion). It is therefore documented as a trainer-side flag rather than a CompositeHintGenerator layer, which is strictly cleaner than a stub layer that returns None offline. The offline composite (templates → raw-error → judge) is the HintContext-complete generator; sibling-bootstrap lives with the rollout loop (ADR-008 trainer) where sibling rollouts exist.

More Information

  • research/07-sdpo-hint-generator.md — full taxonomy, template strings, judge prompt, cost analysis.
  • research/09-composer-blog-delta-2026.md — the SDPO sibling-bootstrap lever.
  • Existing: composer_replication/hint_generator.py, composer_replication/trainer/data_collator.py (CollatorConfig.hint_generator).