fix: execute review: blocks on type: string step outputs (#350) (#351)

* fix: execute review: blocks on type: string step outputs (#350)

Previously, review: blocks declared on type: string outputs were silently
dropped because the review pipeline only matched ReviewRule patterns against
file paths. Authors got no warning and the review appeared correctly
configured. This was especially problematic given that the job_yml
DeepSchema directs authors toward type: string for transient inter-step data.

String outputs with review blocks now produce synthetic ReviewTask objects
with the value carried on a new ReviewTask.inline_content field. The value
is rendered into the instruction file as a "Content to Review" section and
mixed into the review_id content hash so each distinct value gets its own
pass-cache key (cache invalidates on value change, persists when unchanged).

Adds:
- ReviewTask.inline_content field
- build_string_output_review_tasks() in quality_gate, wired into run_quality_gate
- Shared _build_preamble() helper to deduplicate common_job_info+inputs logic
- Requirements: JOBS-REQ-004.8, REVIEW-REQ-005.1.8, REVIEW-REQ-009.1.7
- 18 new tests (unit + end-to-end via run_quality_gate)

Fixes #350

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: format inline-content review tasks as "inline content" (not "0 files")

Follow-up to the issue #350 string-output review fix. The formatter's
_task_name() used len(files_to_review) to describe task scope, which fell
through to the multi-file branch for inline-content tasks and produced
"review of 0 files" — cosmetic wart visible in the parallel-task listing.

Also adds a bespoke test_job (.deepwork/jobs/test_job/) that exercises the
full quality gate end-to-end with four outputs covering every review-level
combination: file_path + output-ref review, string + output-ref review
(NEW), string + arg-level review (NEW, inherited), and string + no review
(control). Used as a manual smoke test when verifying the fix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: apply ruff format to satisfy CI format check

CI runs `ruff format --check`, which my local `uv run ruff check` calls
did not exercise (check runs the linter, not the formatter). Two lines
I had manually broken up were re-joined by ruff format.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: split Makefile lint target into lint (check) and lint-fix (auto-fix)

Previously `make lint` ran `ruff format` and `ruff check --fix` — both in
auto-fix mode — so it silently mutated files instead of reporting issues.
CI, on the other hand, runs `ruff format --check` and `ruff check` in
check-only mode, which is what actually fails a PR. That asymmetry meant a
clean local `make lint` gave no guarantee that CI would pass: running it
would just reformat files in-place, and the developer might miss the
uncommitted changes.

Splits into two targets:

  - `make lint` now mirrors CI (ruff format --check, ruff check, mypy).
    A clean run guarantees the CI Lint job will pass.
  - `make lint-fix` is the previous behavior (auto-fix formatter + linter)
    for convenient local cleanup.

Caught while working on #350 — my local `ruff check` calls did not run the
formatter at all, and even `make lint` would have only silently reformatted
the files, leaving a surprise CI failure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: nhorton

.deepwork/jobs/test_job/fixtures/report.md

@@ -0,0 +1,4 @@
+# Test Job Report
+
+This is a short markdown report produced by the test_job workflow to exercise
+the file_path review path of the DeepWork quality-gate review system.

.deepwork/jobs/test_job/job.yml

@@ -0,0 +1,74 @@
+name: test_job
+summary: "Exercise the DeepWork quality-gate review system with string and file_path outputs to verify string-output reviews run"
+
+step_arguments:
+  - name: report_file
+    description: "A short markdown report file produced by the step — exercises the file_path review path."
+    type: file_path
+
+  - name: summary_text
+    description: "A 1-2 sentence narrative summary of the step's work — exercises a string output review defined at the step output-ref level."
+    type: string
+
+  - name: metrics_text
+    description: "A concise metrics blurb containing at least one numeric value — exercises a string output review defined at the step_argument level (inherited by any step that outputs it)."
+    type: string
+    review:
+      strategy: individual
+      instructions: |
+        Verify the value is a concise metrics summary of 1-3 sentences that contains at
+        least one numeric value. Flag it if the value is empty, a placeholder, or does
+        not mention any numbers.
+
+  - name: notes_text
+    description: "A freeform notes string with no review attached — control case that should produce no review task."
+    type: string
+
+workflows:
+  test_review_system:
+    summary: "Produce four outputs of mixed types and review levels to verify the quality gate reviews them correctly."
+    common_job_info_provided_to_all_steps_at_runtime: |
+      This bespoke test job verifies that the DeepWork quality-gate review system
+      executes reviews on string outputs as well as file_path outputs. Four outputs are
+      produced in a single step, each hitting a distinct review code path. The control
+      case (notes_text) must NOT produce a review task.
+    steps:
+      - name: produce_outputs
+        instructions: |
+          # Produce test outputs for the quality gate
+
+          Create each of the four outputs below, then call `finished_step` with all four
+          values. Keep the content short.
+
+          1. **report_file**: write a small markdown file at
+             `.deepwork/jobs/test_job/fixtures/report.md` (create the parent directory
+             first if needed). Two or three lines — a heading plus a sentence is fine.
+
+          2. **summary_text**: a 1-2 sentence narrative summary describing the work
+             performed in this step.
+
+          3. **metrics_text**: a 1-3 sentence metrics blurb that includes at least one
+             number (e.g., "Produced 4 outputs, 3 reviewed, in under 1 second.").
+
+          4. **notes_text**: any freeform string — this one has no review attached.
+
+          Expected quality-gate result: three review tasks (one per output that has a
+          review configured). `notes_text` should not produce a review task.
+        outputs:
+          report_file:
+            review:
+              strategy: individual
+              instructions: |
+                Verify the file is a valid markdown document with a recognisable heading
+                or non-empty first line. Flag it if the file is empty or contains no
+                markdown structure.
+          summary_text:
+            review:
+              strategy: individual
+              instructions: |
+                Verify the value is a non-empty narrative summary of 1-2 sentences
+                written in plain English that describes some concrete outcome of the
+                step. Flag it if the value is empty, a placeholder, or does not describe
+                anything.
+          metrics_text: {}
+          notes_text: {}

CHANGELOG.md

@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - DeepSchema PostToolUse hook (`deepschema_write`) no longer reports `File is not valid JSON` for YAML files whose name has no extension (e.g. `.deepreview`). The hook now parses target files and the referenced JSON Schema as YAML, which is a superset of JSON, so both formats are accepted regardless of file extension. DW-REQ-011.7.3 updated to match. (Mirrors the fix shipped in #338 for the workflow quality gate.)
+- `review:` blocks declared on `type: string` step outputs are now actually executed. Previously they were silently dropped because the review pipeline only matched against file paths, leaving authors with misconfigured-but-silent quality gates. String output reviews now produce synthetic `ReviewTask` objects with the string value carried on a new `ReviewTask.inline_content` field and rendered into the instruction file as a "Content to Review" section. New requirements: JOBS-REQ-004.8, REVIEW-REQ-005.1.8, REVIEW-REQ-009.1.7. (#350)
 
 ### Removed
 ## [0.13.1] - 2026-04-06

Makefile

@@ -1,5 +1,17 @@
+# `make lint` mirrors CI (.github/workflows/validate.yml) in check-only mode
+# so that a clean local run guarantees CI will pass. Use `make lint-fix` to
+# auto-fix formatter and linter issues locally.
+
+.PHONY: lint lint-fix
+
 lint:
-	@echo "## make lint output"
+	@echo "## make lint output (check-only — matches CI)"
+	uv run ruff format --check src/ tests/
+	uv run ruff check src/ tests/
+	uv run mypy src/
+
+lint-fix:
+	@echo "## make lint-fix output (auto-fix)"
 	uv run ruff format src/ tests/
 	uv run ruff check --fix src/ tests/
 	uv run mypy src/

specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md

@@ -29,7 +29,7 @@ The quality review system evaluates step outputs against defined quality criteri
 4. Each rule's `instructions` MUST be prefixed with a preamble containing workflow `common_job_info` and input context (if available).
 5. Outputs with no review blocks MUST be skipped.
 6. Outputs with `None` values MUST be skipped.
-7. Only `file_path` type arguments with actual file paths MUST generate `ReviewRule` objects.
+7. Only `file_path` type arguments with actual file paths MUST generate `ReviewRule` objects. Reviews declared on `type: string` outputs are handled separately per JOBS-REQ-004.8.
 
 ### JOBS-REQ-004.4: Process Requirements
 
@@ -64,3 +64,12 @@ The quality review system evaluates step outputs against defined quality criteri
 3. For `string` inputs, values MUST be shown inline.
 4. Inputs with no value MUST be shown as "not available".
 5. If the step has no inputs, an empty string MUST be returned.
+
+### JOBS-REQ-004.8: String Output Review Tasks
+
+1. When a step output has `type: string` and a review block (either on the `StepOutputRef` or inherited from the `StepArgument`), `run_quality_gate()` MUST produce a synthetic `ReviewTask` whose content-to-review is the string value itself. Such reviews MUST NOT be silently ignored.
+2. The synthetic task MUST bypass file-pattern matching; the string value MUST be carried on `ReviewTask.inline_content` so the reviewer agent sees it inlined in its instruction file (per REVIEW-REQ-005.1.8).
+3. If a string output has reviews at both the `StepOutputRef` level and the `StepArgument` level, both MUST produce separate synthetic tasks, suffixed `_arg` for the argument-level task to match the file_path naming convention.
+4. The synthetic task's instructions MUST be prefixed with the same preamble used for file-based reviews (workflow `common_job_info` and input context).
+5. String outputs with `None` values MUST be skipped.
+6. The synthetic task's `review_id` MUST incorporate the string value into its content hash so that distinct string values produce distinct cache keys (per REVIEW-REQ-009.1.7).

specs/deepwork/review/REVIEW-REQ-005-instruction-generation.md

@@ -11,10 +11,11 @@ For each `ReviewTask`, the system generates a self-contained markdown instructio
 1. Each instruction file MUST be a valid markdown document.
 2. The file MUST begin with a heading identifying the review rule and scope (e.g., `# Review: python_file_best_practices — src/app.py`).
 3. The file MUST contain a "Review Instructions" section with the rule's resolved instruction text.
-4. The file MUST contain a "Files to Review" section listing the file paths to examine.
+4. The file MUST contain a "Files to Review" section listing the file paths to examine when the task has at least one file to review. Inline-content tasks (see REVIEW-REQ-005.1.8) MUST NOT include a "Files to Review" section.
 5. File paths in the "Files to Review" section MUST be relative to the repository root.
 6. When the task has `additional_files` (unchanged matching files), the file MUST contain an "Unchanged Matching Files" section listing those file paths.
 7. When the task has `all_changed_filenames`, the file MUST contain an "All Changed Files" section listing every changed filename for context.
+8. When the task has `inline_content` set (used for `type: string` step outputs — see JOBS-REQ-004.8), the file MUST contain a "Content to Review" section whose body is the inline content verbatim. The review heading scope MUST read `inline content` when the task has `inline_content` and no `files_to_review`.
 
 ### REVIEW-REQ-005.2: File Path Formatting
 

specs/deepwork/review/REVIEW-REQ-009-review-pass-caching.md

@@ -20,6 +20,7 @@ The mechanism relies on a deterministic `review_id` that encodes the rule name,
 4. The content hash MUST be the first 12 hex characters of the SHA-256 digest of the concatenated contents of all files to review, with files sorted alphabetically before concatenation.
 5. Files that cannot be read MUST contribute the placeholder string `MISSING` instead of their contents.
 6. The same inputs (rule name, file paths, file contents) MUST always produce the same `review_id`.
+7. For inline-content tasks (where `files_to_review` is empty and `inline_content` is set — used for `type: string` step outputs per JOBS-REQ-004.8), the file paths component MUST be the literal `inline` and the content hash MUST be derived from the inline string value so that distinct string values produce distinct `review_id`s.
 
 ### REVIEW-REQ-009.2: `mark_review_as_passed` MCP Tool
 

src/deepwork/jobs/mcp/quality_gate.py

@@ -92,6 +92,27 @@ def _collect_output_file_paths(
     return paths
 
 
+def _build_preamble(
+    step: WorkflowStep,
+    job: JobDefinition,
+    workflow: Workflow,
+    input_values: dict[str, ArgumentValue],
+) -> str:
+    """Build the preamble prefixed to every dynamic review's instructions.
+
+    Combines workflow ``common_job_info`` and the rendered step inputs.
+    Returns an empty string when neither is available.
+    """
+    input_context = _build_input_context(step, job, input_values)
+    common_info = workflow.common_job_info or ""
+    preamble_parts: list[str] = []
+    if common_info:
+        preamble_parts.append(f"## Job Context\n\n{common_info}")
+    if input_context:
+        preamble_parts.append(input_context)
+    return "\n\n".join(preamble_parts)
+
+
 def _build_input_context(
     step: WorkflowStep,
     job: JobDefinition,
@@ -145,16 +166,7 @@ def build_dynamic_review_rules(
     targets.
     """
     rules: list[ReviewRule] = []
-    input_context = _build_input_context(step, job, input_values)
-    common_info = workflow.common_job_info or ""
-
-    # Build preamble with common info and inputs
-    preamble_parts: list[str] = []
-    if common_info:
-        preamble_parts.append(f"## Job Context\n\n{common_info}")
-    if input_context:
-        preamble_parts.append(input_context)
-    preamble = "\n\n".join(preamble_parts)
+    preamble = _build_preamble(step, job, workflow, input_values)
 
     # Process each output
     for output_name, output_ref in step.outputs.items():
@@ -289,6 +301,83 @@ def build_dynamic_review_rules(
     return rules
 
 
+def build_string_output_review_tasks(
+    step: WorkflowStep,
+    job: JobDefinition,
+    workflow: Workflow,
+    outputs: dict[str, ArgumentValue],
+    input_values: dict[str, ArgumentValue],
+    project_root: Path,
+    platform: str = "claude",
+) -> list[ReviewTask]:
+    """Build synthetic ReviewTasks for ``type: string`` outputs with review blocks.
+
+    String outputs have no file to match against, so they cannot flow
+    through the normal ReviewRule/match_files_to_rules pipeline.  Instead,
+    this function creates ``ReviewTask`` objects directly, with the string
+    value carried on ``inline_content`` so the reviewer agent sees it
+    inlined in its instruction file.
+
+    Both output-ref-level and step_argument-level review blocks are
+    honored; when both are present, the argument-level task is suffixed
+    ``_arg`` to distinguish it (matching the file_path rule naming).
+    """
+    tasks: list[ReviewTask] = []
+    preamble = _build_preamble(step, job, workflow, input_values)
+
+    try:
+        source_rel = (job.job_dir / "job.yml").relative_to(project_root)
+    except ValueError:
+        source_rel = job.job_dir / "job.yml"
+    source_location = f"{source_rel}:0"
+
+    for output_name, output_ref in step.outputs.items():
+        arg = job.get_argument(output_name)
+        if not arg or arg.type != "string":
+            continue
+
+        review_blocks: list[ReviewBlock] = []
+        if output_ref.review:
+            review_blocks.append(output_ref.review)
+        if arg.review:
+            review_blocks.append(arg.review)
+
+        if not review_blocks:
+            continue
+
+        value = outputs.get(output_name)
+        if value is None:
+            continue
+        # Defensive: string outputs should always be str, but the runtime
+        # value may technically be any ArgumentValue.
+        inline_value = value if isinstance(value, str) else str(value)
+
+        for i, review_block in enumerate(review_blocks):
+            full_instructions = (
+                f"{preamble}\n\n{review_block.instructions}"
+                if preamble
+                else review_block.instructions
+            )
+            suffix = "_arg" if i > 0 else ""
+            rule_name = f"step_{step.name}_output_{output_name}{suffix}"
+            agent_name: str | None = None
+            if review_block.agent is not None:
+                agent_name = review_block.agent.get(platform)
+
+            tasks.append(
+                ReviewTask(
+                    rule_name=rule_name,
+                    files_to_review=[],
+                    instructions=full_instructions,
+                    agent_name=agent_name,
+                    source_location=source_location,
+                    inline_content=inline_value,
+                )
+            )
+
+    return tasks
+
+
 def run_quality_gate(
     step: WorkflowStep,
     job: JobDefinition,
@@ -321,6 +410,19 @@ def run_quality_gate(
         project_root=project_root,
     )
 
+    # 2b. Build synthetic ReviewTasks for type: string outputs with review blocks.
+    # These bypass file-pattern matching entirely — the string value is
+    # carried on the task via inline_content so the reviewer sees it inline.
+    string_output_tasks = build_string_output_review_tasks(
+        step=step,
+        job=job,
+        workflow=workflow,
+        outputs=outputs,
+        input_values=input_values,
+        project_root=project_root,
+        platform=platform,
+    )
+
     # 3. Load .deepreview rules
     deepreview_rules, _errors = load_all_rules(project_root)
 
@@ -354,7 +456,7 @@ def run_quality_gate(
         dynamic_tasks = match_files_to_rules(output_files, dynamic_rules, project_root, platform)
 
     # 7. Combine all tasks
-    all_tasks = dynamic_tasks + deepreview_tasks
+    all_tasks = dynamic_tasks + string_output_tasks + deepreview_tasks
 
     if not all_tasks:
         return None

src/deepwork/review/config.py

@@ -51,6 +51,7 @@ class ReviewTask:
     additional_files: list[str] = field(default_factory=list)  # Unchanged matching files
     all_changed_filenames: list[str] | None = None
     precomputed_info_bash_command: str | None = None  # Resolved command to run
+    inline_content: str | None = None  # Inline string value for type: string outputs
 
 
 def parse_deepreview_file(filepath: Path) -> list[ReviewRule]:

src/deepwork/review/formatter.py

@@ -137,13 +137,18 @@ def _task_name(task: ReviewTask) -> str:
     rule comes from a subdirectory .deepreview file.  This disambiguates
     same-named rules from different directories (REVIEW-REQ-004.10).
 
+    For inline-content tasks (type: string step outputs per JOBS-REQ-004.8)
+    the scope reads ``inline content`` instead of a file count.
+
     Args:
         task: The ReviewTask to name.
 
     Returns:
         Task name string.
     """
     prefix = _scope_prefix(task)
+    if not task.files_to_review and task.inline_content is not None:
+        return f"{prefix}{task.rule_name} review of inline content"
     if len(task.files_to_review) == 1:
         return f"{prefix}{task.rule_name} review of {task.files_to_review[0]}"
     return f"{prefix}{task.rule_name} review of {len(task.files_to_review)} files"