fix(pipeline): report dynamic hint extraction failures on update path…

[Shivang9983]

… (#1697)

Summary

• Replaced silent except Exception: pass block in repowise.core.pipeline.incremental with structured structlog warning and a
  user-facing consequence log.

• Preserved best-effort non-failing behavior during incremental updates when dynamic hint extraction fails.

• Added regression test test_dynamic_hint_failure_logs_warning_and_does_not_raise to assert proper warning emission without
  crashing graph construction.

Related Issues

Fixes #1697

Test Plan

Ran unit test suite locally:

pytest tests/unit/pipeline/test_update_graph_convergence.py

- [ x ] Tests pass (`pytest`)
- [ ] Lint passes (`ruff check .`)
- [ ] Web build passes (`npm run build`) *(if frontend changes)*

## Checklist

- [ x ] My code follows the project's code style
- [ x ] I have added tests for new functionality
- [ x ] All existing tests still pass
- [ ] I have updated documentation if needed

[repowise-bot]

✅ Health of changed files: 1.5 → 1.6 (+0.1)

📋 At a glance
2 hotspots touched · 2 co-change pairs left out · 1 file with recent fix history · 1 dead-code finding.

Files & modules (2)

• packages (1 file)
  • .../pipeline/incremental.py
• tests (1 file)
  • .../pipeline/test_update_graph_convergence.py

✅ Health gate: passed

📌 Before you merge

• Run .../dead_code/test_partial.py, .../health/test_module_attribution.py, .../health/test_perf_incremental_parity.py: they import the changed files
• .../pipeline/persist.py changed together with .../pipeline/incremental.py in 14 past commits and isn't in this PR
• .../update_cmd/persistence.py changed together with .../pipeline/incremental.py in 13 past commits and isn't in this PR

🔎 More signals (4)

🗺️ Change map

flowchart LR
  subgraph PR ["Changed in this PR (1 with dependents)"]
    f_packages_core_src_repowise_core_pipeline_incremental_py[".../pipeline/incremental.py 🔥"]:::changed
  end
  f_packages_cli_src_repowise_cli_commands_dead_code_cmd_py[".../commands/dead_code_cmd.py"]
  f_packages_core_src_repowise_core_pipeline_incremental_py --> f_packages_cli_src_repowise_cli_commands_dead_code_cmd_py
  f_packages_cli_src_repowise_cli_commands_update_cmd_command_py[".../update_cmd/command.py"]
  f_packages_core_src_repowise_core_pipeline_incremental_py --> f_packages_cli_src_repowise_cli_commands_update_cmd_command_py
  f_packages_cli_src_repowise_cli_commands_update_cmd_incremental_py[".../update_cmd/incremental.py"]
  f_packages_core_src_repowise_core_pipeline_incremental_py --> f_packages_cli_src_repowise_cli_commands_update_cmd_incremental_py
  f_packages_cli_src_repowise_cli_commands_update_cmd_persistence_py[".../update_cmd/persistence.py"]
  f_packages_core_src_repowise_core_pipeline_incremental_py --> f_packages_cli_src_repowise_cli_commands_update_cmd_persistence_py
  more(["+7 more dependents"])
  PR --> more
  w_packages_core_src_repowise_core_pipeline_persist_py(["⚠️ .../pipeline/persist.py changed together 14×, not in PR"]):::warn
  f_packages_core_src_repowise_core_pipeline_incremental_py -.- w_packages_core_src_repowise_core_pipeline_persist_py
  w_packages_cli_src_repowise_cli_commands_update_cmd_persistence_py(["⚠️ .../update_cmd/persistence.py changed together 13×, not in PR"]):::warn
  f_packages_core_src_repowise_core_pipeline_incremental_py -.- w_packages_cli_src_repowise_cli_commands_update_cmd_persistence_py
  t_tests_unit_dead_code_test_partial_py(["✅ .../dead_code/test_partial.py"]):::guard
  t_tests_unit_dead_code_test_partial_py -.-> f_packages_core_src_repowise_core_pipeline_incremental_py
  classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
  classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
  classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d

Loading

_{Solid arrows: code that imports the changed files (11 direct dependents, from the last indexed snapshot). Dashed: history/tests.}

🔥 Hotspots touched (2)

• .../pipeline/test_update_graph_convergence.py: 1 commits/90d, 0 dependents · primary owner: Raghav Chamadiya (100%)
• .../pipeline/incremental.py: 42 commits/90d, 27 dependents · primary owner: Raghav Chamadiya (93%)

🔗 Hidden coupling (1 file)

• .../pipeline/incremental.py co-changes with these files (not in this PR):
  • .../pipeline/persist.py (14×, 🟡 notable)
  • .../update_cmd/persistence.py (13×, 🟡 notable)

💀 Dead code (1 finding)

• 💀 .../pipeline/incremental.py _noop_log (confidence 0.65)

👀 Suggested reviewers @RaghavChamadiya

---

📊 See the full report for this PR
_{Your repo map with this PR's blast radius lit up, every caller of the contracts it changes, and health before and after. No sign-in. · ⭐ Star Repowise · 📥 Install bot · Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot · Updated 2026-08-21 07:36 UTC (since the last push: health 1.9 to 1.6)}

[RaghavChamadiya]

Thanks @Shivang9983, and thanks for leaving the ruff box unticked rather than ticking it and hoping. That told me straight away which failure to look at first.

You have read #1697 correctly. The comment sitting above the block you did not touch makes the case for the one you did:

    # Add framework-aware synthetic edges (conftest, Django, FastAPI, Flask).
    # Best-effort, but not silent: the update path has to report the failure the
    # init path reports, or an index degrades differently depending on how it
    # was built and nothing says which.

Two blocks apart, same file, and the dynamic-hint one was still except Exception: pass. So this is a gap rather than a disagreement about intent, which is the best kind of fix to make.

The CI failure. Ruff, four instances of the same rule, all in your new test:

RUF059 Unpacked variable `parsed_files` is never used
   --> tests/unit/pipeline/test_update_graph_convergence.py:128:9
RUF059 Unpacked variable `source_map` is never used
RUF059 Unpacked variable `repo_structure` is never used
RUF059 Unpacked variable `count` is never used

You only assert on graph_builder, so prefixing the other four with an underscore clears it:

        _parsed_files, _source_map, graph_builder, _repo_structure, _count = build_repo_graph(

One thing I would change, and it makes the diff smaller. That framework-edges except immediately above is the pattern to copy verbatim:

    except Exception as fw_exc:
        logger.warning("framework_edges_failed", error=str(fw_exc))
        log(
            f"[yellow]Framework edge detection skipped: {fw_exc}; framework-invoked "
            "symbols will have no callers in the graph and may read as dead.[/yellow]"
        )

Yours differs in two ways worth closing. It passes exc_info=True, which prints a full traceback into a user-facing update run for a failure the code has already decided is survivable; its sibling deliberately does not. And the log(...) call is one long line where the sibling wraps, which also keeps it clear of the line-length limit. Matching it exactly means the two best-effort blocks read the same way, which is the whole point of the issue.

A few small things while you are in there: there is trailing whitespace on the blank lines inside and after the try, the file has no newline at the end, and the diff removes the blank line between the module docstring and from __future__, which is unrelated to this change and worth putting back.

None of that changes your diagnosis, which is right. Push those and I will approve the workflow run so you get a full matrix, then merge.

[Ayush7614]

Request Changes

Intent is correct — dynamic hints are best-effort and should warn instead of silent pass. No fake code: packages/core/src/repowise/core/pipeline/incremental.py:29 logger = structlog.get_logger(__name__) exists, logger.warning("dynamic_hints_extraction_failed", error=str(exc), exc_info=True) at incremental.py:193 is valid, and build_repo_graph(...):192-194 log("[yellow]Dynamic hint extraction failed...") matches LogFn usage elsewhere.

Blocking — CI lint FAILURE (Python lint (ruff) FAILURE on runs/32370307094/job/96690208258, mergeStateStatus: BLOCKED):

• packages/core/src/repowise/core/pipeline/incremental.py:191 trailing whitespace + 195:1 blank line with spaces — ruff format --check fails:

  191: '            '
  195: '        '
  

• tests/unit/pipeline/test_update_graph_convergence.py:14 missing blank line after module docstring, 107 line-length, 128 RUF059 4 unused unpacks (parsed_files, source_map, repo_structure, count), 132-134 extra blank lines + missing wrapping — ruff check reports 4 RUF059 + ruff format diff. File also missing final newline (\ No newline at end of file).

Fix:

ruff format packages/core/src/repowise/core/pipeline/incremental.py tests/unit/pipeline/test_update_graph_convergence.py
# then fix RUF059 — prefix unused with _:
_parsed_files, _source_map, _graph_builder, _repo_structure, _count = build_repo_graph(...)
# or use _ = build_repo_graph(...) and assert returned graph_builder
# + ensure EOF newline

Non-blocking suggestions:

• Test at test_update_graph_convergence.py:119-138 only asserts capture_log contains "Dynamic hint extraction failed" / "Dynamic edges will be missing" — it never asserts structured logger.warning was called with exc_info=True. Consider adding patch("repowise.core.pipeline.incremental.logger.warning") assertion or renaming test to reflect console-only check.
• Patch target repowise.core.ingestion.dynamic_hints.HintRegistry.extract_all works because HintRegistry is imported inside build_repo_graph:181 after patch starts — fine, but document that coupling; if refactored to top-level import, target must change.

Please run ruff check && ruff format --check locally and push — happy to approve once lint is green.

[Shivang9983]

Thanks for the detailed review @RaghavChamadiya @Ayush7614 I have addressed all the linting and formatting feedback:

Prefixed unused unpack variables with _ in test_update_graph_convergence.py (RUF059 resolved).

Aligned the dynamic hint exception handler with the sibling handler (removed exc_info=True, wrapped strings, and cleaned up whitespace/newlines).

Formatted both files using ruff format and verified with ruff check.

[Ayush7614]

Approved — lint fixed

Re-review after 21ec002 style(pipeline): resolve ruff lint errors.

Verification (pr-1772-new 21ec002):

• packages/core/src/repowise/core/pipeline/incremental.py:214-219 — trailing whitespace / blank lines fixed, now logger.warning("dynamic_hints_extraction_failed", error=str(exc)) + log("[yellow]Dynamic hint extraction failed: ... Dynamic edges will be missing...") split over 2 lines matching sibling framework_edges_failed:192 format. ruff check ✅ ruff format --check ✅ (both files).
• tests/unit/pipeline/test_update_graph_convergence.py:119-135 — RUF059 fixed (_parsed_files, _source_map, _repo_structure, _count), line-length collapse at subgraph_signature line, blank-line + EOF newline fixed; ruff ✅.
• statusCheckRollup now shows Repowise / code health SUCCESS; full CI matrix not visible for this fork push (previously 8 jobs) but local ruff is the former blocker and is now clean. No fake APIs — logger = structlog.get_logger(__name__) at incremental.py:29 exists, HintRegistry.extract_all patch target correct (import inside build_repo_graph:203 so patch before call works).

Previous CHANGES_REQUESTED was solely for lint (Python lint (ruff) FAILURE, mergeStateStatus: BLOCKED). That is resolved — exc_info=True removal to match sibling is intentional per commit message and fine for structured logging.

LGTM — ready to merge (dismissing previous request).

[Shivang9983]

Hi @RaghavChamadiya

Just following up on this PR. The suggested linting and formatting changes have been addressed, local checks are green, and it has received an initial review approval.

Whenever you get a moment, could you please take a look and approve the workflow run / merge? Let me know if any further changes are needed.

Thanks!

[RaghavChamadiya]

Thanks @Shivang9983, and sorry for the wait after you turned this around so quickly. You did everything I asked: the four RUF059 unpacks are underscore-prefixed, the handler now matches its sibling exactly (no exc_info, wrapped strings), and the whitespace and trailing-newline points are closed. Full matrix green on 3.11, 3.12, 3.13 and ruff.

The reason this is worth having is the one from the review: the framework-edges handler two blocks above already said an update path has to report what the init path reports, or an index degrades differently depending on how it was built and nothing says which. The dynamic-hint block was the gap in that, not a disagreement with it.

One thing I am merging anyway rather than sending you round again for a single character. The adjacent string literals join without a space:

f"[yellow]Dynamic hint extraction failed: {exc}. Dynamic edges"
"will be missing from this update.[/yellow]"

renders as Dynamic edgeswill be missing from this update. You have waited long enough on this one, so I will take that space in a follow-up rather than hold the fix for it. Flagging it because it is a genuinely easy trap: implicit concatenation across a line break swallows the space at the seam every time, and the sibling handler you copied avoids it only because its break happens to fall after a space.

Merging. This closes #1697.

[Shivang9983]

Thanks a lot @RaghavChamadiya for the help, review, and merging this!

Also noted the point about the missing space in the string split—I will keep that in mind next time. Glad I could help with this fix, and I look forward to contributing more to Repowise!

[RaghavChamadiya]

@Shivang9983 following up on this, as promised. #1977 is open with the one-character fix.

You reported that the two adjacent literals join without a separator so the warning reads Dynamic edgeswill be missing from this update., and that was still true on main this morning. The trailing space goes on the first literal.

Thanks for flagging it and for waiting. You caught something in a message that only prints on a failure path, which is exactly where nobody looks.

[Shivang9983]

Thanks a lot for the update! @RaghavChamadiya
I really appreciate you taking the time to look into this and get the fix opened. Glad I could help catch this small issue. I’ll definitely keep an eye out for more things like this. Thanks again!