Fix: allow pre-SQL leads to convert via direct bypass

[shaypal5]

Summary

• Adds a rare direct conversion path for pre-SQL leads (mql, sal), breaking the deterministic is_sql=False → never converts invariant that leaked outcome information into features
• Daily probability = standard ConversionHazard rate × _DIRECT_CONVERSION_DISCOUNT (0.01), latent-score-modulated — high enough to break the invariant, low enough that the funnel still matters (~5–12x lower non-SQL conversion rate vs SQL)
• Fixes post-sim is_sql computation: uses state.sql_day is not None instead of also checking current_stage in _SQL_OR_DEEPER (which incorrectly flagged direct-converted leads as SQL since mark_converted() sets stage to closed_won)
• Extends opportunity/customer/subscription creation to handle direct-converted non-SQL leads

Closes #44

Test plan

• test_some_non_sql_leads_convert — with 2000 leads, at least one non-SQL lead converts
• test_non_sql_conversion_rate_much_lower — non-SQL rate is >5x lower than SQL rate
• test_direct_conversion_deterministic — same seed produces same labels
• test_direct_converted_lead_has_opportunity — direct-converted leads get opportunity rows
• test_direct_converted_lead_has_customer_and_subscription — direct-converted leads get customer + subscription rows
• All 793 existing tests pass (2 updated: test_most_converted_leads_are_sql, test_non_sql_non_converted_leads_no_opportunity)
• Lint + format clean

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds a rare “direct conversion” bypass for pre-SQL leads to eliminate the deterministic is_sql=False → never converts leakage invariant, while keeping the normal funnel as the dominant conversion path.

Changes:

• Introduces discounted direct conversion probability for mql/sal leads and adjusts is_sql derivation to rely solely on state.sql_day.
• Extends post-simulation entity creation so direct-converted non-SQL leads still produce opportunity/customer/subscription rows.
• Updates and adds tests to validate non-SQL conversions exist, remain much rarer than SQL conversions, and remain deterministic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
leadforge/simulation/engine.py	Adds direct conversion bypass, fixes is_sql computation, and creates opportunities for direct-converted leads.
tests/simulation/test_engine.py	Updates prior SQL/conversion assertions and adds a new test class covering direct conversion behavior.
.agent-plan.md	Documents the engine fix and the new/updated tests in the project plan.

---

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

[github-actions]

pr-agent-context report:

This run includes unresolved review comments on PR #45 in repository https://github.com/leadforge-dev/leadforge

For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.

After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.

# Copilot Comments

## COPILOT-1
Location: tests/simulation/test_engine.py:218
URL: https://github.com/leadforge-dev/leadforge/pull/45#discussion_r3175344690
Status: outdated
Root author: copilot-pull-request-reviewer

Comment:
    `test_sql_leads_are_flagged` still treats `closed_won` as implying `is_sql`, but direct conversion explicitly allows `closed_won` for non-SQL leads. This assertion can fail as soon as a direct conversion occurs in this 100-lead sim. Consider removing `closed_won` from the stage set (or asserting the implication only for non-terminal SQL+ stages like `sql`→`negotiation`).

## COPILOT-2
Location: tests/simulation/test_engine.py:226
URL: https://github.com/leadforge-dev/leadforge/pull/45#discussion_r3175344707
Root author: copilot-pull-request-reviewer

Comment:
    This assertion divides by `len(converted)` without guarding against the case where no leads convert for this seed/config, which would raise `ZeroDivisionError`. Add an assertion that `converted` is non-empty (or choose a smaller invariant that doesn’t require division).

## COPILOT-3
Location: tests/simulation/test_engine.py:431
URL: https://github.com/leadforge-dev/leadforge/pull/45#discussion_r3175344721
Root author: copilot-pull-request-reviewer

Comment:
    If `sql_rate` happens to be 0.0, the check `non_sql_rate < sql_rate / 5` becomes `non_sql_rate < 0` and will always fail (or be meaningless if both are 0). Add an explicit assertion that there is at least one SQL conversion (or compare absolute rates with a minimum-count requirement).

## COPILOT-4
Location: leadforge/simulation/engine.py:403
URL: https://github.com/leadforge-dev/leadforge/pull/45#discussion_r3175344737
Status: outdated
Root author: copilot-pull-request-reviewer

Comment:
    The module docstring’s “Post-simulation entity creation” section still states that opportunities are created only for leads that reached `sql` or deeper, but the implementation now also creates opportunities for direct-converted pre-SQL leads. Update the docstring to match the new behavior to avoid misleading future readers.

Run metadata:

Tool ref: v4
Tool version: 4.0.21
Trigger: commit pushed
Workflow run: 25234597720 attempt 1
Comment timestamp: 2026-05-01T21:47:47.747164+00:00
PR head commit: 56bbef3ae10b5bc4d508e4c2a7c4cae059ddc259