Skills Design — Business Capability Layer

From Phase 2 onward, the low-confidence/no-route case is handled by FallbackAgentSkill (§3a) — a LangChain agent internally, only for deciding which Tools to call — not by GenericChatSkill, per langchain-rag-router-decision.md and architecture-overview.md §8. GenericChatSkill's Phase 1 role (the generic_chat YAML route's high-confidence handler, and Phase 1's own fallback since FallbackAgentSkill doesn't exist yet) is unchanged. The BaseSkill contract itself is unchanged — FallbackAgentSkill is an ordinary Skill by that contract; only its execute() internals differ. Phase 2 also adds a backstop prompt convention (§2), decided alongside ADR-006-memory-strategy.md and routing-design.md §5b: since memory and RAG are fallback-only, a high-confidence Skill can receive a query that implicitly depends on context it doesn't have. Every Skill's system prompt carries a fixed instruction to say so explicitly rather than silently answering only the part it can.

1. Purpose (Unchanged)

A Skill represents a business capability, not a technical integration. Skills orchestrate Tools and the LLM Service; Skills do not call other Skills; Tools do not call Skills.

2. Skill Contract (Updated)

BaseSkill (abstract):
  name: str
  required_tools: list[str]        # non-LLM tool names this skill declares as dependencies
  llm_service: LLMService            # every Skill receives this at construction, not resolved via a registry
  rules_engine_service: RulesEngineService | None   # Phase 3+; only FinancialSkill/CompanySkill receive a real instance

  async def execute(request: SkillRequest) -> SkillResult

SkillResult:
  content: str
  citations: list[Citation] = []      # empty in Phase 1; populated once RAG lands in Phase 2
  confidence: float
  metadata: dict                      # tokens used, tools invoked, latency, rule findings — sourced from ToolResult/LLMResponse/RuleEvaluationResult

required_tools refers only to non-LLM Tools. llm_service is a first-class constructor parameter on BaseSkill, injected identically for every Skill by the Skill Factory at startup — it is not optional and not looked up per-Skill from a registry, because every Skill needs it and its implementation is singular (one LLMService instance per application).

rules_engine_service is also a constructor parameter on every BaseSkill, per ADR-008 — but unlike llm_service, it's typed RulesEngineService | None and most Skills receive None. Only Skills that produce financial analysis (FinancialSkill, CompanySkill) receive a real instance, and only from Phase 3 onward — in Phase 1 and Phase 2, every Skill receives None here, since the Rules Engine doesn't exist yet. A Skill that doesn't use this dependency simply never references it in execute(); there's no separate "does this Skill support rules" flag beyond the dependency being present or absent.

SkillResult.metadata is where a Skill's RuleEvaluationResult lands once Phase 3 arrives (per rules-engine-design.md §5) — metadata isn't a new field added by this revision, but its expected contents now include rule findings alongside tokens/tools/latency.

Backstop prompt convention (Phase 2, Revision 6)

Every Skill's SYSTEM_PROMPT (CompanySkill, FinancialSkill, NewsSkill, GenericChatSkill, and FallbackAgentSkill) carries a fixed, appended instruction: if part of the user's question refers to something the Skill has no data or context for, say so explicitly rather than answering only the part it can. This is a one-line prompt addition, not a code-path or contract change — it doesn't require conversation history or memory to exist on RequestContext for a given Skill (most Skills never receive either, per ADR-006-memory-strategy.md); it just prevents an LLM call from silently dropping a part of the query it can't address.

This exists specifically because memory and RAG are fallback-only (ADR-006, ADR-007-rag-strategy.md): routing-design.md §5b's reference/comparison gate catches most queries that mix an explicit entity with an implicit reference to prior context, but it's a pattern-matched heuristic, not general language understanding, and won't catch every phrasing. This convention is the deliberate second layer — when the gate misses a case and a high-confidence Skill ends up with a question it can only partially answer, the failure is visible in the response, not silent.

3. Initial Skills (Phase 1, Updated)

In Phase 1, every Skill's rules_engine_service is None regardless of the "Yes/No" column above — that column describes which Skills are designed to eventually receive a real instance (Phase 3), not what they receive today.

