docs: document how to add a new text-generation task in CONTRIBUTING.md

[maowiz]

Description

The "Adding a New Stage or Module" section of CONTRIBUTING.md is still a TODO, and #16 (document how to add a new task) predates the current architecture. This PR adds an "Adding a New Text-Generation Task" section documenting the task system that exists on main today, in nemo_retriever/src/nemo_retriever/models/llm/tasks/:

• subclassing TextGenerationTask, with build_request as the single required method
• the optional parse / _preflight_error hooks and the strict invoke lifecycle with GenerationTaskError phases (request / transport / response / parse)
• construction-time validation conventions, following GenericPromptTask
• registration in tasks/__init__.py
• testing expectations, pointing at the existing nemo_retriever/tests/test_generation_tasks.py

Everything documented was verified against the modules on main. The Table of Contents is updated to match.

Addresses the intent of #16 under the current architecture, and complements the CONTRIBUTING.md refresh tracked in #2395.

Checklist

• Docs-only change; no code touched
• Commit signed off per the DCO section

🤖 Generated with Claude Code

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Greptile Summary

This docs-only PR adds an "Adding a New Text-Generation Task" section to CONTRIBUTING.md, covering the task system in nemo_retriever/src/nemo_retriever/models/llm/tasks/. The new section was verified against the current main source and is largely accurate.

• Documents subclassing TextGenerationTask, the build_request ABC, optional parse/_preflight_error hooks, the strict invoke lifecycle with GenerationTaskError phases, construction-time validation conventions, tasks/__init__.py registration, and testing guidance pointing at the existing test file.
• One documentation inaccuracy: both an inline comment and a design-rule bullet claim that required_inputs is "validated by the lifecycle", but TextGenerationTask.invoke never reads self.required_inputs — validation happens only inside build_request (or _preflight_error) as implemented by the subclass.

Confidence Score: 4/5

Safe to merge; docs-only change with one small inaccuracy that could mislead implementers about input validation.

The added documentation accurately covers the task structure, lifecycle phases, and registration pattern. The one inaccuracy — claiming the lifecycle enforces required_inputs when it does not — could lead a new task author to skip explicit input validation in build_request, resulting in an opaque generic error rather than a helpful one when inputs are missing.

CONTRIBUTING.md lines around the 'Design rules the lifecycle enforces' bullet and the code comment on required_inputs.

Important Files Changed

Filename	Overview
CONTRIBUTING.md	Adds an 'Adding a New Text-Generation Task' section accurately describing the task architecture, with one minor inaccuracy: the docs claim required_inputs is enforced by the lifecycle when it is actually only a convention enforced within each build_request implementation.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant invoke as TextGenerationTask.invoke
    participant preflight as _preflight_error (optional hook)
    participant build as build_request (required)
    participant client as TextCompletionClient
    participant parse as parse (optional hook)

    Caller->>invoke: "invoke(client, **inputs)"
    invoke->>preflight: "_preflight_error(**inputs)"
    preflight-->>invoke: None or error_code
    alt error_code returned
        invoke-->>Caller: "raise GenerationTaskError(phase=request)"
    end
    invoke->>build: "build_request(**inputs)"
    alt build_request raises
        invoke-->>Caller: "raise GenerationTaskError(phase=request)"
    end
    build-->>invoke: GenerationRequest
    invoke->>client: client.complete(messages, ...)
    alt transport error
        invoke-->>Caller: "raise GenerationTaskError(phase=transport)"
    end
    alt unsupported response
        invoke-->>Caller: "raise GenerationTaskError(phase=response)"
    end
    client-->>invoke: (raw_text, latency_s)
    invoke->>parse: parse(raw_text)
    alt parse raises
        invoke-->>Caller: "raise GenerationTaskError(phase=parse)"
    end
    parse-->>invoke: text
    alt empty text
        invoke-->>Caller: "raise GenerationTaskError(phase=response, code=empty_output_error)"
    end
    invoke-->>Caller: GeneratedTextResult(text, latency_s, model)

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
CONTRIBUTING.md:316-320
**`required_inputs` is not validated by the lifecycle**

The inline comment says the attribute is "validated by the lifecycle", and the Design rules section further states "everything `build_request` consumes must be listed in `required_inputs`" as something the lifecycle enforces. Neither claim is accurate. Examining `TextGenerationTask.invoke` in `base.py`, the method never reads `self.required_inputs` — no check against the caller-provided `inputs` is performed. What the lifecycle *does* do is wrap any exception thrown by `build_request` into a `request`-phase `GenerationTaskError`, so a missing key will still surface as an error, but that happens because the subclass implementation raises (e.g. a `KeyError`), not because the framework inspects `required_inputs`. A developer who reads this doc may believe listing a name there is sufficient for validation and skip writing explicit checks in `build_request` or `_preflight_error` — which would produce only a generic `request_error` message instead of a useful one.

_{Reviews (1): Last reviewed commit: "docs: add CONTRIBUTING guide for creatin..." | Re-trigger Greptile}

[greptile-apps]

required_inputs is not validated by the lifecycle

The inline comment says the attribute is "validated by the lifecycle", and the Design rules section further states "everything build_request consumes must be listed in required_inputs" as something the lifecycle enforces. Neither claim is accurate. Examining TextGenerationTask.invoke in base.py, the method never reads self.required_inputs — no check against the caller-provided inputs is performed. What the lifecycle does do is wrap any exception thrown by build_request into a request-phase GenerationTaskError, so a missing key will still surface as an error, but that happens because the subclass implementation raises (e.g. a KeyError), not because the framework inspects required_inputs. A developer who reads this doc may believe listing a name there is sufficient for validation and skip writing explicit checks in build_request or _preflight_error — which would produce only a generic request_error message instead of a useful one.

Prompt To Fix With AI

This is a comment left during a code review.
Path: CONTRIBUTING.md
Line: 316-320

Comment:
**`required_inputs` is not validated by the lifecycle**

The inline comment says the attribute is "validated by the lifecycle", and the Design rules section further states "everything `build_request` consumes must be listed in `required_inputs`" as something the lifecycle enforces. Neither claim is accurate. Examining `TextGenerationTask.invoke` in `base.py`, the method never reads `self.required_inputs` — no check against the caller-provided `inputs` is performed. What the lifecycle *does* do is wrap any exception thrown by `build_request` into a `request`-phase `GenerationTaskError`, so a missing key will still surface as an error, but that happens because the subclass implementation raises (e.g. a `KeyError`), not because the framework inspects `required_inputs`. A developer who reads this doc may believe listing a name there is sufficient for validation and skip writing explicit checks in `build_request` or `_preflight_error` — which would produce only a generic `request_error` message instead of a useful one.

How can I resolve this? If you propose a fix, please make it concise.