Skip to content

release(v0.5.0): Phase 11 + 11.5 + 12 + 12.5 consolidated release - #18

Merged
cemililik merged 12 commits into
mainfrom
development
Apr 29, 2026
Merged

cemililik merged 12 commits into
mainfrom
development

Conversation

@cemililik

@cemililik cemililik commented Apr 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • v0.5.0 release — consolidates Phase 11 (data ingestion), Phase 11.5 (PII review fixes), Phase 12 (data curation maturity), and Phase 12.5 (data-curation polish) into a single comprehensive release.
  • Phase 12.5 polish — four data-curation improvements: silent-failure guards, Presidio canonicalization, Croissant card cleanup, and review-feedback application.
  • Site + user manuals — documentation refresh to reflect new data curation features (PII ML, all-mask, Croissant cards, ingestion improvements) in both EN and TR mirrors.

Changes at a glance

  • forgelm/cli.py, forgelm/data_audit.py, forgelm/wizard.py — Phase 12.5 feature work + silent-failure guards
  • tests/test_phase12_5.py (+528 lines) — comprehensive coverage for new behavior
  • docs/usermanuals/{en,tr}/data/ — new bilingual user manuals for PII ML, all-mask, Croissant cards
  • docs/roadmap/, CHANGELOG.md, README.md — release docs synchronized with merged state
  • site/ — landing pages reflect v0.5.0 messaging
  • pyproject.toml — version bump + extras adjustments

Test plan

  • ruff format . && ruff check . clean
  • pytest tests/ green (including new test_phase12_5.py)
  • forgelm --config config_template.yaml --dry-run succeeds
  • CI passes on the PR
  • Bilingual doc pairs (EN/TR) verified structurally aligned
  • Manual smoke: wizard BYOD flow + data audit pipeline

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added --all-mask (combined secrets+PII masking), --pii-ml (optional ML-based PII detection), --croissant (optional Croissant 1.0 dataset card), and a wizard “audit first” prompt after ingestion.
  • Documentation
    • Consolidated phase notes into a single 0.5.0 release and added user guidance for the new flags, CLI examples, and wizard flow.
  • Tests
    • Added comprehensive Phase 12.5 test coverage validating masking, audit wizard, Croissant output, and ML-PII behavior.

cemililik and others added 4 commits April 29, 2026 03:16
Phase 12.5 (Data Curation Polish) ships the four follow-up items
deferred from Phase 12 — none require new architecture; each is a
small additive surface on top of the Phase 12 ingestion + audit
lineage. Will release with v0.5.3 alongside Phase 14.

Source of truth: docs/roadmap/phase-12-5-backlog.md (now updated to
"landed on development" for all four rows).

Item 3 — `forgelm ingest --all-mask` (XS, no new behaviour)
- One-flag shorthand for `--secrets-mask --pii-mask` in the
  documented mask order. Resolved at the CLI boundary; ingest_path's
  Python API stays untouched. Composes additively with explicit
  flags (set-union, no error).

Item 4 — Wizard "audit first" entry point (S)
- New _offer_audit_for_jsonl(path) helper in forgelm/wizard.py,
  invoked from the three JSONL-finalisation paths:
  _offer_ingest_for_directory (after ingest produces a JSONL),
  _validate_local_jsonl (BYOD path), and
  _prompt_dataset_path_with_ingest_offer (full wizard direct-JSONL).
  Mirrors the Phase 11.5 _offer_ingest_for_directory shape exactly.
  Audit is informational, not a gate — failures fall through to the
  "continue without audit" path so a malformed JSONL or transient
  audit error never crashes the wizard.

Item 2 — `forgelm audit --croissant` (S)
- Opt-in Google Croissant 1.0 dataset card emitted under a new
  ``croissant`` field on AuditReport. Card is conformant with
  http://mlcommons.org/croissant/1.0 — `@type: sc:Dataset`, full
  `@context` block, one `cr:FileObject` per JSONL split, one
  `cr:RecordSet` per split with `cr:Field` entries derived from the
  audit's column-detection layer.
- Existing audit JSON keys are byte-equivalent — block stays empty
  when the flag is off (precedent: secrets_summary / quality_summary).

Item 1 — Presidio PII ML adapter (S–M)
- New `[ingestion-pii-ml]` extra (`presidio-analyzer>=2.2.0,<3.0.0`).
- New `forgelm audit --pii-ml` flag layers Presidio NER on top of
  the regex detector. Adds the unstructured-identifier categories
  the regex inherently misses: `person`, `organization`, `location`.
- New `PII_ML_SEVERITY` table (`person → medium`, `organization →
  low`, `location → low`) merged into _build_pii_severity's
  severity_table snapshot so the new categories grade through the
  existing pii_severity block.
- Pre-flight `_require_presidio()` raises ImportError with an
  install hint when the extra is missing — failures surface before
  any rows are scanned. Per-row Presidio failures swallowed so a
  single malformed row never blocks the audit.

Touch points
- forgelm/cli.py — three new flags on existing subparsers; dispatcher
  signatures threaded.
- forgelm/data_audit.py — Presidio import block + sentinel; new
  PII_ML_SEVERITY / PII_ML_TYPES / _PRESIDIO_ENTITY_MAP constants;
  _require_presidio / _get_presidio_analyzer / detect_pii_ml; new
  croissant field on AuditReport; _build_croissant_metadata helper +
  _CROISSANT_CONTEXT; enable_pii_ml / emit_croissant params on
  audit_dataset / _process_split / _audit_split; enable_pii_ml field
  on _StreamingAggregator; _build_pii_severity now consults the
  merged regex+ML table.
- forgelm/wizard.py — _offer_audit_for_jsonl + three call-site
  injections.
- pyproject.toml — `[ingestion-pii-ml]` extra.
- tests/test_phase12_5.py — 11 new tests, four classes (one per item).
- tests/test_wizard_byod.py — three existing tests get an extra "n"
  to decline the new audit-first offer (offer behaviour has its own
  coverage in test_phase12_5.py).
- README, CHANGELOG, roadmap{,-tr}, phase-12-5-backlog.md,
  docs/standards/architecture.md, docs/guides/{ingestion,data_audit}{,-tr}.md,
  docs/reference/usage{,-tr}.md, notebooks/data_curation.ipynb.

Tests: 833 pass (was 822, +11 new). Ruff clean. Bilingual H2 parity
7/7. CLI smoke + new-flag help text verified.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dio canonicals, Croissant cleanup

Reviewer-driven follow-up to commit cad587d. Addresses every MAJOR
finding plus the actionable MINOR / doc-drift items. 7 new tests; 840
total pass (was 833).

MAJOR

- M1 — _require_presidio now pre-flights the analyzer build and
  re-raises spaCy's "Can't find model" OSError as ImportError with
  the two-line install recipe, so a missing en_core_web_lg surfaces
  before any rows are scanned. Previously the OSError was swallowed
  by detect_pii_ml's per-row bare-Exception, leaving the audit green
  with zero ML PII coverage and no diagnostic — direct violation of
  CLAUDE.md principle #6 (no silent failures). detect_pii_ml's catch
  is now narrowed to (ValueError, RuntimeError) so a model-load
  failure that slips through (e.g. lazy load triggered later) cannot
  be silenced; the install-hint string is consolidated into one
  module-level constant so the two failure surfaces stay in sync.
- M2 — _PRESIDIO_ENTITY_MAP trimmed from 7 keys to the 3 Presidio
  actually emits canonically (PERSON / ORGANIZATION / LOCATION).
  ORG / GPE / NORP / FAC were dead — Presidio's
  model_to_presidio_entity_mapping translates raw spaCy labels to
  canonical Presidio names before they reach AnalyzerResult, so
  raw-label keys never matched. NORP → location was also semantically
  wrong (nationality / religion / political group is a distinct
  privacy signal). The new comment explains the canonicalisation
  layer so a future maintainer doesn't reintroduce raw spaCy keys.
- M3 — Croissant version field dropped. Croissant's `version` is
  the dataset version (sc:version), not the vocabulary version;
  vocab conformance is declared via `conformsTo` (already correct).
  The audit doesn't have first-class evidence for dataset version,
  so the field is omitted entirely; operators who publish the card
  hand-edit `version` like they do `license` / `citeAs`. The
  misnamed _CROISSANT_VOCAB_VERSION constant is removed.
- M4 — `forgelm audit --output-format json` envelope now includes
  the `croissant` key (empty when --croissant is off, populated
  when on) so a CI consumer reading stdout doesn't have to re-slurp
  the on-disk file. Same precedent as secrets_summary /
  quality_summary.

MINOR

- m2 — New `--pii-ml-language LANG` flag (default "en") plumbed
  through audit_dataset → _process_split → _audit_split →
  _StreamingAggregator → detect_pii_ml. Lets a Turkish-majority
  corpus run with the matching spaCy model instead of silently
  scoring zero NER findings under English. Presidio raises a typed
  exception when no engine is registered for the requested language.
- m3 — Croissant card's `url` field now carries `source_input`
  (the as-typed string) instead of the resolved absolute filesystem
  path, and per-split `contentUrl` is the relative file_id. Cards
  published to HuggingFace / MLCommons no longer leak the auditor's
  /Users/... or /home/builder/... layout.
- m5 — Wizard's audit-offer copy now hedges against multi-million-
  row JSONL: under a 100 MB threshold it keeps the friendly "~
  seconds" tone, above it surfaces the file size and an "8-15 MB/s
  of JSONL on a single CPU" expectation so the operator can decide
  before saying yes.
- m7 — _offer_audit_for_jsonl docstring now explicitly cites
  docs/standards/error-handling.md for the bare-Exception ladder,
  making the durability of the swallow durable when the file is
  re-read in 6 months.

TESTS (+7, 840 total)

- test_croissant_section_empty_by_default now pins "key present,
  value empty" instead of the weaker "absent or empty" form.
- New: test_presidio_entity_map_only_canonical_keys — schema pin
  on the trimmed map.
- New: test_presidio_missing_spacy_model_surfaces_install_hint —
  exercises the M1 pre-flight by patching _PresidioAnalyzer to
  raise OSError; asserts ImportError carries the install recipe.