GenericChatSkill no longer resolves an LLMTool via the Tool Registry (as in v1) — it calls self.llm_service.generate(...) directly, which is a strictly simpler dependency graph for the most frequently exercised Skill.

3a. FallbackAgentSkill (Phase 2+, New)

Per langchain-rag-router-decision.md and architecture-overview.md §8: invoked only when RoutingDecision.low_confidence == true or no route matched, after the Orchestrator has called the (real, Phase 2+) Context Builder. It is an ordinary Skill by contract — constructed by the Skill Factory from a config/skills/registry.yaml entry exactly like FinancialSkill or CompanySkill (declares required_tools naming any/all registered Tools it may choose to call, plus the standard LLMService dependency), and returns a typed SkillResult — indistinguishable to the Orchestrator from any other Skill.

The only difference is internal to execute(): instead of a hand-written, fixed Tool-call sequence, it uses a LangChain agent to decide which Tool(s) to call and in what order, because this is the one case where that sequence genuinely isn't known in advance (the Router already couldn't confidently map the query to a single deterministic path). This does not require any change to BaseSkill, the Skill Factory, or the Skill Registry — skill-factory-design.md §3 already treats "what a Skill does inside execute()" as opaque to construction.

Skill → Tools → LLM Service → SkillResult                                    (ordinary Skill, e.g. FinancialSkill)
Skill → [LangChain agent decides which Tools, in what order] → LLM Service → SkillResult   (FallbackAgentSkill)

Unit tests mock/stub the internal LangChain agent the same way other Skills' Tool calls are mocked — the agent's tool-selection logic is not re-tested against a real LLM in the unit suite.

Agent API: LangChain's text-based ReAct agent (langchain.agents.create_react_agent + AgentExecutor), not create_tool_calling_agent and not LangGraph — ADR-009-langchain-scope.md keeps LangGraph uninstalled/unused everywhere, including inside this Skill. create_tool_calling_agent was the first design considered here but was rejected: it requires binding tools natively to a BaseChatModel and reading structured tool_calls off the model response, and LLMRequest/LLMResponse (domain-model-design.md §3) carry neither — extending those Phase 1 contracts for the sole benefit of this one Skill was judged worse than picking an agent style that doesn't need them. create_react_agent reasons entirely in text (the standard "Thought / Action / Action Input / Observation" loop, tool descriptions rendered into the prompt, the model's free-text output parsed by LangChain's ReActSingleInputOutputParser), so it only ever needs plain prompt-in/text-out from the model — exactly what LLMService.generate() already provides. This is the same "agent decides which Tools, in what order" capability without adding a second agent framework to the one pocket that already needs one, and without an undecided contract extension.

Trade-off, stated explicitly rather than left implicit: text-parsed tool selection is less reliable than native tool-calling, particularly as tool count or argument complexity grows — a malformed Action/Action Input block fails to parse where a structured tool_calls response wouldn't. AgentExecutor is constructed with handle_parsing_errors=True so a malformed step becomes a retry/observation rather than a crash. This is judged acceptable because FallbackAgentSkill only ever runs on the already-lower-confidence branch and only wraps a handful of Tools; if a later phase grows its Tool set substantially, this trade-off should be revisited rather than assumed to still hold.

Concrete revisit trigger, not just tool count: ReAct's default single-input Action Input format expects one string per tool call. NewsTool (ticker + date range) and SECFilingsTool (ticker + filing type) are already multi-argument (domain-model-design.md §5) — the model has to serialize multiple fields into that one string, and StructuredTool has to reliably re-parse it back into kwargs before tool.call(**kwargs) runs, with no schema validation on that round-trip. If FallbackAgentSkill shows parsing failures specifically on these multi-argument Tools in practice — not just "tool count grew" — that is the signal to revisit the create_tool_calling_agent alternative (native, schema-validated tool args), even before the Tool set itself grows.

Adapting BaseTool into a LangChain tool: the adapter lives inside fallback_agent_skill.py, not in tools/ — BaseTool itself stays framework-agnostic, per ADR-003-skills-tools-separation.md. Each registered BaseTool the Skill declares in required_tools is wrapped in a thin langchain_core.tools.StructuredTool at construction time; the wrapper's function delegates to tool.call(**kwargs) and serializes the resulting ToolResult (its data on success, its error on failure) into the string/dict shape the LangChain agent expects. ToolResult itself never crosses into agents/LangChain code — same "map at the boundary" rule rag-design.md §4 already applies to RetrievedItem.

