Skip to content

Remove full NetsukeManifest AST debug logs (#657) - #661

Draft
leynos wants to merge 2 commits into
mainfrom
issue-657-remove-full-netsukemanifest-ast-from-graph-generation-debug-logs
Draft

Remove full NetsukeManifest AST debug logs (#657)#661
leynos wants to merge 2 commits into
mainfrom
issue-657-remove-full-netsukemanifest-ast-from-graph-generation-debug-logs

Conversation

@leynos

@leynos leynos commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Summary

This branch removes complete manifest AST logging from graph generation and
excludes manifest-derived target and dependency names from its trace events.
The existing bounded graph-generation metrics, duration histogram, trace span
and fixed error categories remain unchanged.

Closes #657.

Review walkthrough

  • Start with graph_generation.rs to confirm that manifest loading flows directly into the existing bounded telemetry.
  • Then review from_manifest.rs and cycle_detector.rs for the removal of manifest-derived tracing fields.
  • Finish with tests.rs, which captures DEBUG events and rejects a supplied manifest sentinel.
  • keys.rs and every locale catalogue remove the now-unreachable serialisation error key.

Validation

  • make check-fmt: passed.
  • make lint: passed.
  • make doc-coverage: passed (99.13%).
  • make test: passed (2,774 nextest cases and doctests).
  • coderabbit review --agent: two clean review rounds with zero findings.

References

Summary by Sourcery

Remove sensitive manifest details from graph-generation logs while preserving existing bounded metrics and tracing.

Bug Fixes:

  • Prevent manifest contents and manifest-derived target or dependency names from being emitted in graph-generation tracing events.

Enhancements:

  • Retain bounded graph-generation telemetry while removing obsolete manifest serialization and unresolved-dependency logging paths.

Tests:

  • Add coverage verifying that manifest sentinel content does not appear in DEBUG events.

Chores:

  • Remove the unused manifest serialization localization key from all locale catalogues.

Stop serialising the complete user-supplied manifest in the runner and
remove its no-longer-reachable localisation context.

Keep graph-generation telemetry limited to its existing bounded labels,
metrics, and tracing fields.
Keep graph-generation diagnostics to aggregate counts and fixed categories
instead of target and dependency names supplied by a manifest.

Add a DEBUG-level regression that rejects a manifest sentinel in all
captured graph-generation events.
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Graph generation no longer serializes or logs the full manifest AST, and IR tracing no longer exposes manifest-derived target or dependency names. Existing bounded telemetry and error handling remain intact, while a DEBUG-event regression test and removal of the obsolete serialization localization key/catalogue entries enforce the privacy-focused behavior.

Sequence diagram for privacy-preserving graph generation telemetry

sequenceDiagram
    participant Runner
    participant ManifestLoader
    participant GraphTelemetry
    participant BuildGraph
    participant DebugCapture

    Runner->>ManifestLoader: load_manifest_with_stage_reporting()
    ManifestLoader-->>Runner: manifest
    Runner->>GraphTelemetry: instrument_graph_generation()
    GraphTelemetry->>BuildGraph: generate graph from manifest
    BuildGraph-->>GraphTelemetry: bounded graph metrics
    GraphTelemetry-->>Runner: graph result
    DebugCapture-->>Runner: capture DEBUG events
    Note over Runner,DebugCapture: Full AST serialization and manifest-derived names are not emitted
Loading

File-Level Changes

Change Details Files
Remove complete manifest AST serialization from graph-generation debug logging while preserving the existing generation pipeline and bounded telemetry.
  • Delete the DEBUG-gated manifest-to-JSON serialization and its localization error path.
  • Keep manifest loading, stage reporting, graph-generation metrics, duration histogram, tracing span, and fixed error categories unchanged.
src/runner/graph_generation.rs
src/localization/keys.rs
locales/ar/messages.ftl
locales/cs/messages.ftl
locales/cy/messages.ftl
locales/da/messages.ftl
locales/de/messages.ftl
locales/el/messages.ftl
locales/en-GB/messages.ftl
locales/en-US/messages.ftl
locales/es-419/messages.ftl
locales/es-ES/messages.ftl
locales/fa/messages.ftl
locales/fi/messages.ftl
locales/fr/messages.ftl
locales/gd/messages.ftl
locales/he/messages.ftl
locales/hi/messages.ftl
locales/hu/messages.ftl
locales/id/messages.ftl
locales/it/messages.ftl
locales/ja/messages.ftl
locales/ko/messages.ftl
locales/nb/messages.ftl
locales/nl/messages.ftl
locales/pl/messages.ftl
locales/pt-BR/messages.ftl
locales/pt-PT/messages.ftl
locales/ro/messages.ftl
locales/ru/messages.ftl
locales/sv/messages.ftl
locales/th/messages.ftl
locales/tr/messages.ftl
locales/uk/messages.ftl
locales/vi/messages.ftl
locales/zh-Hans/messages.ftl
locales/zh-Hant/messages.ftl
Remove manifest-derived target and dependency names from IR tracing.
  • Stop including target names in implicit-dependency debug events while retaining dependency counts.
  • Stop logging missing dependency and unresolved dependency paths; continue counting and treating unresolved dependencies as external files.
  • Update cycle-detection and graph-lowering documentation to reflect the telemetry behavior.
src/ir/from_manifest.rs
src/ir/cycle_detector.rs
Add regression coverage that verifies manifest content is absent from DEBUG events.
  • Capture DEBUG-level events during graph generation with a test subscriber.
  • Assert that a sentinel appearing in target names, dependencies, and commands is never emitted.
src/runner/tests.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#657 Remove the complete NetsukeManifest AST DEBUG log and its serialization from the graph-generation path.
#657 Ensure graph-generation and related tracing do not expose unbounded user-supplied manifest content, including target names, dependency names, variables, or macro bodies.
#657 Preserve bounded graph-generation observability, including counters, duration metrics, tracing, and fixed error categories, while removing obsolete serialization diagnostics and adding regression coverage.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No quality gates enabled for this code.

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.

Remove full NetsukeManifest AST from graph-generation DEBUG logs

1 participant