- New: test_presidio_entity_map_collapses_findings_to_buckets —
  behavioural test that would have caught the original M2 dead-key
  bug. Stubs the analyzer to emit canonical + raw spaCy labels
  and asserts only canonicals are honoured.
- New: TestCroissantMultiSplit — multi-split corpus produces one
  cr:FileObject + cr:RecordSet per split with relative contentUrl
  (anti-regression for m3).
- New: TestAllMaskSymmetric — symmetric set-union case
  (`--secrets-mask --all-mask`) so a future refactor that drops one
  branch trips a test.
- New: TestPiiMlJsonEnvelope — confirms M4: croissant present in
  stdout envelope when on, empty when off.

DOC DRIFT (5 places)

- README.md install matrix and Phase 12.5 feature line: spell out
  the spaCy model download as a separate step.
- CHANGELOG.md Phase 12.5 entry: recipe-first install instructions
  for --pii-ml, explicit OSError → ImportError contract,
  --pii-ml-language flag mention, Croissant url/contentUrl path
  leak fix, version field omission rationale.
- docs/guides/data_audit.md and docs/guides/data_audit-tr.md:
  replaced "Presidio's first analyzer build downloads a spaCy
  model (~ 50 MB)" — wrong on every supported Presidio version —
  with a two-step install block + non-English language hint.
- notebooks/data_curation.ipynb "What's next" cell: same fix.

Verification: ruff clean (78 files), 840 tests pass (13 skipped),
7/7 bilingual H2 parity, --pii-ml / --pii-ml-language / --croissant
all surface in `forgelm audit --help`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…prehensive v0.5.0

Originally planned as four sequential PyPI tags (v0.5.0 / v0.5.1 /
v0.5.2 / v0.5.3). The four phases form one coherent surface (ingest →
polish → mature → polish) that's hard to use in parts; consolidating
into a single comprehensive v0.5.0 release keeps the user-facing
release notes focused while git history retains the four phases as
separate commit batches.

pyproject.toml
- version: 0.5.2rc1 → 0.5.0

CHANGELOG.md
- Collapse [Unreleased] + [0.5.2] + [0.5.1] + [0.5.0] (summary stub)
  into a single [0.5.0] — 2026-04-29 entry.