Adapting LLMService into a LangChain chat model (the create_react_agent bridge): create_react_agent + AgentExecutor require a langchain_core.language_models.BaseChatModel to call at each reasoning step — but every Skill, including FallbackAgentSkill, is only ever allowed to reason via LLMService.generate() (skills-design.md §7 anti-pattern list; architecture-overview.md §5). The bridge is a thin adapter class, _LLMServiceChatModel(BaseChatModel), defined inside fallback_agent_skill.py (same "adapter lives at the LangChain boundary, not in the service it wraps" rule as the StructuredTool adapter above) — its _generate()/_agenerate() translates the incoming LangChain BaseMessage list (the rendered ReAct prompt, including tool descriptions and scratchpad) into an LLMRequest(prompt=..., context=request.context), calls self.llm_service.generate(...), and wraps the returned LLMResponse.content — plain text, the model's next "Thought/Action/Action Input" turn — into a LangChain ChatResult/AIMessage. Because ReAct never calls BaseChatModel.bind_tools() or reads AIMessage.tool_calls, the adapter doesn't need to implement either — it's a pure text passthrough. AgentExecutor is constructed with this adapter as its model, so every LLM call the agent's internal reasoning loop makes — not just the final answer — still goes through LLMService's retry/failover/cost-tracking/observability pipeline; LLMService.generate()'s contract (llm-service-design.md §4) is unchanged and unaware this caller exists. This is the designed exception to "a Skill must go through LLMService" (§7): the adapter is the mechanism by which it does, not a bypass of it. Without this adapter, constructing AgentExecutor against a raw ChatAnthropic/ChatOpenAI instance would silently reintroduce the exact anti-pattern §7 already rejects (a Skill instantiating an LLM provider client directly) — so fallback_agent_skill.py must never construct a bare LangChain chat model itself, only _LLMServiceChatModel wrapping the injected llm_service.

Feeding RequestContext to the agent, and citations: retrieved_context (already populated by the Context Builder before FallbackAgentSkill.execute() runs) is formatted into a delimited context block and included in the agent's prompt alongside the user query — the same delimited-block discipline security-design.md §4 requires for any retrieved content reaching an LLM. Once Milestone 4 populates conversation_history/memory_items (memory-design.md §5), the same prompt-construction step includes them as their own delimited sections alongside retrieved_context — this is the reference-gate + memory path's whole point (ADR-006-memory-strategy.md): a query that reached FallbackAgentSkill specifically because it referenced prior context must actually see that context in the agent's prompt, not just have it sit populated and unused on RequestContext.

Citation selection default (Phase 2, not re-decided per milestone): every RetrievedItem included in the delimited context block is copied into SkillResult.citations as a Citation — Phase 2 does not attempt to parse the agent's output or re-score relevance to detect which specific items it "actually used"; that would require either output-parsing heuristics or a second LLM call, neither designed here. Citing everything provided is the simple, implementable default; narrowing it to provably-referenced items is deferred until real usage shows the over-citation is a problem. The source of any ToolResult the agent chose to call is copied into SkillResult.citations the same way — the same field every other Skill leaves empty in Phase 1 and is expected to start populating once RAG lands (domain-model-design.md §3).

4. Skill Registry and Construction (Updated — now via Skill Factory)

config/skills/registry.yaml (moved from backend/src/skills/registry.yaml — see architecture-overview.md §4 for the rationale):

skills:
  company_skill:
    class: CompanySkill
    required_tools: [company_profile_tool]
    rules_engine: true
    llm_config_ref: default
  financial_skill:
    class: FinancialSkill
    required_tools: [financial_metrics_tool, company_profile_tool]
    rules_engine: true
    llm_config_ref: default
  news_skill:
    class: NewsSkill
    required_tools: [news_tool]
    rules_engine: false
    llm_config_ref: default
  generic_chat:
    class: GenericChatSkill
    required_tools: []
    rules_engine: false
    llm_config_ref: default

Construction is no longer a direct registry-loader read (as described in Revision 2) — it goes through the Skill Factory (skill-factory-design.md, architecture-overview.md §7c). The factory resolves each entry's required_tools, rules_engine, and llm_config_ref fields and injects the corresponding LLMService, Tool instances, and RulesEngineService | None into the named Skill class's constructor.

class: is a name, not a dotted import path. Revision 2 showed class: skills.company_skill.CompanySkill, implying string-based dynamic import. That's superseded: the Skill Factory resolves class: against an explicit SKILL_CLASS_MAP (a small, hand-maintained dict in backend/src/skills/factory.py), not importlib. This is a deliberate choice, not a simplification for its own sake — see skill-factory-design.md §5 for why: dynamic string-based import is functionally equivalent to the plugin auto-discovery this architecture explicitly rules out (§16), just with YAML instead of a filesystem as the trigger.

llm_tool still never appears in any Skill's required_tools — that dependency remains implicit and universal, injected via llm_config_ref rather than declared as a Tool.

What adding a new Skill actually requires, per the Skill Factory design: (1) write the Skill class, (2) add one line to SKILL_CLASS_MAP, (3) add the YAML entry above. The registry YAML alone is not sufficient to add a Skill — a Skill's execute() behavior is always hand-written Python; the registry only declares what it depends on. See skill-factory-design.md §4 for the full boundary between what YAML may declare (composition) and what it may never express (behavior, prompts, branching).

5. Skill Composition Rule (Unchanged)

The Orchestrator decides execution order and result merging when a route requires multiple Skills. Skills remain unaware of each other.

6. Testing Approach (Updated)

• Unit tests instantiate a Skill with mocked Tools, a mocked/fake LLMService, and (where declared) a mocked/fake RulesEngineService (a simple stub returning canned LLMResponses / RuleEvaluationResults), asserting orchestration logic without any network access at all. A Skill with rules_engine_service=None is tested to confirm it never references the dependency.
• Contract tests assert every Skill in the registry only declares Tools that exist in config/tools/registry.yaml, that execute() matches BaseSkill, and that no Skill imports llm/factory.py directly (a lint rule, not just a convention).
• Skill Factory tests (new — see skill-factory-design.md §8) assert every class: value in config/skills/registry.yaml resolves in SKILL_CLASS_MAP, and that a resolved Skill instance receives exactly the dependencies its registry entry declares — no more, no less.
• FallbackAgentSkill (Phase 2+): unit tests mock/stub the internal LangChain agent's tool-selection decision the same way other Skills' Tool calls are mocked — no real LLM call in the unit suite. An integration test confirms it is reachable only via the low-confidence/no-route path, never from a high-confidence route.

7. Anti-Patterns Explicitly Rejected (Updated)

• ❌ A Skill directly instantiating an LLM provider client or importing llm/factory.py — must go through LLMService. This includes FallbackAgentSkill's AgentExecutor: it must be constructed against the _LLMServiceChatModel adapter (§3a) wrapping the injected llm_service, never a raw ChatAnthropic/ChatOpenAI/etc. instance — the adapter is how the agent satisfies this rule, not an exception to it.
• ❌ A Skill calling another Skill's execute().
• ❌ A Skill reading any config/*.yaml file directly — a Skill only sees what the Orchestrator/Skill Factory passes it.
• ❌ Re-adding an LLMTool to the Tool Registry "for consistency" — this was the exact pattern this revision removed; see ADR-004-llm-service-layer.md.
• ❌ A Skill calling RulesEngineService directly when its registry entry declares rules_engine: false — if a Skill needs rule evaluation, that's a registry change, not a workaround inside execute().
• ❌ Encoding a Skill's prompt template, branching logic, or response shaping in config/skills/registry.yaml — YAML declares dependencies only; see skill-factory-design.md §4 for the full boundary.
• ❌ A Skill other than FallbackAgentSkill (or a future dynamically-planning Skill, per architecture-overview.md §8) using a LangChain agent internally — every other Skill's Tool-call sequence is fixed, hand-written Python, because the Router already established enough confidence that the sequence is known.