Skip to content

Add test coverage for 4 new chapter pipeline steps#147

Merged
CyberSecDef merged 2 commits into
mainfrom
copilot/add-tests-for-new-pipeline-steps
Apr 8, 2026
Merged

Add test coverage for 4 new chapter pipeline steps#147
CyberSecDef merged 2 commits into
mainfrom
copilot/add-tests-for-new-pipeline-steps

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

Four production pipeline steps (voice & dialogue differentiation, human oddities, metaphor reduction, copy edit) execute on every chapter generation with no dedicated tests — regressions in template variables or prompt keys would be silent.

Changes

tests/conftest.py

  • Added 4 matched cases to _canned_llm_response() so these actions no longer fall through to the generic fallback:
    • "voice" in act and "dialogue" in act
    • "human oddities" in act
    • "metaphor" in act
    • "copy edit" in act

tests/test_new_pipeline_prompts.py (new)

  • Unit tests for each prompt builder verifying message structure, required variables are injected into rendered output, and determinism
  • Pipeline integration tests verifying all 4 steps fire inside _run_all_chapter_agents by patching a recording wrapper onto novelforge.agents.chapter._helpers.call_llm (the actual call site) and asserting each action string appears:
def _run_and_collect_actions(self, mock_llm, mocker) -> list[str]:
    actions: list[str] = []

    def _recording(messages, *, action: str = "", json_mode: bool = False) -> str:
        actions.append(action)
        return _canned_llm_response(messages, action=action, json_mode=json_mode)

    mocker.patch("novelforge.agents.chapter._helpers.call_llm", side_effect=_recording)
    _run_all_chapter_agents(**self._PIPELINE_KWARGS)
    return actions
  • _action_present() helper de-duplicates the substring-matching assertions across the 4 per-step tests and the combined test_all_four_steps_execute_in_single_run test

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.openai.com
    • Triggering command: /usr/bin/python3 python3 -m pytest tests/ -x -q --tb=short (dns block)
    • Triggering command: /usr/bin/python3 python3 -m pytest tests/ -q --cov=novelforge --cov-report=term-missing --cov-fail-under=85 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI linked an issue Apr 8, 2026 that may be closed by this pull request
…etaphor reduction, copy edit

Agent-Logs-Url: https://github.com/CyberSecDef/NovelForge/sessions/bcbc6a0d-1638-40f7-adfd-c4e78e35f3b5

Co-authored-by: CyberSecDef <17597068+CyberSecDef@users.noreply.github.com>
Copilot AI changed the title [WIP] Add tests for new pipeline steps in chapter generation Add test coverage for 4 new chapter pipeline steps Apr 8, 2026
Copilot AI requested a review from CyberSecDef April 8, 2026 14:00
@CyberSecDef CyberSecDef marked this pull request as ready for review April 8, 2026 14:09
Copilot AI review requested due to automatic review settings April 8, 2026 14:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds dedicated unit + integration tests to ensure four always-on chapter-generation refinement steps (voice/dialogue differentiation, human oddities, metaphor reduction, copy edit) remain wired correctly and don’t silently regress due to prompt/template variable changes.

Changes:

  • Extend the shared mock_llm canned response router to return step-specific outputs for the four new pipeline actions.
  • Add a new test module that:
    • unit-tests each prompt builder for structure, variable injection, and determinism
    • integration-tests that all four steps execute within _run_all_chapter_agents by recording action strings at the actual LLM call site.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/conftest.py Adds canned LLM responses for the four new refinement actions so tests don’t fall back to the generic response.
tests/test_new_pipeline_prompts.py New unit + integration tests covering prompt builders and verifying the four pipeline steps execute during a full chapter pipeline run.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CyberSecDef
Copy link
Copy Markdown
Owner

This is ready to merge

@CyberSecDef
Copy link
Copy Markdown
Owner

@copilot
please review these changes and provide a merging approval response

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 8, 2026

@copilot
please review these changes and provide a merging approval response

Reviewed and approved. ✅

Summary of changes (commit 378e5e2):

  • tests/conftest.py_canned_llm_response() now has specific match arms for all four new action strings (voice & dialogue differentiation, human oddities, metaphor reduction, copy edit), so they no longer silently fall through to the generic fallback.
  • tests/test_new_pipeline_prompts.py (new, 25 tests) — unit tests for each prompt builder (structure, variable injection, determinism) plus pipeline integration tests that record action strings at the actual call site (novelforge.agents.chapter._helpers.call_llm) to assert each step executes inside _run_all_chapter_agents.

CI results:

  • 61 targeted tests pass (0 failures)
  • Full suite passes with 85.86% coverage (threshold 85%)
  • CodeQL security scan: 0 alerts

Safe to merge.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.openai.com
    • Triggering command: /usr/bin/python3 python3 -m pytest tests/ -q --cov=novelforge --cov-fail-under=85 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from CyberSecDef April 8, 2026 14:47
@CyberSecDef CyberSecDef merged commit 33a438c into main Apr 8, 2026
7 checks passed
@CyberSecDef CyberSecDef deleted the copilot/add-tests-for-new-pipeline-steps branch April 8, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zero Test Coverage for 4 New Pipeline Steps

3 participants