fix(sync): reconcile branch-diff fast-path scope with issue's explicitly requested modules (#1980)

[sohni-tagirisa]

Fixes #1980. Part of epic PR #1981.

Root cause

Step 7 of the issue-sync flow (run_agentic_sync, pdd/agentic_sync.py) committed to modules_to_sync = branch_modules unconditionally whenever _detect_modules_from_branch_diff() returned a non-empty set. The issue title/body and the loaded architecture inventory were already in scope at that point but never consulted, so the deterministic fast path silently under-scoped relative to what the issue explicitly requested. Repro (found by the #1868 E2E validation): issue explicitly asks to sync greeter and textutil; work-branch diff touches only greeter → only greeter synced, the greeter→textutil dependency edge dropped with a yellow warning, Success reported. The LLM-identify path scoped both correctly from the same issue.

Chosen semantics: deterministic union (still 0 LLM calls)

Before committing to the fast path, deterministically extract explicitly-named known modules from the issue title/body (_extract_issue_named_modules) and union any that the diff missed into the scope, with a clear log of which modules came from the diff vs the issue. Union was chosen over falling through to LLM identification because:

• it preserves the fast path's core contract (deterministic, free, reproducible) — a mismatch is exactly the case where the issue's explicit request is unambiguous, so no LLM judgment is needed;
• falling through would spend an LLM call to rediscover information we already hold deterministically, and would make scope non-reproducible between runs;
• issue-only modules go through the same downstream pipeline as every other scope source (_normalize_modules_for_sync, _filter_invalid_basenames, _drop_ambiguous_basenames (pdd sync should resolve modules by canonical path, not ambiguous short basenames #1677), dep-graph expansion), so nothing bypasses existing validation.

When the diff-detected scope already covers every issue-named module, behavior is byte-for-byte unchanged: same scope, same output, no LLM calls.

Precision/recall trade-off in extraction

Only tokens that unambiguously resolve against the known architecture inventory count as "explicitly named" — precision over recall, because a false positive here inflates a write-mode sync scope:

1. Backticked inline-code tokens (`greeter`, `prompts/greeter_python.prompt`) resolving to a known basename after .prompt/language-suffix stripping.
2. Prompt-file path tokens anywhere in the text (the FILES_MODIFIED-style lists the GitHub App writes) — this is the exact shape of the Epic: agentic sync E2E validation & force-local reliability #1868 repro fixture.
3. Bare word-boundary mentions, but only for basenames whose final component contains an underscore (e.g. ci_validation): such names cannot be ordinary prose words. A single-word module named e.g. python is never pulled in by prose ("written in python"); requesting it requires backticks or its prompt path. Case-sensitive; ambiguous tails (multiple architecture entries) are skipped, consistent with pdd sync should resolve modules by canonical path, not ambiguous short basenames #1677.

Accepted recall loss (documented on purpose): an issue that names a single-word module in plain prose without backticks or a prompt path does not trigger the union — behavior is then no worse than today. Extraction matches only title/body (not comments), mirroring where the issue's request lives; comments routinely quote logs naming unrelated modules.

Hard boundaries preserved

• _branch_diff_is_runtime_llm_only (chore: PDD sync for #879 #1396): untouched arm; runtime *_LLM templates never resolve during extraction (both the inventory and extract_module_from_include exclude them), covered by test_runtime_llm_templates_never_extracted.
• PDD_CHANGED_MODULES (pdd sync: consume PDD_CHANGED_MODULES as authoritative module-identification fast path #1883): arm untouched — see sibling note in comments.

Test plan (TDD: red pushed first, commit 854ae13)

• tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation — 7 helper unit tests (backticks, prompt paths, prose guard for a python-named module, underscored-name prose match, longer-token non-match, *_LLM exclusion, no-architecture no-op) + 3 run_agentic_sync integration tests (union repro asserting both modules reach dry-run and the runner with run_agentic_task never called; superset-unchanged; prose-non-inflation).
• tests/test_agentic_sync_mocked_e2e.py::test_branch_diff_scope_reconciled_with_issue_named_modules — hermetic real-CLI repro: work branch touches only greeter, fixture issue names both prompts → both child dry-run validations run, zero provider calls.
• Full tests/test_agentic_sync.py (247 passed) and full mocked E2E suite (5 passed); pylint pdd/agentic_sync.py introduces no new findings vs the epic baseline (verified by diffing pylint output; the resume E2E test's state fixture now marks both fixture modules succeeded because the fixture issue explicitly names both prompts).

🤖 Generated with Claude Code

[sohni-tagirisa]

Pre-fix investigation summary

Bug mechanics (verified on the epic base = current main):

• _detect_modules_from_branch_diff() (pdd/agentic_sync.py) diffs main...HEAD for changed non-_LLM .prompt files and returns path-qualified module keys.
• Step 7 of run_agentic_sync: if branch_modules: modules_to_sync = branch_modules — unconditional. title, body, filtered comments and the loaded architecture inventory are all in scope at that line but never consulted. Downstream, dep-graph expansion only sees the single diff module, so the greeter→textutil edge is dropped with a yellow "not in scope" style warning and the run reports Success.

Inventory / helper reuse: the flow already loads architecture via _load_architecture_json; _architecture_module_basenames() yields the known syncable basenames (runtime *_LLM entries excluded by _basename_from_architecture_filename). Suffix normalization reuses _strip_language_suffix_preserving_path / extract_module_from_include — no new normalization logic was invented. No pre-existing issue-text→module matcher existed (the only issue-text consumer was the LLM identify prompt), so _extract_issue_named_modules is new, placed next to the diff detector.

Prior art mined:

• pdd sync: consume PDD_CHANGED_MODULES as authoritative module-identification fast path #1883 (325d70f93): PDD_CHANGED_MODULES fast path — established the pattern of a deterministic scope source feeding the shared normalization/validation pipeline, with hard failure on unresolved targets.
• chore: PDD sync for #879 #1396 (runtime-LLM-only boundary): example of a carefully-bounded deterministic decision in the same step; its arm and semantics are untouched here, and extraction can never resolve a *_LLM template (regression-tested).
• pdd sync should resolve modules by canonical path, not ambiguous short basenames #1677 ambiguity handling and Carry a resolved sync unit through agentic sync #1675 path-qualified keys informed the coverage check (_issue_modules_missing_from_scope compares full keys and tails) and the decision to skip ambiguous bare tails during extraction.

Sibling hunt — PDD_CHANGED_MODULES arm (#1883): it can in principle also under-scope vs the issue text, but it is a different contract, not the same bug shape: the env var is an explicit, authoritative operator/App directive ("consume as authoritative" per #1883), it only fires when the branch diff is empty, and #1883 already hard-fails on unresolved targets rather than silently dropping. Overriding an explicit directive with issue-text inference would break that contract, so it is deliberately left unchanged; if reconciliation is ever wanted there, it should be a separate follow-up decision on #1883's semantics.

[sohni-tagirisa]

Red test output (commit 854ae13, before the fix)

The repro integration test fails with exactly the under-scope from #1980, and the helper unit tests fail on the missing extractor. The two passing tests are the intentional behavior-unchanged guards (superset scope, prose non-inflation), which must pass both before and after the fix.

  /Users/sohnitagirisa/.local/state/agent-worktrees/promptdriven__pdd/fix-1980/pdd/llm_invoke.py:1330: UserWarning: All LiteLLM cache configuration attempts failed. Caching is disabled.
    warnings.warn("All LiteLLM cache configuration attempts failed. Caching is disabled.")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_extracts_backticked_known_modules
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_extracts_prompt_file_paths
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_prose_word_matching_single_word_module_is_not_extracted
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_underscored_module_name_in_prose_is_extracted
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_underscored_name_inside_longer_token_is_not_extracted
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_runtime_llm_templates_never_extracted
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_no_architecture_extracts_nothing
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_union_issue_named_modules_missing_from_diff
=========== 8 failed, 2 passed, 237 deselected, 2 warnings in 2.48s ============

[sohni-tagirisa]

Mocked-E2E evidence (hermetic real-CLI run, post-fix)

The under-scope repro is exercised through the real code path (python -m pdd --force --local sync <issue-url> --no-steer --dry-run --no-github-state as a subprocess, all external binaries shimmed, zero billing): the fixture issue explicitly names both prompts/greeter_python.prompt and prompts/textutil_python.prompt, while the work branch commits a change to greeter only. The new test asserts (a) "Detected modules from branch diff" plus the reconciliation line "adding them to the sync scope: ['textutil']", (b) real child dry-run validation subprocesses ran for both greeter and textutil, and (c) claude_calls.log is empty — the fast path stayed at zero provider calls.

Full suite transcript (python3 -m pytest tests/test_agentic_sync_mocked_e2e.py -vv):

tests/test_agentic_sync_mocked_e2e.py::test_dry_run_pipeline_end_to_end PASSED [ 20%]
tests/test_agentic_sync_mocked_e2e.py::test_identification_failure_posts_error_comment PASSED [ 40%]
tests/test_agentic_sync_mocked_e2e.py::test_branch_diff_detection_skips_llm PASSED [ 60%]
tests/test_agentic_sync_mocked_e2e.py::test_branch_diff_scope_reconciled_with_issue_named_modules PASSED [ 80%]
tests/test_agentic_sync_mocked_e2e.py::test_resume_skips_succeeded_modules PASSED [100%]

=================== 5 passed, 2 warnings in 63.87s (0:01:03) ===================

(Re-run after the lint refactor: 5 passed in 68.43s.) Unit suite: python3 -m pytest tests/test_agentic_sync.py -q → 247 passed (includes the 10 new TestBranchDiffIssueScopeReconciliation tests). python3 -m pylint pdd/agentic_sync.py → no new findings vs the epic baseline (9.47/10; remaining findings in the touched line ranges are pre-existing, e.g. _branch_diff_is_runtime_llm_only R0911).

Note on test_resume_skips_succeeded_modules: its state fixture now marks both fixture modules succeeded, because the shared fixture issue explicitly names both prompts and the reconciled scope is correctly ['greeter', 'textutil']; the test's purpose (resume skips already-succeeded modules without any provider call) is unchanged.

[sohni-tagirisa]

Codex adversarial review — PR #1983 @ 09e2905

Verdict: DON'T MERGE
Findings: 3 total (2 P1, 1 P2)

P1 (blocking)

• [P1] Comment-only FILES_MODIFIED requests are ignored — pdd/agentic_sync.py:2871-2908
  The normal identify path preserves signal-bearing comments, including FILES_MODIFIED, but the branch-diff reconciliation extracts only from title and body. If the body only describes the issue and a later comment names prompts/textutil_python.prompt, the fast path still syncs only the diff modules and can report success while dropping the comment-requested module.
  Suggested fix: Run extraction over the same filtered issue content used for identify-modules, or pass filtered_comments into _extract_issue_named_modules and add a regression with a comment-only FILES_MODIFIED marker.

• [P1] Path-qualified duplicate-tail modules can be treated as already covered — pdd/agentic_sync.py:252-380
  Prompt-path tokens are reduced to just the filename, and _issue_modules_missing_from_scope treats any matching tail as covered. In a repo with extensions/a/src/page and extensions/b/src/page, a diff touching a/page can mask an issue request for backticked extensions/b/src/page or extensions/b/prompts/src/page_python.prompt, so the explicitly requested module is never added.
  Suggested fix: Preserve prompt-path qualification when converting prompt paths to module keys, and only use tail coverage when that tail is globally unambiguous; path-qualified issue modules should require exact full-key coverage.

P2 (non-blocking)

• [P2] Bare hyphenated module names are never extracted — pdd/agentic_sync.py:346-350
  The bare prose matcher only considers tails containing _, so identifier-like basenames such as github-app-api or check-run are missed unless the issue uses backticks or a prompt path. That leaves a false-negative class parallel to underscored names.
  Suggested fix: Include hyphenated identifier tails in the escaped boundary matcher, or explicitly document and test that hyphenated modules require backticks/path references.

Notes

• The runtime-only _LLM.prompt branch and PDD_CHANGED_MODULES fallback arm appear structurally unchanged.
• The mocked-E2E resume fixture update is legitimate for the current fixture because the issue body names both prompts.
• Targeted PR tests passed: 12 passed, with only existing LiteLLM cache warnings.

(Posted by the orchestrator on behalf of codex exec — sandboxed review had no network access. Orchestrator note: both P1s independently match my own review pass; confirmed FIX.)

[sohni-tagirisa]

Implementer response — round 2

Thanks — all three findings are valid. Plan per finding:

P1a (comment-only FILES_MODIFIED ignored) — Status: FIX. Extraction will scan the same signal-bearing content the identify path uses: _extract_issue_named_modules gains a comments parameter and the step-7 call site passes the already-computed filtered_comments (post-_filter_low_signal_comments, so machine-noise comments stay excluded). Regression: helper test with a comment-only FILES_MODIFIED marker, plus a run_agentic_sync integration test where only a comment names the second module.

P1b (duplicate-tail coverage masking) — Status: FIX. Two changes: (i) prompt-path tokens are converted to path-qualified module keys using the same <project-prefix>/<sub-path-under-prompts>/<stripped-basename> derivation as _detect_modules_from_branch_diff, instead of being reduced to the filename; (ii) _issue_modules_missing_from_scope now takes the architecture inventory and only accepts tail-based coverage when the tail maps to exactly one known module — path-qualified issue keys otherwise require full-key (or consistent path-suffix) coverage. Regression: duplicate-tail inventory (extensions/a/src/page + extensions/b/src/page), diff covers a's, issue names b's path-qualified → union happens; unit tests for both the extraction qualification and the coverage predicate.

P2 (hyphenated bare basenames never prose-matched) — Status: FIX (verified real). Checked the derivation: nothing normalizes hyphens away — _basename_from_architecture_filename("check-run_python.prompt") returns check-run (language-suffix stripping only splits on the final _<lang> token), and _filter_invalid_basenames' non-prompt fallback stems (e.g. frontend *.tsx entries) make hyphenated identifiers plausible inventory members. The class-3 gate extends from _ to _ or - in the tail; the existing boundary lookarounds already exclude --adjacent longer identifiers, so precision is preserved. Test: hyphenated basename mentioned in prose is extracted; embedded in a longer hyphenated token is not.

Process: red tests first as a separate test(sync): commit pushed before the fix, then implementation + targeted pytest + pylint, then red/green evidence comment.

[sohni-tagirisa]

Round-2 red/green evidence

Red (commit 11c9122, tests-only, pushed before the fix) — 8 failed / 13 passed in TestBranchDiffIssueScopeReconciliation:

E       TypeError: _extract_issue_named_modules() got an unexpected keyword argument 'comments'
tests/test_agentic_sync.py:5993: TypeError
>       assert named == ["extensions/b/src/page"]
E       AssertionError: assert [] == ['extensions/b/src/page']
tests/test_agentic_sync.py:6020: AssertionError
E       TypeError: _issue_modules_missing_from_scope() takes 2 positional arguments but 3 were given
tests/test_agentic_sync.py:6037: TypeError
E       TypeError: _issue_modules_missing_from_scope() takes 2 positional arguments but 3 were given
tests/test_agentic_sync.py:6049: TypeError
E       TypeError: _issue_modules_missing_from_scope() takes 2 positional arguments but 3 were given
tests/test_agentic_sync.py:6059: TypeError
>       assert named == ["check-run"]
E       AssertionError: assert [] == ['check-run']
tests/test_agentic_sync.py:6075: AssertionError
        assert success is True
        mock_agentic_task.assert_not_called()
>       assert sorted(dry_run_modules) == ["greeter", "textutil"]
E       AssertionError: assert ['greeter'] == ['greeter', 'textutil']
tests/test_agentic_sync.py:6148: AssertionError
        assert success is True
        mock_agentic_task.assert_not_called()
>       assert sorted(dry_run_modules) == [
E       AssertionError: assert ['extensions/a/src/page'] == ['extensions/...s/b/src/page']
tests/test_agentic_sync.py:6211: AssertionError
  /Users/sohnitagirisa/.local/state/agent-worktrees/promptdriven__pdd/fix-1980/pdd/llm_invoke.py:1330: UserWarning: All LiteLLM cache configuration attempts failed. Caching is disabled.

Failures map 1:1 to the findings: P1a (test_extracts_modules_from_filtered_comments, test_union_from_comment_only_files_modified), P1b (test_prompt_path_tokens_preserve_path_qualification, test_duplicate_tail_does_not_mask_missing_issue_module, test_union_path_qualified_duplicate_tail_not_masked, plus signature-change reds on the two coverage guards), P2 (test_hyphenated_module_name_in_prose_is_extracted). test_backticked_path_qualified_key_resolves_exactly and test_hyphenated_name_inside_longer_token_is_not_extracted pass pre-fix by design (guards).

Green (commit 84ae8d0):

• python3 -m pytest tests/test_agentic_sync.py -q → 258 passed (21/21 in the reconciliation class)
• python3 -m pytest tests/test_agentic_sync_mocked_e2e.py -q → 5 passed in 72.49s
• python3 -m pylint pdd/agentic_sync.py → 9.49/10, zero findings in the touched line ranges (baseline epic tip: 9.47)

One deliberate deviation from the suggested P1b fix: instead of adding path-suffix coverage for path-qualified issue keys, coverage is now strictly exact full key OR globally-unambiguous tail. A path-suffix rule reintroduces the same masking through bare or partially-qualified keys (e.g. issue key page vs diff extensions/a/src/page), while a globally-unique tail provably refers to one module and safely bridges the key-form difference between the diff derivation (repo path, #1675) and architecture basenames. Duplicate-tail requests therefore require the exact known key — which is precisely what the extraction now produces from qualified prompt paths and backticked keys.

[sohni-tagirisa]

Codex adversarial review — PR #1983 @ 84ae8d0

Verdict: DON'T MERGE
Findings: 3 total (2 P1, 1 P2)

P1 (blocking)

• [P1] Duplicate filename entries still mask path-qualified issue modules — pdd/agentic_sync.py:382-460
  The stricter exact-or-globally-unique-tail rule is correct for the new directory-qualified test fixture, but it is not complete for existing supported architectures where distinct modules share the same filename basename and differ by prompt subdir/filepath (for example two Next.js page_TypeScriptReact.prompt entries). _architecture_module_basenames() dedupes those to one page, so prompts/app/settings/page_TypeScriptReact.prompt resolves to bare page, and _issue_modules_missing_from_scope(["app/login/page"], ["page"], arch) returns []. That means a diff touching login still masks an explicit settings-page request, reopening the agentic sync: branch-diff fast path silently under-scopes vs issue's explicitly requested modules #1980 under-scope class.
  Suggested fix: Build the extraction/coverage inventory from real module identities including duplicate architecture entries and path/filepath-derived choices, and preserve path-qualified prompt-path tokens as scheduler keys instead of collapsing them to a bare known tail.

• [P1] Prompt path tokenizer drops supported route-segment characters — pdd/agentic_sync.py:234-290
_ISSUE_PROMPT_PATH_RE only allows alnum, _, ., /, and -, so valid PDD/Next.js paths such as prompts/app/users/[id]/page_TypeScriptReact.prompt are tokenized as only page_TypeScriptReact.prompt. In duplicate page route projects this either extracts nothing or collapses to the wrong bare tail, so an explicit FILES_MODIFIED path for a dynamic route can still be silently omitted from the widened scope.
  Suggested fix: Replace the narrow prompt-path regex with a tokenizer that captures full non-whitespace .prompt paths while trimming markdown/list punctuation, and add a duplicate dynamic-route regression test.

P2 (non-blocking)

• [P2] Surviving workflow comments and fenced noise can inflate scope — pdd/agentic_sync.py:330-343
  The new comment scan concatenates every body that survives _filter_low_signal_comments, but that filter intentionally only removes a few PDD sync/state prefixes. Other machine comments like ## Step 9/13... survive, and _issue_candidate_tokens then treats any prompt path anywhere in those bodies (or fenced logs/code blocks in the issue text) as an explicit requested module. A stale or abandoned bot step comment containing FILES_MODIFIED: prompts/textutil_python.prompt can therefore add textutil even when the current title/body and branch diff do not request it.
  Suggested fix: For comments, scan only explicitly trusted marker payloads (or ignore known workflow step comments except the intended FILES_MODIFIED source), and avoid applying bare-name extraction to comment/log/code-fence text.

Notes

• The runtime-LLM-only and PDD_CHANGED_MODULES arms remain structurally unchanged.
• The two-module mocked-E2E resume fixture is a legitimate consequence for the greeter/textutil fixture.

(Round-2 re-review; posted by the orchestrator on behalf of codex exec.)

[sohni-tagirisa]

Implementer response — round 3

All three findings are valid; all three are FIXING. The duplicate-page shape is confirmed real production data (pdd_cloud's Next.js routes, including bracketed dynamic segments), so P1s are treated as the App's normal case, not an edge.

P1#1 (duplicate-filename entries mask path-qualified requests) — Status: FIXING. Root cause: extraction/coverage consumed _architecture_module_basenames, which dedupes by derived basename, collapsing two same-filename entries into one identity. Fix: a real-identity index built per entry (duplicates preserved) — primary identity from the filename derivation, exact-match aliases additionally from _architecture_entry_aliases (filename + filepath-derived keys). Rules: tail-based resolution/coverage only when the tail maps to exactly one DISTINCT real identity (pre-dedup); path-qualified issue keys otherwise require exact coverage; and a prompt-path token that derives a path-qualified key via _module_key_from_prompt_path is accepted directly as a scheduler key (same key form the branch-diff detector already emits, #1675) provided its tail exists in the inventory. Regression: two same-tail entries, diff covers one, issue names the other via prompt path → union with the path-qualified key.

P1#2 (tokenizer drops [/] route segments) — Status: FIXING. _ISSUE_PROMPT_PATH_RE becomes a non-whitespace .prompt capture with leading markdown punctuation trimmed (backticks/quotes/parens; the .prompt anchor already excludes trailing punctuation), so prompts/app/users/[id]/page_TypeScriptReact.prompt keeps its full path. All downstream matching uses re.escaped tokens or plain string comparison, so brackets are inert. Regression: dynamic-route FILES_MODIFIED path resolves to the right path-qualified module in a duplicate-tail inventory.

P2 (surviving workflow comments / fenced noise inflate scope) — Status: FIXING. (a) Comments are no longer scanned as free text at all: after stripping fenced code blocks, only FILES_MODIFIED: marker payloads are consumed — parsed with the same _extract_marker_paths walker the change orchestrator produces/consumes those markers with — and only as class-2 prompt-path tokens. A marker quoted inside a log fence is treated as a log, not a directive. (b) In title/body, class-3 bare-name extraction runs on fence-stripped text (an unclosed fence conservatively drops the remainder); class-1 backticks and class-2 prompt paths stay global. Tests: fenced stale step-comment must not inflate scope; a genuine FILES_MODIFIED marker line must.

Consciously unfixed (ACCEPTED, recall-only): a backticked bare name whose tail is duplicated across real identities (e.g. `page`) still resolves to nothing — with duplicate identities there is no deterministic referent; the request needs a path-qualified key or prompt path (mirrors #1677's ambiguity contract). Also, a stale-but-well-formed FILES_MODIFIED marker outside a fence in a surviving comment remains indistinguishable from a genuine one by format; scope inflation there is bounded to known-inventory prompt paths and mirrors what the LLM-identify path reads from the same comment.

Process: red tests first as a separate test(sync): commit pushed before the fix, then implementation + targeted pytest + pylint, then evidence reply.

[sohni-tagirisa]

Round-3 red/green evidence

Red (commit c7186e0, tests-only, pushed before the fix) — 8 failed / 23 passed in TestBranchDiffIssueScopeReconciliation:

  /Users/sohnitagirisa/.local/state/agent-worktrees/promptdriven__pdd/fix-1980/pdd/llm_invoke.py:1330: UserWarning: All LiteLLM cache configuration attempts failed. Caching is disabled.
    warnings.warn("All LiteLLM cache configuration attempts failed. Caching is disabled.")
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_duplicate_filename_entries_do_not_collapse_to_one_identity
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_bare_backtick_of_duplicated_tail_resolves_to_nothing
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_duplicate_filename_tail_does_not_cover_missing_module
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_bracketed_dynamic_route_prompt_path_is_tokenized
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_comment_prompt_path_outside_marker_is_ignored
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_comment_marker_inside_log_fence_is_ignored
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_fenced_code_in_body_does_not_bare_name_match
FAILED tests/test_agentic_sync.py::TestBranchDiffIssueScopeReconciliation::test_union_duplicate_filename_prompt_path_not_masked
8 failed, 23 passed, 237 deselected, 2 warnings in 1.81s

Failure-to-finding map: P1#1 — test_duplicate_filename_entries_do_not_collapse_to_one_identity, test_bare_backtick_of_duplicated_tail_resolves_to_nothing, test_duplicate_filename_tail_does_not_cover_missing_module, test_union_duplicate_filename_prompt_path_not_masked; P1#2 — test_bracketed_dynamic_route_prompt_path_is_tokenized; P2 — test_comment_prompt_path_outside_marker_is_ignored, test_comment_marker_inside_log_fence_is_ignored, test_fenced_code_in_body_does_not_bare_name_match. The two passing new tests (test_comment_genuine_marker_line_still_extracts, test_bare_name_outside_fence_still_matches) are pre-fix guards that must stay green.

Green (commit 8bd1575):

• python3 -m pytest tests/test_agentic_sync.py -q → 268 passed (31/31 in the reconciliation class)
• python3 -m pytest tests/test_agentic_sync_mocked_e2e.py -q → 5 passed in 67.10s
• python3 -m pylint pdd/agentic_sync.py → 9.51/10, zero findings in the touched line ranges (epic baseline: 9.47)

Implementation notes: the identity index reuses the existing per-entry alias derivation (_architecture_entry_aliases: filename- and filepath-derived keys) rather than inventing a new one; the trusted-marker parser is the change orchestrator's own _extract_marker_paths (bullets, inline terminators, multi-line payloads all behave exactly as the marker producer defines); and a prompt-path-derived qualified key is scheduled in the same key form the branch-diff detector emits, which downstream validation (_filter_invalid_basenames tail check, #1677 ambiguity handling) already accepts. Accepted recall trade-offs stated in the round-3 response comment stand: a bare backticked name colliding with a duplicated tail resolves to nothing (needs a path), and a well-formed unfenced FILES_MODIFIED marker in a surviving comment is trusted by format.

[sohni-tagirisa]

Codex adversarial review — PR #1983 @ 8bd1575

Verdict: DON'T MERGE
Findings: 4 total (2 P1, 2 P2)

P1 (blocking)

• [P1] Combined architecture duplicates still collapse across project scopes — pdd/agentic_sync.py:328-334
load_combined_architecture_data() concatenates nested architecture entries without source-path metadata, but the new index dedupes on only (primary, filepath_str). Two projects can legitimately both declare filename: "src/page_python.prompt" and filepath: "src/page.py"; the second entry is skipped, so tail_to_identities["page"] looks globally unique. Then a branch diff for extensions/a/src/page can be considered to cover an issue request for extensions/b/prompts/src/page_python.prompt, silently recreating the under-scope failure this PR is meant to prevent.
  Suggested fix: Preserve architecture scope in the identity key, or stop deduping architecture-only entries unless they are proven to be the same absolute output.

• [P1] Prompt-path scheduler keys can lose dependency edges — pdd/agentic_sync.py:498-503
  The direct path-token branch emits keys like app/settings/page when only the tail exists in the architecture. That key can be accepted by the child sync via filepath-suffix matching, but _build_targeted_dep_graph() later asks build_dep_graph_from_architecture_data() to find an architecture entry by aliases, where flat-filename entries expose page / src/app/settings/page, not app/settings/page. The result is an empty dependency list with no warning for a newly widened module that may have declared dependencies.
  Suggested fix: Resolve prompt-path-derived keys back to the matched architecture entry, or teach targeted dependency graph construction the same filepath-suffix matching used by sync resolution.

P2 (non-blocking)

• [P2] Fenced issue-body tokens still widen scope — pdd/agentic_sync.py:557-562
  Comments strip fences before marker parsing, and class-3 body matching uses stripped text, but class-1 backticks and class-2 prompt paths scan the raw title/body. A fenced prior-run log in the issue body containing FILES_MODIFIED: prompts/textutil_python.prompt still adds textutil to the write scope.
  Suggested fix: Remove fenced code blocks from the issue body before class-1/class-2 extraction as well.

• [P2] Longer Markdown fences are parsed as alternating triple fences — pdd/agentic_sync.py:261-283
  Valid Markdown fences can be four or more backticks/ tildes. The stripper stores only the first three characters, so a four-backtick quote containing a triple-fenced snippet can leak the text between inner triple fences; that leaked text can include a marker payload or bare module name.
  Suggested fix: Track the full opening fence character and length, and close only on a fence of the same character with at least that length.

Notes

• Targeted reconciliation tests and the two updated mocked-E2E tests passed locally.

(Round-3 re-review; posted by the orchestrator on behalf of codex exec. Orchestrator triage: P1s go to verify-first — implementer must confirm or refute against load_combined_architecture_data and _build_targeted_dep_graph behavior before coding; both P2s are mechanical FIXes.)

[sohni-tagirisa]

Implementer response — round 4

Branch rebased onto the updated epic base (263b492, post-#1982 squash) via clean cherry-pick; gh pr diff still shows only the three PR files. Verdicts after verify-first:

P1#1 (cross-project identity collapse) — Status: VERIFIED, FIXING. Confirmed against the source: load_combined_architecture_data (pdd/architecture_registry.py) concatenates every architecture file's entries with combined.extend(extract_modules(data)) — no source-path qualification whatsoever — so two nested projects that both declare filename: "src/page_python.prompt" / filepath: "src/page.py" yield two string-identical entries, and my (primary, filepath) dedupe merges them into one falsely-unique tail. Fix: drop the dedupe entirely. With an unqualified combined inventory, string-identical entries are indistinguishable, so the only safe deterministic treatment is ambiguity (no bare resolution, no tail coverage — exact/path-qualified requests still work). Cost is recall-only, confined to a module literally double-listed with identical strings; that trade-off is stated in the code comment.

P1#2 (dependency edges lost for prompt-path-derived keys) — Status: VERIFIED (empirically), FIXING. Reproduced directly: build_dep_graph_from_architecture_data(arch, ["app/settings/page", "textutil"]) returns {'app/settings/page': [], ...} with zero warnings, while the filepath alias src/app/settings/page resolves the declared textutil edge. Note the gap predates this PR — a plain branch-diff key for the same file (prompts/app/settings/page_TypeScriptReact.prompt → app/settings/page) hits the identical silent edge loss — my widening just extended its reach. Fix at graph construction (Codex's second suggested option, which heals both producers): _build_targeted_dep_graph pre-resolves any group target that fails alias matching to the UNIQUELY suffix-matched entry's filepath-derived alias before calling the builder, and remaps the result back to the original scheduler keys. Scheduler keys themselves stay unchanged — deliberately, because the diff detector emits the prompt-subpath form for the same file, and switching the issue-side key to a filepath alias would break exact coverage in the superset case and double-schedule one module. Regression test asserts the widened module's declared dependencies survive into the dep graph, with a uniqueness guard (ambiguous suffix → left unresolved, existing behavior).

P2#1 (fenced title/body tokens widen scope via classes 1/2) — Status: FIXING. Mechanical: title/body is fence-stripped once, before all three classes (comments were already stripped before marker parsing). Test: fenced prior-run log containing a FILES_MODIFIED line and an inline-backticked module name must not widen scope; unfenced tokens still do.

P2#2 (4+ char fences leak inner triple fences) — Status: FIXING. Mechanical: the stripper now records the opening fence character and length and closes only on a fence of the same character with >= length (CommonMark rule). Test: a ````-fenced block containing an inner ``` pair leaks nothing.

Process: red tests first as a separate test(sync): commit pushed before the fix, then implementation + targeted pytest + mocked E2E + pylint, then evidence reply.