- Theme paragraph + consolidation note explaining the merger.
- Internal phase boundaries preserved as section markers
  (### Phase 12.5 / Phase 12 / Phase 11.5 / Phase 11) so reviewers can
  map back to PR history (#11, #12, #13, current).
- [Unreleased] is now empty (next minor picks up Phase 14).

README.md
- "Document Ingestion & Data Audit (v0.5.0 — polished in v0.5.1,
  matured in v0.5.2)" → "(v0.5.0 — Phases 11 + 11.5 + 12 + 12.5
  consolidated)".
- Drop "v0.5.2 added X" / "v0.5.1 added Y" / "Phase 12.5 polish
  (v0.5.3)" history phrasing — features now read as part of v0.5.0.

docs/roadmap.md + roadmap-tr.md
- Status table: four separate "🟡 Merged" rows for Phases 11 / 11.5 /
  12 / 12.5 collapsed into one combined row pointing at
  releases.md#v050. Phase 14 row reslotted from v0.5.3 to v0.5.1.
- "Latest PyPI: v0.4.5" wording trimmed (the v0.5.0/.1/.2 sentence
  about three pending tags is gone).
- "Next: v0.5.3" → "Next: v0.5.1" (Phase 14).
- Phase count: 16 complete + 3 planned → 17 complete + 2 planned
  (12.5 moves into the consolidated release; 13 + 14 remain planned).
- Mermaid diagram: P11 / P115 / P12 / P125 all point at the same
  V2[v0.5.0 pending] node; styling bumped to merged-but-not-tagged
  light-green.

docs/roadmap/releases.md
- v0.5.0 / v0.5.1 / v0.5.2 sections collapsed into a single comprehensive
  v0.5.0 entry organised by phase (### Phase 11 / 11.5 / 12 / 12.5).
- v0.5.3 (Phase 14) → v0.5.1 (Phase 14).

docs/roadmap/phase-{11-5,12-5,12-data-curation,14-pipeline}.md
- Consolidation note at the top of each historical phase plan
  ("originally targeted vX.Y.Z, ships as part of consolidated v0.5.0;
  see releases.md#v050"). Body kept intact as the historical planning
  trail.
- phase-14: Target release v0.5.3 → v0.5.1; "starts after v0.5.2 PyPI
  tag" → "starts after v0.5.0 PyPI tag".
- phase-12-5-backlog rows: "ships with v0.5.3" → "ships with v0.5.0".

docs/guides/{ingestion,data_audit}{,-tr}.md +
docs/reference/usage{,-tr}.md
- Drop the "v0.5.2 added" / "v0.5.1 promoted" history phrasing inside
  feature descriptions. Features now read as v0.5.0 capabilities;
  phase labels (Phase 11 / 11.5 / 12 / 12.5) are kept as breadcrumbs
  for reviewers.
- Section headings like "Document Ingestion (v0.5.0+; token-aware in
  v0.5.1; markdown + secrets-mask in v0.5.2)" → "(v0.5.0)".
- Inline comments in CLI examples drop "v0.5.2:" / "v0.5.3 (Phase 12.5):"
  prefixes since the features all ship in the same release.

notebooks/data_curation.ipynb
- Install pin: 0.5.2 → 0.5.0 (cells 1 + 16).
- "Phase 12.5 audit add-ons (v0.5.3)" markdown → "Other v0.5.0 audit
  add-ons".
- Section 2 header drops the "Phase 12.5 shorthand (v0.5.3)" framing
  for --all-mask; the flag is just part of the v0.5.0 release.

forgelm/cli.py
- One internal comment tweaked from "v0.5.1 consumers" to
  "any pre-Phase-12 JSON consumer" (the legacy envelope key still has
  a real reason to exist; the framing just stops referencing a
  release that won't exist as a separate PyPI tag).
- Phase labels in user-visible help text (Phase 11.5 / 12 / 12.5) kept
  intact — they're descriptive history, not version pins.

Verification
- ruff clean (78 files)
- 840 tests pass (13 skipped)
- 7/7 bilingual H2 parity (configuration / usage / distributed /
  data_preparation / architecture / ingestion / data_audit)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Site: hero badge v0.5.2 → v0.5.0 across 6 locales + index fallback;
  quickstart shows --all-mask shorthand; compliance Article 10 card
  mentions Presidio ML-NER + Croissant 1.0 in all 6 translations.
- _meta.yaml: register all-mask, pii-ml, croissant-card under data/
  with titles in all 6 supported locales.
- New EN + TR pages (3 each): all-mask shorthand semantics, Presidio
  ML-NER opt-in adapter (two-step install + severity tiers + pre-flight
  rationale), Croissant 1.0 dataset card emission.
- Cross-link sweep: ingestion + audit + pii-masking pages updated to
  reference the three new pages on both EN and TR sides.

Localization: only EN + TR carry authored content; de/fr/es/zh fall
back to EN via the build script (56 pages each, build clean).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@sourcery-ai sourcery-ai 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.

Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters

@coderabbitai

coderabbitai Bot commented Apr 29, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@cemililik has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 38 minutes and 57 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 049c566e-b57b-4899-9310-2d8805852117

📥 Commits

Reviewing files that changed from the base of the PR and between 502ff88 and a4b2bb2.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • docs/usermanuals/en/data/pii-ml.md
  • forgelm/data_audit.py
📝 Walkthrough

Walkthrough

Consolidates Phases 11–12.5 into a single v0.5.0 release and adds Phase 12.5 features: --all-mask (combined secrets+PII masking), optional --pii-ml (Presidio ML‑NER), --croissant dataset‑card emission, a --quality-filter hook, and a wizard “audit first” interactive post‑ingest flow.

Changes

Cohort / File(s) Summary
Release & Roadmap
CHANGELOG.md, docs/roadmap.md, docs/roadmap-tr.md, docs/roadmap/releases.md, docs/roadmap/*
Consolidates Phases 11/11.5/12/12.5 under v0.5.0, removes duplicated 0.5.x entries, updates roadmap tables, Mermaid graphs, and release-note structure.
Guides & Reference
README.md, docs/guides/..., docs/reference/usage.md, docs/reference/usage-tr.md, docs/guides/*.md
Reframes version text to v0.5.0; documents --all-mask, --pii-ml (+language/model preflight), --croissant, and --quality-filter; updates examples and CLI refs.
User Manuals & i18n
docs/usermanuals/_meta.yaml, docs/usermanuals/en/data/*, docs/usermanuals/tr/data/*
Adds pages for Combined Masking, ML‑PII, and Croissant card; updates ingestion/audit manuals, cross‑refs, localized nav entries and examples.
Architecture & Notebook
docs/standards/architecture.md, notebooks/data_curation.ipynb
Adds ingestion-pii-ml optional extra to architecture; updates notebook to v0.5.0 and documents --all-mask, Croissant, and Presidio+spaCy install snippet.
CLI
forgelm/cli.py
Adds --all-mask to ingest; adds --croissant, --pii-ml, and --pii-ml-language to audit; threads flags into audit invocation and surfaces report.croissant in JSON output.
Audit Core
forgelm/data_audit.py
Implements Presidio ML‑NER path (detect_pii_ml, severity mapping, _require_presidio preflight), Croissant emitter (_build_croissant_metadata / report.croissant), quality_summary wiring, and merges ML findings with regex PII counts.
Wizard
forgelm/wizard.py
Adds post‑ingest “audit first” hook prompting to run audit on produced JSONL (size warning >100MB), lazy imports, error recovery, and BYOD prompt adjustments to avoid double prompts.
Project Metadata
pyproject.toml
Bumps project version to 0.5.0 and adds optional extra ingestion-pii-ml = ["presidio-analyzer>=2.2.0,<3.0.0"].
Website & Quickstart
site/index.html, site/compliance.html, site/quickstart.html, site/js/translations.js
Updates public version label to v0.5.0, expands compliance text with optional Presidio/Croissant mentions, updates quickstart to use --all-mask and document new audit flags (i18n strings updated).
Tests
tests/test_phase12_5.py, tests/test_wizard_byod.py
Adds comprehensive Phase 12.5 tests covering --all-mask semantics, wizard audit hook behavior and error handling, Croissant emission mapping and path handling, and Presidio ML preflight/failure semantics; updates BYOD tests to decline audit prompt.

Sequence Diagram

sequenceDiagram
    actor User
    participant Wizard as "Wizard (CLI)"
    participant Ingest as "Ingestion Module"
    participant Audit as "Audit Module"
    participant Report as "AuditReport / Croissant"

    User->>Wizard: run `forgelm ingest --wizard ...`
    Wizard->>Ingest: ingest_path(...) -> produce JSONL
    Ingest-->>Wizard: return JSONL path

    Wizard->>Wizard: check JSONL size (warn if >100MB)
    Wizard->>User: "Audit this dataset first? (y/n)"

    alt User accepts
        User-->>Wizard: y
        Wizard->>Audit: audit_dataset(jsonl_path, enable_pii_ml?, emit_croissant?, quality_filter?)
        Audit->>Audit: preflight (presidio/spaCy checks), dedup, PII regex + ML
        Audit-->>Report: AuditReport (pii_summary, quality_summary, croissant)
        Wizard->>User: summarize_report(report) -> display verdict
    else User declines or audit error
        User-->>Wizard: n / error
        Wizard->>User: continue without audit
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 A tidy hop, a consolidation cheer,

Four phases tucked into one release this year.
Croissant cards and Presidio eyes,
The wizard asks first, then shows the prize.
--all-mask hops in — short, sweet, and clear.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly summarizes the main change: consolidating four phases into a v0.5.0 release.
Description check ✅ Passed The PR description is well-structured with summary, changes overview, and test plan, covering the main objectives of consolidating phases and Phase 12.5 features.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch development

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 38 minutes and 57 seconds.

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

@codacy-production

codacy-production Bot commented Apr 29, 2026 •

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 42 medium

Alerts:
⚠ 42 issues (≤ 0 issues of at least minor severity)

Results:
42 new issues

Category Results
ErrorProne 42 medium

View in Codacy

🟢 Metrics 101 complexity · 15 duplication

Metric Results
Complexity 101
Duplication 15

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request consolidates Phases 11, 11.5, 12, and 12.5 into a single v0.5.0 release, introducing several data curation and governance features including a Presidio-based ML-NER PII detector, Google Croissant 1.0 dataset card emission, an '--all-mask' ingestion shorthand, and a wizard-integrated audit entry point. My feedback focuses on clarifying documentation phrasing, ensuring explicit links for environment setup in air-gapped scenarios, and improving error logging for the new audit hooks.

Comment thread CHANGELOG.md Outdated
Comment on lines +7 to +8
(no unreleased changes — next minor will pick up Phase 14, tracked in
[docs/roadmap.md](docs/roadmap.md))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The phrasing '(no unreleased changes — next minor will pick up Phase 14...)' is slightly ambiguous. It is better to state clearly that the release is finalized and future work is tracked elsewhere.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✅ Addressed in commit 7fae240 — reworded to make the Unreleased section state explicit ("Empty — v0.5.0 is finalized. The next minor (v0.5.1) covers Phase 14...").

Comment thread docs/guides/data_audit.md
Comment on lines +371 to +372
pip install 'forgelm[ingestion-pii-ml]'
python -m spacy download en_core_web_lg # ~ 50 MB; required

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The instruction to download the spaCy model is critical. Ensure that the installation instructions for the spaCy model are explicitly linked to the environment setup section to avoid runtime errors in air-gapped or restricted environments.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Status: declined — already inline.

The fail-loud install-recipe behaviour is documented immediately below the install block (this same section, lines 375-381):

Without the model, forgelm audit --pii-ml raises an ImportError with the same install hint before any rows are scanned (the pre-flight check in forgelm.data_audit._require_presidio catches spaCy's OSError and re-raises it as the typed install error). This fail-loud behaviour exists because earlier prototypes silently scored zero ML findings on a missing model — a critical compliance blind spot for an opt-in detector that operators expect to actually run.

Operators in air-gapped environments hit this fail-loud path before any rows are scanned, with the same install recipe. The pre-flight + the install hint live in _require_presidio at the top of the module so the recipe is in one canonical place. No separate "environment setup" section exists to link to — the docs are intentionally flat for this guide.

Comment thread forgelm/wizard.py
Comment on lines +547 to +556
except Exception as exc: # noqa: BLE001 — see docstring
# Last-ditch guard: a malformed JSONL row mid-file, a datasketch
# version-mismatch surfaced through datasketch's own internals, or
# an unanticipated edge case in a downstream audit module must NOT
# crash the wizard. The audit is informational; we log loudly and
# let the operator continue. ``KeyboardInterrupt`` /
# ``SystemExit`` propagate by virtue of being ``BaseException``
# subclasses, so Ctrl-C still aborts the wizard cleanly.
print(f" Audit could not run: {exc}")
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

While catching Exception is necessary to prevent wizard crashes, ensure that specific expected errors (like FileNotFoundError) are logged with enough context for the user to troubleshoot the audit failure.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Status: declined — current behaviour already surfaces the exception type and message.

The wizard renders interactive output via print() (project convention for the Step-N flow), and print(f" Audit could not run: {exc}") already includes the exception type and message in the rendered string — e.g. Audit could not run: [Errno 2] No such file or directory: '/path/to/foo.jsonl' for FileNotFoundError.

Adding except FileNotFoundError as exc: would emit the same string with no additional context. The catch-all except Exception is intentional (with the inline docstring rationale at lines 547-554) so a malformed JSONL row, a datasketch version mismatch, or any other downstream audit-module failure doesn't crash the wizard mid-flow.

KeyboardInterrupt and SystemExit propagate by virtue of being BaseException subclasses, so Ctrl-C still aborts cleanly. Leaving as-is.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Around line 96-99: Update the fenced code block that contains the two commands
("pip install 'forgelm[ingestion-pii-ml]'" and "python -m spacy download
en_core_web_lg'") by adding a language tag (e.g., ```bash) immediately after the
opening backticks so the block becomes a bash code block and satisfies
markdownlint MD040.

In `@docs/guides/data_audit-tr.md`:
- Around line 415-416: Update the Croissant field list in the docs snippet that
currently reads "dataset seviyesinde kimlik (`name`, `description`, `version`,
`datePublished`, `url`)" by removing the `version` token so it matches the
generator output; edit the string in docs/guides/data_audit-tr.md where the
dataset identity list is defined (the line containing the backticked list with
name, description, version, datePublished, url) and remove `version` from the
comma-separated backticked items, keeping punctuation and localization style
consistent with surrounding sentences.

In `@docs/guides/data_audit.md`:
- Around line 409-423: The docs list includes a `version` Croissant field that
the code does not emit; update the documentation to remove `version` from the
documented Croissant field list (or explicitly note it's omitted) to match the
implementation in forgelm/data_audit.py::_build_croissant_metadata(); ensure the
docs reflect that dataset-level identity contains name, description,
datePublished, and url only (or state that `version` will appear only if
_build_croissant_metadata() is changed to populate it).
- Around line 322-425: The CLI synopsis block that begins with "forgelm audit
PATH \" is missing the new flags; update that copy-pasteable CLI example to
include --pii-ml, --pii-ml-language <lang>, and --croissant (e.g. append them to
the options list or add a separate example showing their usage) so the
documented invocation matches the newly described features (pii-ml,
pii-ml-language, croissant) in the preceding sections.

In `@docs/guides/ingestion.md`:
- Around line 143-159: The CLI reference block for the "forgelm ingest" command
is missing the new shorthand flag --all-mask; update the ingest
synopsis/reference block that currently lists --pii-mask and --secrets-mask to
also include --all-mask so the guide is consistent with the new "All-in-one
masking" section and clarifies that --all-mask composes with --pii-mask and
--secrets-mask.

In `@docs/roadmap-tr.md`:
- Around line 12-31: The roadmap text is internally inconsistent: the top claims
Faz 11 + 11.5 + 12 + 12.5 were consolidated into a single release `v0.5.0`, but
later map entries still reference separate tags `v0.5.1` / `v0.5.2` / `v0.5.3`;
update the later roadmap map and the "Sonraki" section so all references reflect
the consolidation (replace mentions of `v0.5.1`/`v0.5.2`/`v0.5.3` with `v0.5.0`
where appropriate, adjust the entries for Faz 14 and Faz 13 to show they follow
`v0.5.0`, and ensure the "PyPI'deki son sürüm" / "Başlangıçta dört ardışık PyPI
tag'i" wording matches the single `v0.5.0` consolidation described near `Faz
11`/`Faz 11.5`/`Faz 12`/`Faz 12.5`).

In `@docs/roadmap.md`:
- Around line 12-31: The roadmap text contradicts itself by saying Phases
11/11.5/12/12.5 were consolidated into v0.5.0 while later still mapping them to
v0.5.1/v0.5.2/v0.5.3; update the document map so all references to Phase 11,
11.5, 12, and 12.5 point to v0.5.0 (remove the old split v0.5.1/v0.5.2/v0.5.3
mapping), keep Phase 14 listed as v0.5.1 and Phase 13 as v0.6.0-pro, and ensure
the table rows/summary blocks (the lines mentioning "Phase 11 + 11.5 + 12 + 12.5
— Document Ingestion & Data Curation Pipeline", "v0.5.0", "Phase 14 —
Multi-Stage Pipeline Chains", and "Phase 13 — Pro CLI & Observability
Dashboard") are consistent so the page no longer contradicts itself.

In `@docs/usermanuals/en/data/croissant-card.md`:
- Around line 130-132: Replace the broken fragment links '#/data/audit',
'#/compliance/annex-iv', and '#/compliance/gdpr' in croissant-card.md with the
project's resolvable internal documentation links (use the docs tree's canonical
relative/absolute doc URLs used elsewhere), e.g., update the three list items
for "Dataset Audit", "Annex IV", and "GDPR / KVKK" to point to their actual
markdown targets per docs/standards (use the same link style as other docs files
so they pass MD051 validation).

In `@docs/usermanuals/en/data/pii-ml.md`:
- Around line 79-87: The docs are wrong about Presidio errors being "surfaced
loudly" because detect_pii_ml() currently catches ValueError/RuntimeError
(around the handler at line ~518) and returns {} per row; update the code to
perform a single pre-flight validation of the requested language instead of
swallowing per-row errors: add a language-model check in _require_presidio() (or
at the start of detect_pii_ml()) that verifies the requested --pii-ml-language
is registered/available and if not either raise a clear exception or log an
error and abort the audit before scanning rows; ensure the same unique symbols
detect_pii_ml() and _require_presidio() are modified so failures for custom
languages are not silently converted to empty findings.

In `@docs/usermanuals/tr/data/audit.md`:
- Line 103: Satırdaki `--croissant` açıklamasında kullanılan dış bağlantı
`http://mlcommons.org/croissant/` güvenli değil; düzeltmek için metindeki URL'yi
`https://mlcommons.org/croissant/` olarak güncelleyin (aynı metin içinde
Croissant 1.0 referansını içeren `--croissant` açıklamasını bulun ve yalnızca
protokolü http → https olarak değiştirin).

In `@docs/usermanuals/tr/data/pii-ml.md`:
- Around line 79-86: The docs claim language-registration failures are raised,
but detect_pii_ml() currently swallows ValueError/RuntimeError per row; change
detect_pii_ml so it fails loudly by doing a pre-flight language registration
check (validate the requested PII-ML engine/language once at the start) and
raise an explicit exception if the engine or spaCy model is not available
instead of catching and returning {} per-row; alternatively, if you prefer
per-row handling, update the catch to re-raise registration-related exceptions
so callers observe the failure (refer to detect_pii_ml and its current
ValueError/RuntimeError except block).

In `@forgelm/data_audit.py`:
- Around line 2473-2490: The code builds file_id from the split label (file_id =
f"{split_name}.jsonl") and sets contentUrl/url from source_input, which causes
wrong filenames for single-file/alias audits and leaks absolute paths; instead
derive the file_id and contentUrl from the real source filename supplied in the
split metadata (e.g. use info.get("source_filename") or info.get("source") /
source_input when present), sanitize it with os.path.basename (or a normalized
published name) to strip local filesystem prefixes, and fall back to the split
label only if no source filename exists; update the distribution.append block
(the file_id, "name", "contentUrl" and any url variable) to use this sanitized
source-derived name so cards reference the real source file names and do not
leak absolute paths.
- Around line 480-526: The current detect_pii_ml() swallows ValueError from
Presidio (causing unsupported --pii-ml-language to silently return {} per row);
change it to call _require_presidio() up front (or validate in audit_dataset())
and then obtain the analyzer via _get_presidio_analyzer() and explicitly verify
the requested language is registered on the AnalyzerEngine (raise ValueError if
not) so misconfiguration fails fast; after that, keep the per-row RuntimeError
resilience but do not catch ValueError from analyzer.analyze() so
unsupported-language errors surface immediately.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b319bd3-f126-45ea-a6a4-519b415f5a3b

📥 Commits

Reviewing files that changed from the base of the PR and between 4f0457f and 7d0f150.

📒 Files selected for processing (40)
  • CHANGELOG.md
  • README.md
  • docs/guides/data_audit-tr.md
  • docs/guides/data_audit.md
  • docs/guides/ingestion-tr.md
  • docs/guides/ingestion.md
  • docs/reference/usage-tr.md
  • docs/reference/usage.md
  • docs/roadmap-tr.md
  • docs/roadmap.md
  • docs/roadmap/phase-11-5-backlog.md
  • docs/roadmap/phase-12-5-backlog.md
  • docs/roadmap/phase-12-data-curation-maturity.md
  • docs/roadmap/phase-14-pipeline-chains.md
  • docs/roadmap/releases.md
  • docs/standards/architecture.md
  • docs/usermanuals/_meta.yaml
  • docs/usermanuals/en/data/all-mask.md
  • docs/usermanuals/en/data/audit.md
  • docs/usermanuals/en/data/croissant-card.md
  • docs/usermanuals/en/data/ingestion.md
  • docs/usermanuals/en/data/pii-masking.md
  • docs/usermanuals/en/data/pii-ml.md
  • docs/usermanuals/tr/data/all-mask.md
  • docs/usermanuals/tr/data/audit.md
  • docs/usermanuals/tr/data/croissant-card.md
  • docs/usermanuals/tr/data/ingestion.md
  • docs/usermanuals/tr/data/pii-masking.md
  • docs/usermanuals/tr/data/pii-ml.md
  • forgelm/cli.py
  • forgelm/data_audit.py
  • forgelm/wizard.py
  • notebooks/data_curation.ipynb
  • pyproject.toml
  • site/compliance.html
  • site/index.html
  • site/js/translations.js
  • site/quickstart.html
  • tests/test_phase12_5.py
  • tests/test_wizard_byod.py

Comment thread CHANGELOG.md Outdated
Comment thread docs/guides/data_audit-tr.md Outdated
Comment thread docs/guides/data_audit.md
Comment thread docs/guides/data_audit.md
Comment thread docs/guides/ingestion.md
Comment thread docs/usermanuals/en/data/pii-ml.md Outdated
Comment thread docs/usermanuals/tr/data/audit.md Outdated
Comment thread docs/usermanuals/tr/data/pii-ml.md Outdated
Comment thread forgelm/data_audit.py
Comment thread forgelm/data_audit.py
cemililik and others added 5 commits April 29, 2026 21:16
Major (CodeRabbit):
- data_audit.py: pre-flight `--pii-ml-language` against the analyzer's
  `supported_languages` so unsupported languages raise ValueError before
  any rows are scanned, instead of being silently swallowed by the
  per-row except handler in detect_pii_ml().
- data_audit.py: derive Croissant `file_id` from the real source filename
  (Path.name) instead of the canonical split label, so single-file audits
  (`policies.jsonl`) and alias layouts (`dev.jsonl` → split `validation`)
  reference the file actually on disk.
- data_audit.py: sanitize the Croissant `url` field to a basename so
  cards published to HuggingFace / MLCommons don't leak the auditor's
  absolute filesystem path (`/Users/...` / `/home/builder/...`).

Docs alignment:
- pii-ml.md (en + tr): rewrite the language-selection + programmatic-API
  sections to describe the actual pre-flight behaviour instead of the
  earlier "errors propagate to the caller" claim.
- data_audit.md / data_audit-tr.md: drop `version` from the Croissant
  field list (the generator omits it by design) and add `--pii-ml`,
  `--pii-ml-language`, `--croissant` to the CLI synopsis blocks.
- ingestion.md / ingestion-tr.md: add `--all-mask` to the CLI synopsis.
- roadmap.md / roadmap-tr.md: align the documentation map with the
  v0.5.0 consolidation narrative (no more orphan v0.5.1/v0.5.2/v0.5.3
  references for already-merged phases).
- audit.md (tr) + croissant-card.md (en + tr) + data_audit.md guides:
  upgrade prose links to mlcommons.org from http to https. Code-example
  references to the JSON-LD `cr:` namespace IRI stay http to match the
  spec-canonical identifier the audit emits.
- CHANGELOG.md: add `bash` language tag to the install-recipe fence
  (markdownlint MD040).

Tests:
- TestCroissantFileIdReflectsRealFilename — single-file + alias layouts
  + url-no-leak regression.
- TestPresidioLanguagePreflight — ValueError for unsupported language,
  default English passes.

Skipped (false positives):
- 42 Codacy MD051 link-fragment warnings on `#/...` hash-router links —
  established convention used in 488 places under docs/usermanuals/.
- 2 SonarCloud HOTSPOT-LOW on `http://mlcommons.org/croissant/` JSON-LD
  identifiers (canonical Croissant 1.0 namespace IRIs, not network
  endpoints) — requires manual "Safe" review in the SonarCloud UI.
- Sourcery: PR diff exceeded their 150KB limit.
- Qodo: free-tier limit reached.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codacy MD051 (42 link-fragment warnings):
- Add docs/usermanuals/.markdownlint-cli2.yaml disabling MD051 only for
  the user-manual SPA tree. The `#/<section>/<page>` pattern is the
  hash-router scheme parsed at runtime by site/js/guide.js::parseHash;
  these are route identifiers consumed after the markdown is baked into
  site/js/usermanuals/<lang>.js by tools/build_usermanuals.py — they
  are NOT in-page anchors and MD051 cannot validate them. The
  convention is part of the public navigation contract (used in 488
  places). MD051 stays enabled everywhere else (CHANGELOG, README,
  docs/standards, docs/guides, docs/roadmap).

SonarCloud python:S5332 (2 LOW security hotspots):
- Annotate the two http:// strings in `_CROISSANT_CONTEXT` and the
  emitted `conformsTo` value with `# NOSONAR` + an explanatory comment.
  These are JSON-LD canonical namespace IRIs as defined by the Croissant
  1.0 spec — RDF identifiers, not network endpoints. Strict consumers
  (mlcroissant validator, MLCommons reference loaders) compare these
  strings lexically; switching to `https://` would diverge from the
  spec-canonical form and break exact-match consumers. Note: SonarCloud
  may still surface these as hotspots-to-review even with NOSONAR
  because hotspots have a separate "human review" workflow; the user
  may need to mark them "Safe" once in the SonarCloud UI to clear the
  Quality Gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The per-directory .markdownlint-cli2.yaml under docs/usermanuals/ did
not clear the 42 MD051 warnings because Codacy's markdownlint
integration invokes the linter once with a fixed root-config flag and
does not walk per-directory config inheritance the way a local
markdownlint-cli2 run would.

Move the config to the repo root and disable MD051 globally with a
detailed rationale comment explaining the trade-off. The user-manual
SPA at site/guide.html uses `#/<section>/<page>` hash-router links
parsed by site/js/guide.js::parseHash — these are route identifiers,
not in-page anchors, so MD051 produces 42 false positives that drown
out genuine signal across 488 link sites.

Trade-off: a small number of standards/guides files use real
`#anchor-name` links that MD051 would otherwise validate. We accept
the loss because those files are human-edited and code review catches
obvious anchor breakage; all other markdownlint rules stay enforced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codacy's markdownlint integration reads the original markdownlint
config format (.markdownlint.yaml / .markdownlint.json) at the repo
root, not the markdownlint-cli2 variant. The previous
.markdownlint-cli2.yaml was silently ignored by Codacy, leaving the 42
MD051 warnings in place across two re-runs.

Switch to .markdownlint.yaml with the same MD051: false setting and
the same documented rationale. The format is also forward-compatible
with markdownlint-cli2 (which reads both shapes).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adding a markdownlint config file caused Codacy to switch from its own
narrow rule set (42 MD051 warnings) to markdownlint's default rule set
(229 issues including MD013 line-length, MD004 list-style, MD014
dollar-sign, MD025 single-h1, etc.). The trade-off was clearly worse:
229 cosmetic warnings drown out signal far more than the 42
documented-false-positive MD051 hash-router warnings.

Drop the config file. Path forward for the remaining 42 false
positives:
1. Mark them \"False Positive\" in the Codacy UI on a per-finding
   basis (preserves Codacy as a useful gate going forward), OR
2. Disable rule MD051 at project level in Codacy UI (admin action,
   one-time, applies to all future PRs), OR
3. Accept that Codacy reports them as ACTION_REQUIRED — this is a
   soft check that does not block merge unless the branch-protection
   policy requires it.

SonarCloud remains green (Quality Gate passed, 0 hotspots).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cemililik

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (1)
docs/usermanuals/en/data/croissant-card.md (1)

130-132: ⚠️ Potential issue | 🟡 Minor

Fix unresolved internal doc links in “See also”.

Line 130–132 use #/... fragments that fail MD051 in this repo layout. Switch to resolvable relative markdown links.

Suggested patch
-- [Dataset Audit](`#/data/audit`) — where `--croissant` is invoked.
-- [Annex IV](`#/compliance/annex-iv`) — the EU AI Act Article 11 artefact that the audit feeds into.
-- [GDPR / KVKK](`#/compliance/gdpr`) — broader regulatory context.
+- [Dataset Audit](./audit.md) — where `--croissant` is invoked.
+- [Annex IV](../compliance/annex-iv.md) — the EU AI Act Article 11 artefact that the audit feeds into.
+- [GDPR / KVKK](../compliance/gdpr.md) — broader regulatory context.

As per coding guidelines docs/**/*.md: Documentation must follow docs/standards/documentation.md and docs/standards/localization.md standards.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/usermanuals/en/data/croissant-card.md` around lines 130 - 132, The "See
also" links using `#/...` fragments (the entries "Dataset Audit", "Annex IV",
and "GDPR / KVKK") are unresolved and trigger MD051; update each link to a
resolvable relative markdown link to the actual documentation pages (replace the
`#/data/audit`, `#/compliance/annex-iv`, and `#/compliance/gdpr` fragments with
correct relative .md targets in the docs tree) so the links resolve in this repo
layout and conform to docs/standards/documentation.md and
docs/standards/localization.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/usermanuals/en/data/pii-ml.md`:
- Line 8: Replace the fragment-style internal links like "#/data/pii-masking" in
docs/usermanuals/en/data/pii-ml.md (occurring at the text around the line that
mentions "PII Masking" and also at the other referenced locations) with
resolvable internal doc links per the docs standards: use a relative or absolute
docs path (e.g., ../ or /docs/... style) that points to the target markdown file
instead of the "#/..." fragment, update all occurrences noted (the fragment at
"PII Masking" on line ~8, the one around line ~24, and the group at ~137–140),
and ensure the new links conform to docs/standards/documentation.md and
docs/standards/localization.md to satisfy MD051.

In `@docs/usermanuals/tr/data/croissant-card.md`:
- Around line 130-132: Replace unresolved fragment links in croissant-card.md —
specifically the link targets `#/data/audit`, `#/compliance/annex-iv`, and
`#/compliance/gdpr` — with correct relative .md paths to their corresponding
docs (e.g., the Audit, Annex IV, and GDPR/KVKK markdown files). Update the three
link targets in the paragraph containing "[Veri Seti Denetimi]", "[Annex IV]"
and "[GDPR / KVKK]" so they point to the actual relative file locations instead
of `#/...` fragments to satisfy docs/standards and fix MD051.

In `@docs/usermanuals/tr/data/pii-ml.md`:
- Line 8: Replace internal fragment-style links like [PII
Maskeleme](`#/data/pii-masking`) with relative .md links that resolve (e.g. [PII
Maskeleme](../data/pii-masking.md) or the correct relative path from the current
doc); update every occurrence of the "#/..." fragment format (including the
instance in the string "Varsayılan [PII Maskeleme](`#/data/pii-masking`)" and
other similar fragments) to use proper relative .md targets so MD051 warnings
are resolved and localization/documentation standards are followed.

In `@forgelm/data_audit.py`:
- Around line 405-418: _get_presidio_analyzer currently always returns a default
_PresidioAnalyzer (which constructs a presidio AnalyzerEngine with no args and
therefore only "en"), so the CLI flag --pii-ml-language / operator-config cannot
influence supported_languages; change _get_presidio_analyzer and
_PresidioAnalyzer to accept and propagate a language/model parameter (e.g.,
language or nlp_engine_provider) so callers can pass the configured
--pii-ml-language value, ensure the AnalyzerEngine is instantiated using a
custom NlpEngineProvider when a non-"en" language is requested, and update any
call sites (preflight checks and detect_pii_ml) to call
_get_presidio_analyzer(language) so supported_languages reflects the actual
analyzer configuration.
- Around line 537-545: The except block catching (ValueError, RuntimeError)
should log the recoverable Presidio ML‑NER per-row errors before returning an
empty dict: change the handler to capture the exception (e.g., except
(ValueError, RuntimeError) as exc:) and call logger.warning with a concise
message and the exception (include exc_info=True or pass exc) plus any available
per-row context (row id/record/index from the surrounding scope) so it's clear
this was a per-row ML‑NER failure; leave the return {} behavior intact and keep
_require_presidio semantics unchanged.

---

Duplicate comments:
In `@docs/usermanuals/en/data/croissant-card.md`:
- Around line 130-132: The "See also" links using `#/...` fragments (the entries
"Dataset Audit", "Annex IV", and "GDPR / KVKK") are unresolved and trigger
MD051; update each link to a resolvable relative markdown link to the actual
documentation pages (replace the `#/data/audit`, `#/compliance/annex-iv`, and
`#/compliance/gdpr` fragments with correct relative .md targets in the docs
tree) so the links resolve in this repo layout and conform to
docs/standards/documentation.md and docs/standards/localization.md.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 76cb804e-d7e2-4b22-a600-610931a45f35

📥 Commits

Reviewing files that changed from the base of the PR and between 7d0f150 and 42676af.

📒 Files selected for processing (15)
  • CHANGELOG.md
  • docs/guides/data_audit-tr.md
  • docs/guides/data_audit.md
  • docs/guides/ingestion-tr.md
  • docs/guides/ingestion.md
  • docs/roadmap-tr.md
  • docs/roadmap.md
  • docs/usermanuals/en/data/audit.md
  • docs/usermanuals/en/data/croissant-card.md
  • docs/usermanuals/en/data/pii-ml.md
  • docs/usermanuals/tr/data/audit.md
  • docs/usermanuals/tr/data/croissant-card.md
  • docs/usermanuals/tr/data/pii-ml.md
  • forgelm/data_audit.py
  • tests/test_phase12_5.py

Comment thread docs/usermanuals/en/data/pii-ml.md Outdated

# ML-NER PII Detection — `--pii-ml`

The default [PII Masking](#/data/pii-masking) layer is regex-anchored and covers the *structured* identifiers GDPR Article 10 cares about (email, phone, IBAN, credit card, national IDs). Presidio NER adds the *unstructured* identifiers regex inherently misses: person names, organisation names, geographic locations.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Status: declined — false positive against the project's SPA convention.

#/<section>/<page> is the user-manual SPA hash-router scheme parsed at runtime by site/js/guide.js:69-71 (parseHash). These are route identifiers, not in-page anchors — markdownlint's MD051 can't distinguish the two and produces 42 false positives across 488 link sites.

Switching to relative .md paths (../data/audit.md) would break SPA navigation: tools/build_usermanuals.py does not rewrite link targets, so <a href="../data/audit.md"> would render as a literal href that the browser navigates to a non-existent static URL.

The proper resolution is on the Codacy side: either disable rule MD051 at project level in the Codacy UI (one-time admin action), or mark these specific findings as False Positive. SonarCloud's Quality Gate is already passing.

This thread covers the same issue across 4 files (en/tr × pii-ml/croissant-card); leaving as-is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@cemililik, understood — thanks for the detailed explanation. The #/<section>/<page> scheme being a runtime hash-router contract (not in-page anchors) is a meaningful distinction, and the reasoning around tools/build_usermanuals.py not rewriting link targets makes the relative-path suggestion actively harmful here. I'll record this for future reviews.


✏️ Learnings added
Learnt from: cemililik
Repo: cemililik/ForgeLM PR: 18
File: docs/usermanuals/en/data/pii-ml.md:8-8
Timestamp: 2026-04-29T20:40:44.227Z
Learning: In the ForgeLM repository (cemililik/ForgeLM), links in the format `#/<section>/<page>` (e.g., `#/data/pii-masking`, `#/data/audit`, `#/operations/air-gap`) within `docs/usermanuals/**/*.md` files are intentional SPA hash-router route identifiers, NOT in-page anchors. They are parsed at runtime by `site/js/guide.js` (the `parseHash` function around lines 69-71). markdownlint rule MD051 ("link-fragments") produces false positives for these links (42 false positives across 488 link sites). Do NOT suggest replacing these with relative `.md` paths — `tools/build_usermanuals.py` does not rewrite link targets, so relative paths like `./pii-masking.md` would render as broken static URLs in the SPA and break navigation. The correct resolution is to suppress MD051 at the Codacy project level as a false positive.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +130 to +132
- [Veri Seti Denetimi](#/data/audit) — `--croissant`'ın çağrıldığı yer.
- [Annex IV](#/compliance/annex-iv) — audit'in beslediği EU AI Act Madde 11 artefaktı.
- [GDPR / KVKK](#/compliance/gdpr) — daha geniş düzenleyici bağlam.

@coderabbitai coderabbitai Bot Apr 29, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

“Bakınız” linkleri çözülmeyen fragment formatında.

Line 130–132’deki #/... bağlantıları bu docs ağacında çözümlenmiyor (MD051). Relative .md hedeflerine geçin.

Önerilen yama
-- [Veri Seti Denetimi](`#/data/audit`) — `--croissant`'ın çağrıldığı yer.
-- [Annex IV](`#/compliance/annex-iv`) — audit'in beslediği EU AI Act Madde 11 artefaktı.
-- [GDPR / KVKK](`#/compliance/gdpr`) — daha geniş düzenleyici bağlam.
+- [Veri Seti Denetimi](./audit.md) — `--croissant`'ın çağrıldığı yer.
+- [Annex IV](../compliance/annex-iv.md) — audit'in beslediği EU AI Act Madde 11 artefaktı.
+- [GDPR / KVKK](../compliance/gdpr.md) — daha geniş düzenleyici bağlam.

As per coding guidelines docs/**/*.md: Documentation must follow docs/standards/documentation.md and docs/standards/localization.md standards.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- [Veri Seti Denetimi](#/data/audit) — `--croissant`'ın çağrıldığı yer.
- [Annex IV](#/compliance/annex-iv) — audit'in beslediği EU AI Act Madde 11 artefaktı.
- [GDPR / KVKK](#/compliance/gdpr) — daha geniş düzenleyici bağlam.
- [Veri Seti Denetimi](./audit.md) — `--croissant`'ın çağrıldığı yer.
- [Annex IV](../compliance/annex-iv.md) — audit'in beslediği EU AI Act Madde 11 artefaktı.
- [GDPR / KVKK](../compliance/gdpr.md) — daha geniş düzenleyici bağlam.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 130-130: Link fragments should be valid

(MD051, link-fragments)


[warning] 131-131: Link fragments should be valid

(MD051, link-fragments)


[warning] 132-132: Link fragments should be valid

(MD051, link-fragments)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/usermanuals/tr/data/croissant-card.md` around lines 130 - 132, Replace
unresolved fragment links in croissant-card.md — specifically the link targets
`#/data/audit`, `#/compliance/annex-iv`, and `#/compliance/gdpr` — with correct
relative .md paths to their corresponding docs (e.g., the Audit, Annex IV, and
GDPR/KVKK markdown files). Update the three link targets in the paragraph
containing "[Veri Seti Denetimi]", "[Annex IV]" and "[GDPR / KVKK]" so they
point to the actual relative file locations instead of `#/...` fragments to
satisfy docs/standards and fix MD051.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Status: declined — false positive against the project's SPA convention.

#/<section>/<page> is the user-manual SPA hash-router scheme parsed at runtime by site/js/guide.js:69-71 (parseHash). These are route identifiers, not in-page anchors — markdownlint's MD051 can't distinguish the two and produces 42 false positives across 488 link sites.

Switching to relative .md paths (../data/audit.md) would break SPA navigation: tools/build_usermanuals.py does not rewrite link targets, so <a href="../data/audit.md"> would render as a literal href that the browser navigates to a non-existent static URL.

The proper resolution is on the Codacy side: either disable rule MD051 at project level in the Codacy UI (one-time admin action), or mark these specific findings as False Positive. SonarCloud's Quality Gate is already passing.

This thread covers the same issue across 4 files (en/tr × pii-ml/croissant-card); leaving as-is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@cemililik, understood — I apologize for the false positive. The #/<section>/<page> format is a SPA hash-router convention, not a standard Markdown in-page anchor, and MD051 cannot distinguish between the two. Switching to relative .md paths would indeed break runtime navigation since tools/build_usermanuals.py does not rewrite link targets. I'll keep this in mind for future reviews.


✏️ Learnings added
Learnt from: cemililik
Repo: cemililik/ForgeLM PR: 18
File: docs/usermanuals/tr/data/croissant-card.md:130-132
Timestamp: 2026-04-29T20:40:46.196Z
Learning: In the ForgeLM repository (cemililik/ForgeLM), `#/<section>/<page>` link targets in docs/usermanuals/**/*.md files are SPA hash-router route identifiers parsed at runtime by site/js/guide.js (parseHash, lines 69-71). They are NOT in-page anchors. markdownlint MD051 produces false positives for all 42 such occurrences across 488 link sites. Do NOT flag these as broken fragment links. Switching to relative `.md` paths would break SPA navigation because tools/build_usermanuals.py does not rewrite link targets.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


# ML-NER PII Tespiti — `--pii-ml`

Varsayılan [PII Maskeleme](#/data/pii-masking) katmanı regex-anchored'dır ve GDPR Madde 10'un önemsediği *yapılandırılmış* identifier'ları (email, telefon, IBAN, kredi kartı, ulusal kimlikler) kapsar. Presidio NER, regex'in doğal olarak kaçırdığı *yapılandırılmamış* identifier'ları ekler: kişi isimleri, organizasyon isimleri, coğrafi yerler.

@coderabbitai coderabbitai Bot Apr 29, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

TR sayfasındaki dahili linkler çözümlenemiyor (#/...).

Line 8, Line 24 ve Line 137–140 için fragment formatı MD051 uyarısı üretiyor; relative .md linklere geçilmeli.

Önerilen yama
-Varsayılan [PII Maskeleme](`#/data/pii-masking`) katmanı regex-anchored'dır ...
+Varsayılan [PII Maskeleme](./pii-masking.md) katmanı regex-anchored'dır ...

-- Pre-staged spaCy NER modeli olmadan air-gap ortamda çalışıyorsunuz — [Air-Gap Operasyonu](`#/operations/air-gap`)'na bakın.
+- Pre-staged spaCy NER modeli olmadan air-gap ortamda çalışıyorsunuz — [Air-Gap Operasyonu](../operations/air-gap.md)'na bakın.

-- [PII Maskeleme](`#/data/pii-masking`) — her zaman açık olan regex katmanı.
-- [Veri Seti Denetimi](`#/data/audit`) — `--pii-ml`'in çağrıldığı yer.
-- [GDPR / KVKK](`#/compliance/gdpr`) — düzenleyici bağlam.
-- [Air-Gap Operasyonu](`#/operations/air-gap`) — spaCy modelini ön-stage etme.
+- [PII Maskeleme](./pii-masking.md) — her zaman açık olan regex katmanı.
+- [Veri Seti Denetimi](./audit.md) — `--pii-ml`'in çağrıldığı yer.
+- [GDPR / KVKK](../compliance/gdpr.md) — düzenleyici bağlam.
+- [Air-Gap Operasyonu](../operations/air-gap.md) — spaCy modelini ön-stage etme.

As per coding guidelines docs/**/*.md: Documentation must follow docs/standards/documentation.md and docs/standards/localization.md standards.

Also applies to: 24-24, 137-140

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 8-8: Link fragments should be valid

(MD051, link-fragments)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/usermanuals/tr/data/pii-ml.md` at line 8, Replace internal
fragment-style links like [PII Maskeleme](`#/data/pii-masking`) with relative .md
links that resolve (e.g. [PII Maskeleme](../data/pii-masking.md) or the correct
relative path from the current doc); update every occurrence of the "#/..."
fragment format (including the instance in the string "Varsayılan [PII
Maskeleme](`#/data/pii-masking`)" and other similar fragments) to use proper
relative .md targets so MD051 warnings are resolved and
localization/documentation standards are followed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Status: declined — false positive against the project's SPA convention.

#/<section>/<page> is the user-manual SPA hash-router scheme parsed at runtime by site/js/guide.js:69-71 (parseHash). These are route identifiers, not in-page anchors — markdownlint's MD051 can't distinguish the two and produces 42 false positives across 488 link sites.

Switching to relative .md paths (../data/audit.md) would break SPA navigation: tools/build_usermanuals.py does not rewrite link targets, so <a href="../data/audit.md"> would render as a literal href that the browser navigates to a non-existent static URL.

The proper resolution is on the Codacy side: either disable rule MD051 at project level in the Codacy UI (one-time admin action), or mark these specific findings as False Positive. SonarCloud's Quality Gate is already passing.

This thread covers the same issue across 4 files (en/tr × pii-ml/croissant-card); leaving as-is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@cemililik, understood — thanks for the thorough explanation. The #/<section>/<page> links are SPA route identifiers consumed by parseHash at runtime, not in-page anchors, so MD051 is indeed a false positive here and relative .md paths would break navigation. I'll record this for future reviews.

(ᴗ_ ᴗ。)


✏️ Learnings added
Learnt from: cemililik
Repo: cemililik/ForgeLM PR: 18
File: docs/usermanuals/tr/data/pii-ml.md:8-8
Timestamp: 2026-04-29T20:40:46.342Z
Learning: In the ForgeLM repository (`docs/usermanuals/**/*.md`), links in the format `#/<section>/<page>` (e.g., `#/data/pii-masking`, `#/data/audit`, `#/compliance/gdpr`, `#/operations/air-gap`) are intentional SPA hash-router route identifiers parsed at runtime by `site/js/guide.js` (`parseHash` function). They are NOT standard in-page markdown anchors. markdownlint MD051 produces false positives for these links (42 false positives across 488 link sites). Do NOT suggest replacing them with relative `.md` paths — `tools/build_usermanuals.py` does not rewrite link targets, so relative `.md` hrefs would break SPA navigation by pointing to non-existent static URLs.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment thread forgelm/data_audit.py Outdated
Comment thread forgelm/data_audit.py Outdated
Comment on lines +537 to +545
except (ValueError, RuntimeError): # pragma: no cover — Presidio edge cases
# Per-row resilience for the narrow class of failures Presidio
# raises on bad input or transient engine state. ``OSError`` is
# deliberately NOT caught here — that's the missing-spaCy-model
# signal and ``_require_presidio``'s pre-flight should have
# converted it to ImportError before any row was scanned. If
# one slips through (e.g. lazy model load triggered later),
# surfacing it loudly is the correct behaviour.
return {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd /tmp/repo && head -c 1000 forgelm/data_audit.py | head -20

Repository: cemililik/ForgeLM

Length of output: 121


🏁 Script executed:

cd /tmp/repo && wc -l forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 121


🏁 Script executed:

cd /tmp/repo && sed -n '520,560p' forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 121


🏁 Script executed:

cd /tmp/repo && sed -n '480,550p' forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 121


🏁 Script executed:

wc -l forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 88


🏁 Script executed:

sed -n '520,560p' forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 1952


🏁 Script executed:

sed -n '470,550p' forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 4161


🏁 Script executed:

cat -n docs/standards/error-handling.md

Repository: cemililik/ForgeLM

Length of output: 8231


🏁 Script executed:

grep -n "logger\|logging\|warn" forgelm/data_audit.py | head -30

Repository: cemililik/ForgeLM

Length of output: 710


🏁 Script executed:

sed -n '1,50p' forgelm/data_audit.py

Repository: cemililik/ForgeLM

Length of output: 1954


Add logging for Presidio ML-NER per-row errors.

Returning {} without logging hides whether detection succeeded (no PII) or failed (error). Per error-handling standards, "log and swallow" patterns require logging unless the path is explicitly non-fatal (webhooks, cleanup). Use logger.warning() to surface recoverable errors while continuing per-row resilience:

Fix
-    except (ValueError, RuntimeError):  # pragma: no cover — Presidio edge cases
+    except (ValueError, RuntimeError) as exc:  # pragma: no cover — Presidio edge cases
         # Per-row resilience for the narrow class of failures Presidio
         # raises on bad input or transient engine state. ``OSError`` is
         # deliberately NOT caught here — that's the missing-spaCy-model
         # signal and ``_require_presidio``'s pre-flight should have
         # converted it to ImportError before any row was scanned. If
         # one slips through (e.g. lazy model load triggered later),
         # surfacing it loudly is the correct behaviour.
+        logger.warning(
+            "Presidio ML-NER skipped a row due to %s (language=%s): %s",
+            type(exc).__name__,
+            language,
+            exc,
+        )
         return {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` around lines 537 - 545, The except block catching
(ValueError, RuntimeError) should log the recoverable Presidio ML‑NER per-row
errors before returning an empty dict: change the handler to capture the
exception (e.g., except (ValueError, RuntimeError) as exc:) and call
logger.warning with a concise message and the exception (include exc_info=True
or pass exc) plus any available per-row context (row id/record/index from the
surrounding scope) so it's clear this was a per-row ML‑NER failure; leave the
return {} behavior intact and keep _require_presidio semantics unchanged.

… log per-row Presidio failures

Two CodeRabbit findings on the previous review pass were valid and code-tractable:

1. _get_presidio_analyzer was hardcoded to AnalyzerEngine() (English-only).
   The --pii-ml-language flag added in the previous commit only validated
   the request against the analyzer's supported_languages — for non-English
   it always raised because the analyzer never registered anything but
   English. Now _get_presidio_analyzer accepts a language parameter and
   builds a per-language NlpEngineProvider for the codes in
   _SPACY_MODEL_FOR_LANGUAGE (en, de, es, fr, it, ja, ko, nl, pl, pt, ru,
   zh, plus 'xx' multilingual fallback). Languages outside the map raise
   ValueError up front with a hint pointing at xx or the Python-API custom
   path. lru_cache widened to maxsize=8 for realistic multi-language audits.

2. The per-row (ValueError, RuntimeError) handler in detect_pii_ml swallowed
   silently, leaving operators with zero diagnostic when ML coverage was
   unexpectedly empty. Now logs at DEBUG with exc_info, language, and the
   exception — visible under --log-level DEBUG, silent in normal runs so
   pathological-corpora floods don't drown out audit findings.

Tests:
- TestPresidioLanguagePreflight.test_unmapped_language_raises_value_error_at_analyzer_build
- TestPresidioPerRowErrorLogging.test_per_row_value_error_emits_debug_log

Docs: pii-ml.md (en + tr) updated with the supported-language list, the
de_core_news_lg example, and the xx multilingual-fallback recipe.

CodeRabbit's remaining 4 findings (convert #/data/... links to relative .md
paths) were rejected:

- The #/<section>/<page> pattern is the SPA hash-router convention used
  in 488 places across docs/usermanuals/, parsed at runtime by
  site/js/guide.js::parseHash. tools/build_usermanuals.py does NOT
  rewrite link targets — switching to ../foo.md would render as a literal
  href that the browser navigates to a non-existent static URL, breaking
  the user-manual SPA navigation. The MD051 false positives stay; the
  documented path forward is to disable MD051 in the Codacy UI at project
  level (one-time admin action). SonarCloud is already passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cemililik

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread docs/usermanuals/en/data/pii-ml.md Outdated

# ML-NER PII Detection — `--pii-ml`

The default [PII Masking](#/data/pii-masking) layer is regex-anchored and covers the *structured* identifiers GDPR Article 10 cares about (email, phone, IBAN, credit card, national IDs). Presidio NER adds the *unstructured* identifiers regex inherently misses: person names, organisation names, geographic locations.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Status: declined — false positive against the project's SPA convention.

#/<section>/<page> is the user-manual SPA hash-router scheme parsed at runtime by site/js/guide.js:69-71 (parseHash). These are route identifiers, not in-page anchors — markdownlint's MD051 can't distinguish the two and produces 42 false positives across 488 link sites.

Switching to relative .md paths (../data/audit.md) would break SPA navigation: tools/build_usermanuals.py does not rewrite link targets, so <a href="../data/audit.md"> would render as a literal href that the browser navigates to a non-existent static URL.

The proper resolution is on the Codacy side: either disable rule MD051 at project level in the Codacy UI (one-time admin action), or mark these specific findings as False Positive. SonarCloud's Quality Gate is already passing.

This thread covers the same issue across 4 files (en/tr × pii-ml/croissant-card); leaving as-is.

….5.1 plan

Addresses Gemini-medium suggestion on PR #18: the prior phrasing
'(no unreleased changes — next minor will pick up Phase 14, tracked in
docs/roadmap.md)' was ambiguous about whether the v0.5.0 release was
finalized vs. still mid-flight. Reword to 'Empty — v0.5.0 is finalized.
The next minor (v0.5.1) covers Phase 14 ...' so the section state is
explicit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
forgelm/data_audit.py (1)

2506-2546: Consider extracting "@type" to a constant to satisfy SonarCloud.

SonarCloud flags the literal "@type" appearing 6 times across the Croissant metadata construction. While this is a JSON-LD vocabulary key and the duplication is semantically meaningful, extracting it to a module-level constant would silence the linter and marginally improve maintainability.

♻️ Optional refactor
+_JSONLD_TYPE_KEY: str = "@type"
+
 _CROISSANT_CONTEXT: Dict[str, Any] = {
     "@language": "en",
     ...

Then use _JSONLD_TYPE_KEY in the distribution/recordSet builders. This is a low-priority nit — the current form is perfectly readable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` around lines 2506 - 2546, SonarCloud complaints come
from repeating the literal "@type" in the Croissant JSON-LD build; introduce a
module-level constant (e.g. _JSONLD_TYPE_KEY = "@type") and replace all
occurrences of the string literal in _CROISSANT_CONTEXT and the related
distribution/recordSet builders with that constant (search for uses in the
Croissant metadata construction code and update keys like "data": {"@id":
"cr:data", "@type": "@json"} to use _JSONLD_TYPE_KEY) so the linter is satisfied
while preserving identical runtime behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/usermanuals/en/data/pii-ml.md`:
- Line 8: Replace the British spelling "organisation names" in the sentence that
starts "The default PII Masking layer..." with the American spelling
"organization names" so it matches the rest of the document and the code
category name "organization"; ensure any other occurrences of "organisation" in
this file are changed to "organization" for consistency.

---

Nitpick comments:
In `@forgelm/data_audit.py`:
- Around line 2506-2546: SonarCloud complaints come from repeating the literal
"@type" in the Croissant JSON-LD build; introduce a module-level constant (e.g.
_JSONLD_TYPE_KEY = "@type") and replace all occurrences of the string literal in
_CROISSANT_CONTEXT and the related distribution/recordSet builders with that
constant (search for uses in the Croissant metadata construction code and update
keys like "data": {"@id": "cr:data", "@type": "@json"} to use _JSONLD_TYPE_KEY)
so the linter is satisfied while preserving identical runtime behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e717c18f-c8e8-4b07-8eb6-b7ab64dab2ac

📥 Commits

Reviewing files that changed from the base of the PR and between 42676af and 502ff88.

📒 Files selected for processing (4)
  • docs/usermanuals/en/data/pii-ml.md
  • docs/usermanuals/tr/data/pii-ml.md
  • forgelm/data_audit.py
  • tests/test_phase12_5.py

Comment thread docs/usermanuals/en/data/pii-ml.md Outdated
…ey constants

CodeRabbit findings on commit 7fae240:

1. docs/usermanuals/en/data/pii-ml.md:8 — switch British 'organisation
   names' to American 'organization names' to match the rest of the
   file (frontmatter, sample output, severity table, code category
   name 'organization' from _PRESIDIO_ENTITY_MAP).

2. forgelm/data_audit.py — extract _JSONLD_TYPE_KEY and _JSONLD_ID_KEY
   module-level constants, replace literal '@type' / '@id' uses inside
   _build_croissant_metadata. The literals only remain inside
   _CROISSANT_CONTEXT (the verbatim JSON-LD spec snippet) where they
   are part of the spec data, not duplicated keys. Drops the literal
   count from 6× '@type' / 7× '@id' down to 2× of each — below
   SonarCloud S1192's default threshold (3+) so the informational
   issues clear without affecting runtime behaviour.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@cemililik
cemililik merged commit 48a8eb9 into main Apr 29, 2026
10 of 11 checks passed
cemililik added a commit that referenced this pull request May 6, 2026
… drift guard

Wave 5 / Faz 30 Task J: a CI guard that catches the bug class we've
chased four times this wave (GH-8 verify-log, GH-11 benchmark,
GH-16 --export-bundle, GH-18 --target kserve, GH-20 --pii-locale,
F-W4-01 verify-audit --output-dir/--json, F-W4-PS-02 approve --run-id).

Tool design (749 LOC, pure stdlib):
- Discovers the live parser surface by spawning `python3 -m forgelm.cli
  --help` + `forgelm <sub> --help` for every shipped subcommand —
  resilient to internal `_parser.py` reorganisation (Option A in the
  task spec).
- Walks docs/{guides,reference,usermanuals/{en,tr}/} + README.md for
  fenced bash/shell code blocks containing `forgelm <sub> ...`
  invocations.
- Reports three drift classes: (1) subcommand not in parser surface;
  (2) flag not in parser surface for that subcommand; (3) flag value
  not in parser's `choices` list.
- Skips `docs/analysis/` (gitignored research) + `docs/marketing/`
  (gitignored strategy) by default.
- False-positive avoidance: skips synopsis lines (containing `[` or
  `(...|...)` choice indicators), and skips code blocks in
  "Wrong:"/"Don't:"/"Anti-pattern:"/"Legacy:"/"Yanlış:"-tagged
  context. Forward-reference framing (`planned`/`roadmap`/`future`/
  `not in v0.5.5`/`(planned)`) within ±3 lines of the cited line is
  also tolerated as a legitimate ghost callout.
- Exit codes follow `tools/` contract: 0 advisory / 1 strict.

Tests (298 LOC, 15 cases — 3:23 wall-clock since each spawns the
real forgelm parser surface):
- Real shipping syntax (`verify-audit ./outputs/audit_log.jsonl
  --require-hmac`) → exit 0.
- Each historical drift class regression-pinned (verify-audit
  --output-dir/--json, benchmark --model X, deploy --target kserve,
  chat --top-p).
- Forward-reference framing + Wrong:-tagged blocks → tolerated.
- --quiet suppression of the OK summary.
- Parser-surface discovery sanity tests (synthetic empty repo,
  missing-subcommand short-circuit).

CI wiring:
- New advisory-mode step in `.github/workflows/ci.yml` runs the tool
  with `continue-on-error: true` after the bilingual-parity step.
  Matches the `check_anchor_resolution.py` Faz 26 precedent: gate
  lands first, baseline cleanup follows, then a later commit flips
  to `--strict` fail-the-build mode.

Baseline drift on the live tree: 40 findings across 20 docs. Top
hits cluster in `docs/usermanuals/{en,tr}/reference/cli.md` (legacy
flag tables drifted from the audit/verify-audit/chat/deploy/ingest
parsers as Wave 2-4 reshaped them) and a handful of phase-13/14
"planned" subcommands (`forgelm trend`, `forgelm compare-runs`)
documented as if shipped. Cleanup is the next Wave 5 commit.

Validation
- ruff format / check — clean.
- python3 -m pytest tests/test_check_cli_help_consistency.py --no-cov
  -q — 15/15 pass in 3:23.
- python3 -m pytest --no-cov -q — full suite green (1428 tests now;
  +15 from this commit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cemililik added a commit that referenced this pull request May 6, 2026
…gent review findings

- Wave 5 PR #34 round 3 absorption: full sweep of CodeRabbit's 6 OOD +
  20 Minor inline review comments PLUS 24 multi-agent review findings
  (code-correctness / privacy-security / standards / test-rigor docs
  under docs/analysis/code_reviews/wave5-*.md).

HIGH/Major absorptions:
- 4 broken `forgelm.cli.subcommands.*` symbol citations across
  purge/approvals reference docs (en+tr): _run_row_erasure →
  _run_purge_row_id, _handle_pending → _run_approvals_list_pending,
  _handle_show → _run_approvals_show (F-W5-PRIV-02).
- configuration.md (en+tr): drop `kserve | triton` ghost choices and
  the entire `## deployment:` ghost YAML block (no `DeploymentConfig`
  in ForgeConfig; extra="forbid" rejects it at load) — replace with a
  v0.6.0+ Phase 14 callout (F-W5-S1, F-W5-S2).
- ingestion.md (en+tr): drop conflicting "instructions format
  default" claim; document raw-chunk JSONL with summary-only
  --output-format {text,json} (CR-OOD ingestion).
- closure-plan PR #33 status: line 131 said "PR #33 pending" but
  line 119 said merged with `01e40ba` (CR-OOD closure-plan).
- leakage.md (en+tr): remove `audit.leakage_check.remove_cross_split_overlap`
  knob reference — knob doesn't exist in code; replace with the
  documented jq-based removal flow (CR-OOD leakage).
- TR + EN experiment-tracking: drop `$ ` prompt prefix in `text`-tagged
  preview blocks for `forgelm compare-runs` (CR-OOD experiment-tracking).
- roadmap mermaid (en+tr): add Phase 12.6 (Closure Cycle) → v0.5.5
  node + edge so visual flow matches table (CR-OOD roadmap).

Minor absorptions:
- phase-12-6-closure-cycle.md L123: stale "(currently 0.5.1rc1)"
  parenthetical → already-bumped during Wave 5 (F-W5-S4 / CR Minor).
- gdpr-erasure.md (en+tr): rename duplicate `### What gets recorded`
  / `### Neyi kaydeder` H3 to disambiguate Article 17 vs 15 sections
  (CR Minor #2 + #20).
- verify-gguf.md (en+tr) L147: revise misleading "still match SHA-256"
  hash explanation; clarify magic-header layer doesn't catch
  truncation independent of sidecar (CR Minor #3).
- verify_audit link mismatch in 4 reference docs (en+tr × verify_gguf
  + verify_annex_iv): label text now matches href (CR Minor #4 + #8).
- air_gap_deployment-tr.md L19: add `text` language tag to ASCII
  diagram fence (CR Minor #6).
- site/index.html L374: drop internal "Phase 19" milestone leak; tag
  reads `embed` (user-facing label) (CR Minor #7).
- air_gap_deployment.md L193: fix dangling sentence fragment in
  HF_TOKEN gated-models tip (CR Minor #9).
- performance.md (en+tr) L184: drop ghost `forgelm audit
  --output-format parquet`; replace with pandas/pyarrow conversion
  step (CR Minor #10).
- phase-11-5-backlog.md L10-12: link text → package paths matching
  href (forgelm/data_audit.py → forgelm/data_audit/, etc.) (CR Minor #11).
- testing.md L140: stale `23/23` → `39/39` bilingual parity count
  (CR Minor #12).
- testing.md L198: `python3` prefix consistency in checklist
  (CR Minor #13).
- supply-chain.md (en+tr): bandit cadence aligned to standards doc
  (Nightly 03:00 UTC + release tag, not "every PR + main push") (CR
  Minor #14 + #15).
- translations.js L1927: "Three values" → "Two values" (only
  forgelm-lang + forgelm-theme listed) (CR Minor #16).
- approvals_subcommand.md (en+tr) L26: staging path → match L84 JSON
  sample (`final_model.staging.<run_id>` canonical, run-id-less form
  honoured as fallback) (CR Minor #17 + #19).
- translations.js L1445/L1462/L1504/L1521 (en+tr): `<code>reject</code>`
  → `<code>forgelm reject</code>` for fully-qualified copy/paste
  parity (CR Minor #18).
- _meta.yaml header: add formal DE/FR/ES/ZH deferred-policy banner
  per closure-plan Task L (F-W5-S6).
- release.md L20 + L359: stale "0.3.1rc1 current" + "no
  __version__" claims; corrected to 0.5.5 + reflect that __version__
  + __api_version__ ARE exported per Library API standard (F-W5-S3).
- purge_subcommand.md (en+tr): add WARNING-on-fallback language for
  --output-dir asymmetry vs reverse-pii (F-W5-PRIV-05).
- verify-audit.md (en+tr): replace overstated "escrow" wording with
  KMS/HSM-based separation (F-W5-PRIV-06).

Tool tightening + new tests:
- check_cli_help_consistency.py: tighten _FORWARD_REF_MARKERS to
  whole-phrase forms (drop bare "planned"/"future"/"roadmap" which
  matched "planned attacks" / "future work" / "roadmap-driven" in
  unrelated prose); add Turkish equivalents for bilingual docs
  (F-W5-01).
- test_check_cli_help_consistency.py: 3 new regression tests:
  bare-substring negative (F-W5-01), >±3-line forward-ref window
  negative (F-W5-TR-01a), and Wrong:-block scope-to-one-block
  resumption (F-W5-TR-01b). Suite 17 → 20 tests, all green.

User-manual orphan fix:
- _meta.yaml: register `reference/json-output` page (existed in
  filesystem since Wave 2 but never declared in nav config — was
  invisible on the live site). Now declared = on-disk = 65 each.

Skipped (with reason):
- CR Minor #5 (library_api.md `forgelm.__version__` example):
  INVALID — verified `python3 -c "import forgelm; print(forgelm.__version__)"`
  prints 0.5.5 cleanly; both __version__ and __api_version__ ARE
  exported per Library API standard. CodeRabbit's web-search must
  have hit pre-Wave-5 PyPI. Doc is correct; code matches.
- F-W5-S5 (parity tool autodiscover usermanuals): registry shape
  refactor (~50 new pairs) is scope creep for round 3; defer to v0.6.
- F-W5-S8 (compliance_summary-tr.md): explicitly deferred to v0.6 by
  the finding itself.
- F-W5-S9, F-W5-TR-11/12 (NIT cite/style): non-load-bearing.
- F-W5-02/03/04 (cognitive complexity, unterminated fence,
  resume-after-Wrong unit test): refactor scope; covered by F-W5-TR-01
  block-scope tests for the resume case.
- F-W5-TR-02 (cleanup commit baseline regression test): the live CI
  --strict gate IS the guard; doubling up the integration-test
  layer is defensible but not blocking.
- F-W5-TR-03/05/06/07/08/09/10 (extras): non-blocking coverage gaps.

Self-review gauntlet (all green):
  ruff format/check ✅
  bilingual_parity --strict 39/39 ✅
  anchor_resolution --strict 264/264 ✅
  cli_help_consistency --strict 425/425 ✅ (reduced from 449 due to
    intentional invocation removals: ghost YAML block, parquet ghost,
    4 broken symbol-cite paths, etc.)
  site_claims --strict (artefacts/templates/GPU/version) ✅
  field_descriptions --strict ✅
  pytest 1433 passed / 14 skipped (+5 new tests vs round 2) ✅
  forgelm --config config_template.yaml --dry-run ✅
  build_usermanuals.py: en/tr 65/65 OK, JS bundles regenerate ✅

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant