[cemililik]

Summary

This PR brings the development branch up to v0.5.0 state and lands Phase 11 — Document Ingestion & Data Audit plus post-release housekeeping from v0.4.5.

• Phase 11 — Document Ingestion (forgelm ingest): Multi-format extractors (PDF / DOCX / EPUB / TXT / Markdown) with paragraph + sliding chunking, optional --pii-mask. Output is {"text": ...} JSONL the trainer accepts. OCR is out of scope; see new guide for Tesseract / AWS Textract handoff.
• Phase 11 — Dataset Audit (forgelm --data-audit): CPU-only audit producing data_audit_report.json with per-split metrics, top-3 language detection, simhash near-duplicate detection, cross-split leakage check, PII regex (Luhn + TC Kimlik validated), schema-drift detection. Auto-inlines into EU AI Act Article 10 governance artifact when present at training time.
• 19 review findings applied, 14 deferred to a documented Phase 11.5 backlog (LSH banding, streaming reader, etc.).
• Post-v0.4.5 sync: roadmap / phase-12-quickstart / releases.md updated to reflect v0.4.5 PyPI publish; pyproject.toml + forgelm/__init__.py bumped to 0.5.0rc1 for the next dev cycle.
• README: PyPI badge added; all 9 notebooks listed (was 5/9).

Phase 11 — what's new

New modules

• forgelm/ingestion.py — single file or directory tree → SFT-ready JSONL. Sliding + paragraph chunking, recursive walk, encrypted-PDF detection, binary-content warning, deterministic file ordering.
• forgelm/data_audit.py — JSONL → audit JSON. Sample count, length distribution (p50/p95), language detection, simhash near-duplicate detection (within-split + cross-split leakage), PII regex with Luhn + TC Kimlik checksum validation, schema-drift detection.

New CLI surface

forgelm ingest ./policies/ --recursive --output data/policies.jsonl
forgelm ingest ./book.epub --strategy sliding --chunk-size 2048 --overlap 200 --output data/book.jsonl
forgelm ingest ./customer_emails/ --pii-mask --output data/anon.jsonl

forgelm --data-audit data/policies.jsonl --output ./audit/
forgelm --data-audit data/  # split-keyed directory

New / updated guides

• docs/guides/ingestion.md — full ingestion guide including OCR handoff recipes (Tesseract via ocrmypdf, AWS Textract).
• docs/guides/data_audit.md — full audit guide: split layouts, alias map, PII patterns, Article 10 integration.
• docs/qms/sop_data_management.md — audit pipeline added to the data-management SOP.
• docs/standards/architecture.md — module table + extras matrix updated.
• docs/roadmap.md, docs/roadmap-tr.md, docs/roadmap/phase-11-data-ingestion.md, docs/roadmap/releases.md — Phase 11 marked Done.
• docs/roadmap/phase-11-5-backlog.md — new backlog file capturing 12 deferred follow-ups (LSH banding, streaming reader, PII severity tiers, etc.).

Optional dependency

pip install -e ".[ingestion]"   # pypdf, python-docx, ebooklib, beautifulsoup4, langdetect

Plain TXT / Markdown ingestion + the full audit module work without the extra (everything is pure stdlib in those paths).

Review findings — applied vs deferred

19 of 34 findings from the post-Phase-11 code review are applied in this PR. Highlights:

#	Bug	Fix
1	semantic strategy crashed at runtime	Removed from --strategy choices
2	--overlap 199 --chunk-size 200 → quadratic chunk count	Reject overlap > chunk_size // 2 up front
3	Heterogeneous JSONL columns silently dropped	Compute union of keys + schema_drift_columns
6	Phone PII regex over-matched (timestamps, line numbers, ISO dates)	Anchored to +CC or (area) formats
7	DE Personalausweis regex matched IATA codes / UUID fragments	Narrowed to [A-Z]\d{7,8}[A-Z0-9]?
9	Encrypted PDF surfaced as opaque "extraction failed"	Explicit ValueError with qpdf / pdftk hint
10	Binary masquerading as text silently corrupted JSONL	Warn at >1% Unicode replacement chars
14	dev.jsonl / eval.jsonl not recognised as splits	Alias map → canonical
24	--data-audit --output-format json flooded stdout	Summary on stdout, full report to disk
26	Pseudo-split fallback silent	Warning + report note
27	Audit report carried only absolute path	Added source_input field
30	--pii-mask redacted count not surfaced	IngestionResult.pii_redaction_counts per category
31	One bad split aborted whole audit	Per-split tolerance: skip + report under error key
34	Audit notes informational, not actionable	Notes now suggest forgelm ingest --pii-mask etc.

Plus three "green tier" follow-ups landed in this PR:

• ebooklib options= deprecation silenced
• Audit progress logging (per-split open + every-5000-rows simhash + near-dup start)
• OCR handoff documentation (Tesseract + AWS Textract worked recipes)

12 items remain in phase-11-5-backlog.md — LSH banding, streaming reader, token-aware chunk sizing, PII severity tiers, etc.

Other commits in this PR

• 47232fc — post-v0.4.5 release sync (roadmap / phase-12-quickstart / releases reflect PyPI publish; bump to 0.5.0rc1)
• ed81e09 — README: PyPI badge + all 9 notebooks listed (was 5/9)

Test plan

• ruff format . + ruff check . — clean
• pytest tests/ — 718 passed, 11 skipped (8 pre-existing TRL/torch FSDP skips, 3 PDF roundtrip skips when pypdf not installed)
• Manual smoke: forgelm ingest TXT_DIR + forgelm --data-audit JSONL end-to-end
• Progress logging verified on 6000-row audit (split open + simhash progress + near-dup start lines fire at expected intervals)
• Phase 11 nightly CI smoke added (TXT-only, no extras required)
• CLI --help regression — every subcommand still parses
• Reviewer: spot-check forgelm ingest --pii-mask on a sample with mixed PII; verify counts in IngestionResult.pii_redaction_counts
• Reviewer: confirm pip install -e ".[ingestion]" in a fresh venv pulls all 5 deps (pypdf, python-docx, ebooklib, beautifulsoup4, langdetect)

Notes for reviewer

• This PR is large because Phase 11 is the next planned phase post-v0.4.5; everything is additive (no breaking changes to existing trainers, configs, or CLI surface).
• 0.5.0rc1 is set in pyproject.toml so a release-engineering pass after merge can drop rc1 and tag.
• The 14 deferred review items are explicit choices — see the backlog file for the rationale per item.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Document ingestion CLI: recursive multi-format → SFT-ready JSONL with paragraph/sliding chunking and optional PII masking.
  • Dataset audit CLI: produces data_audit_report.json with per-split quality metrics, language detection, simhash near-duplicate and cross-split leakage reports.
  • Audit findings auto-inlined into governance/export bundles.

• Documentation

  • New ingestion and data-audit guides, quickstart updates, runnable notebooks, and translated guides.

• Tests

  • Comprehensive ingestion and audit test suites.

• Chores

  • Version bumped to 0.5.0rc1 and new ingestion optional-deps extra.

• CI

  • Nightly end-to-end ingest+audit smoke check.

[sourcery-ai]

Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 57 minutes and 28 seconds.

⌛ 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: 7070de3d-f09e-4777-ba10-ac18849f329c

📥 Commits

Reviewing files that changed from the base of the PR and between ebfcd3d and 13c3bdb.

📒 Files selected for processing (4)

• docs/guides/data_audit.md
• docs/guides/ingestion.md
• forgelm/ingestion.py
• tests/test_ingestion.py

📝 Walkthrough

Walkthrough

This PR delivers Phase 11 "Document Ingestion & Data Audit" for v0.5.0: adds forgelm ingest to convert PDF/DOCX/EPUB/TXT/Markdown into SFT-ready JSONL with paragraph/sliding chunking and optional PII redaction, and adds forgelm --data-audit to emit dataset quality/governance reports (language, simhash dedup, cross-split leakage, PII counts) with trainer inlining support for EU AI Act Article 10.

Changes

Cohort / File(s)	Summary
Core Ingestion Module forgelm/ingestion.py	New module implementing multi-format extraction (.pdf, .docx, .epub, .txt, .md), paragraph and sliding-window chunking, deterministic recursive traversal, optional ingest-time PII masking, JSONL {"text": ...} output, IngestionResult dataclass and summarizers.
Core Data Audit Module forgelm/data_audit.py	New audit pipeline exposing AuditReport and audit_dataset() for single-file or split-directory inputs; computes per-split text stats, schema-union drift, optional language detection, regex+validator PII detection (Luhn/TR ID checks), 64-bit simhash fingerprinting and near-duplicate Hamming comparisons, cross-split leakage metrics, operator notes, and can write data_audit_report.json.
CLI Integration forgelm/cli.py	Adds ingest subcommand (chunk controls, --overlap, --strategy, --pii-mask, --recursive) and top-level --data-audit plus --output; dispatches to ingest_path/audit_dataset, supports human/JSON summary output, and maps common failures to exit codes.
Compliance Integration forgelm/compliance.py	generate_data_governance_report() refactored to inline training.output_dir/data_audit_report.json into the governance report under data_audit when present; read/parse errors warn and do not abort generation.
Wizard Directory Handling forgelm/wizard.py	_validate_local_jsonl now detects directory inputs, returns None to re-prompt, and emits a forgelm ingest --recursive hint.
Package Configuration forgelm/__init__.py, pyproject.toml	Version bumped to 0.5.0rc1; new [project.optional-dependencies].ingestion extra added for document parsing dependencies.
Tests — Ingestion tests/test_ingestion.py	New comprehensive tests for supported formats, chunking strategies (sliding/paragraph), JSONL output schema, PII masking, recursion behavior, unsupported-extension skipping, and optional extractor cases guarded by dependency presence.
Tests — Data Audit tests/test_data_audit.py	New comprehensive audit tests covering PII detection/masking (email, CC Luhn, TR ID, SSN rules), simhash determinism and Hamming thresholds, split discovery/aliasing, near-duplicate and cross-split leakage detection, schema drift tolerance, malformed JSONL resilience, and report serialization.
Tests — Wizard tests/test_wizard_byod.py	Updates directory-rejection assertion to expect an explicit directory message and a forgelm ingest hint.
Compliance & Tests Integration tests/test_compliance.py	Adds tests verifying governance inlining behavior for present/malformed/missing data_audit_report.json.
CI Workflow .github/workflows/nightly.yml	Adds nightly workflow step performing a minimal end-to-end ingest → audit flow and asserting non-empty outputs.
Docs — Guides & Usage docs/guides/ingestion.md, docs/guides/data_audit.md, docs/reference/usage.md, docs/guides/quickstart.md	Adds ingestion and audit guides and usage docs: CLI examples, chunking semantics, PII masking behavior, audit metrics/report structure, troubleshooting, end-to-end ingest→audit examples, and quickstart updates.
Docs — Roadmap / Changelog / README / Architecture CHANGELOG.md, README.md, docs/roadmap*.md, docs/standards/architecture.md	Updates changelog and README for v0.5.0 Phase 11; marks Phase 11 complete in roadmap, adds Phase 11.5 backlog, updates architecture docs to list new modules and new optional extras.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI as forgelm ingest<br/>(CLI)
    participant Extractor as Format Extractor<br/>(PDF/DOCX/EPUB/TXT)
    participant Chunker as Chunker<br/>(paragraph/sliding)
    participant PII as PII Masker
    participant Writer as JSONL Writer
    participant Output as /tmp/out.jsonl

    User->>CLI: forgelm ingest input/ --output /tmp/out.jsonl
    CLI->>Extractor: enumerate files (recursive?)
    Extractor->>Extractor: parse per-format text
    Extractor->>Chunker: send extracted text
    Chunker->>Chunker: split according to strategy
    Chunker->>PII: deliver chunks
    alt --pii-mask enabled
        PII->>PII: detect & redact sensitive matches
    end
    PII->>Writer: write {"text": "..."} lines
    Writer->>Output: persist JSONL
    Output-->>CLI: return IngestionResult
    CLI-->>User: summary + next-step hint

Loading

sequenceDiagram
    participant User
    participant CLI as forgelm --data-audit<br/>(CLI)
    participant Resolver as Split Resolver
    participant Reader as JSONL Reader
    participant Metrics as Metrics Engine
    participant Overlap as Cross-Split Detector
    participant Report as Report Writer
    participant OutputDir as output_dir/

    User->>CLI: forgelm --data-audit --input dataset.jsonl --output output_dir
    CLI->>Resolver: map input -> splits (train/validation/test)
    Resolver->>Reader: read JSONL lines per split
    Reader->>Metrics: extract text fields & compute stats
    Metrics->>Metrics: detect PII, compute simhash, near-duplicates, language
    Metrics->>Overlap: compare fingerprints across splits
    Overlap->>Report: record leakage & leaked counts
    Report->>OutputDir: write data_audit_report.json
    OutputDir-->>CLI: return AuditReport summary & path
    CLI-->>User: summary + report path

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 I nibble pages, parse each line,

Chunk by chunk the texts align,
Secrets masked and echoes found,
Audit JSONs stored and bound,
A hop from docs to training shine! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.95% 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 title clearly identifies the main change: landing Phase 11 (Document Ingestion & Data Audit) and syncing post-v0.4.5 changes for v0.5.0 release.
Description check	✅ Passed	The description thoroughly covers the Summary, detailed Changes with multiple subsections, Type selection (New feature + Documentation), comprehensive Testing checklist, and project Checklist items.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 415 complexity · 2 duplication

Metric	Results
Complexity	415
Duplication	2

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]

Code Review

This pull request implements Phase 11 of the roadmap, introducing a document ingestion pipeline and a dataset audit tool. The ingestion module supports PDF, DOCX, EPUB, TXT, and Markdown formats with paragraph and sliding window chunking strategies, including optional PII masking. The audit tool provides metrics on sample counts, length distributions, language detection, near-duplicate detection via simhash, and cross-split leakage checks. Feedback focused on performance bottlenecks in the auditing logic due to quadratic complexity in duplicate detection and a discrepancy in DOCX extraction where table content was being ignored despite documentation claims.

[gemini-code-assist]

The find_near_duplicates function implements a nested loop with O(n^2) complexity. For the documented target scale of ~50,000 rows, this results in approximately 1.25 billion iterations. In pure Python, this will be extremely slow and may take several minutes or even hours to complete. Additionally, if the dataset contains many near-duplicates, the pairs list can grow very large, potentially leading to high memory consumption (e.g., 10k identical rows would generate ~50 million tuples). Since the audit report only requires the count of near-duplicate pairs, consider using a more efficient approach like LSH-based bucketing (pigeonhole principle with blocks) and only returning the count to save memory.

[gemini-code-assist]

This cross-split overlap check also has O(N_a * N_b) complexity. If both splits are large (e.g., 50k rows each), this will perform up to 2.5 billion Hamming distance calculations in pure Python, which is likely to be a significant performance bottleneck for the CLI tool.

[gemini-code-assist]

The _extract_docx function currently only extracts text from doc.paragraphs, which excludes content within tables. This contradicts the documentation in docs/guides/ingestion.md (line 183) which states that DOCX tables are flattened to plain text. To align with the documentation and ensure comprehensive data ingestion, table content should be extracted as well.

def _extract_docx(path: Path) -> str:
    try:
        from docx import Document
    except ImportError as exc:  # pragma: no cover
        raise ImportError(
            "DOCX ingestion requires the 'ingestion' extra. Install with: pip install 'forgelm[ingestion]'"
        ) from exc

    doc = Document(str(path))
    text_parts = [p.text for p in doc.paragraphs if p.text and p.text.strip()]
    # Also extract text from tables to align with documentation
    for table in doc.tables:
        for row in table.rows:
            for cell in row.cells:
                if cell.text and cell.text.strip():
                    text_parts.append(cell.text)
    return "\n\n".join(text_parts)

[coderabbitai]

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

docs/standards/architecture.md (1)

71-71: ⚠️ Potential issue | 🟡 Minor

Stale module count.

The "Seventeen single-file modules" claim is now off — the table at lines 79-96 has 18 rows after ingestion.py and data_audit.py were added (and arguably inference.py from Phase 10 is also unaccounted for). Bump the prose so it stays in step with the topology table.

📝 Suggested fix

-Seventeen single-file modules. **No sub-packages inside `forgelm/`.** If a module grows past ~1000 lines and has cohesive subsections, split into `module_name/` package, but keep the public API at `forgelm.module_name.X` so imports don't break.
+Eighteen single-file modules. **No sub-packages inside `forgelm/`.** If a module grows past ~1000 lines and has cohesive subsections, split into `module_name/` package, but keep the public API at `forgelm.module_name.X` so imports don't break.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/standards/architecture.md` at line 71, Update the stale sentence
"Seventeen single-file modules." to accurately reflect the current module count
(at least 18) or, better, remove the hard-coded number and replace it with a
dynamic phrasing like "multiple single-file modules" or "N single-file modules"
that is maintained alongside the topology table; change the exact text token
"Seventeen single-file modules." in the architecture prose so it matches the
modules listed (including ingestion.py, data_audit.py, and inference.py) and
ensure the guidance about splitting modules into module_name/ packages (and
keeping public API at forgelm.module_name.X) remains unchanged.

README.md (1)

241-241: ⚠️ Potential issue | 🟡 Minor

Stale test count in Project Structure block.

The PR description states 718 passed, 11 skipped, but this line still advertises # 430 passed (+34 skipped) across 30 test files. Either drop the count (it goes stale every PR) or update it to match v0.5.0rc1.

📝 Proposed fix

-tests/               # 430 passed (+34 skipped) across 30 test files
+tests/               # Unit + integration suite (run `pytest tests/`)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 241, The "tests/" line in README.md contains a stale test
summary ("# 430 passed (+34 skipped) across 30 test files") that doesn't match
the PR/test run (718 passed, 11 skipped) — update or remove this count to avoid
future rot: either replace the existing summary text next to the "tests/" entry
with the current v0.5.0rc1 results (718 passed, 11 skipped) or remove the
numeric summary entirely so the "tests/" bullet remains without hard-coded
counts; edit the line that begins with "tests/" in README.md to apply the
change.

♻️ Duplicate comments (1)

docs/roadmap-tr.md (1)

20-20: ⚠️ Potential issue | 🟡 Minor

Same v0.4.0 date drift as roadmap.md.

CHANGELOG.md line 105 has v0.4.0 = 2026-04-25, not 2026-04-26. Fix here to keep the EN/TR mirror in sync once docs/roadmap.md is corrected.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/roadmap-tr.md` at line 20, The v0.4.0 entry in docs/roadmap-tr.md
currently shows the date 2026-04-26 which drifts from CHANGELOG.md and the EN
roadmap; update the date string for the `v0.4.0` entry to 2026-04-25 so the
Turkish mirror (`v0.4.0` — Post-Training Tamamlama) matches the canonical
sources.

🧹 Nitpick comments (13)

forgelm/compliance.py (1)

139-151: Article 10 inlining is well-scoped.

The defensive getattr chain, file existence check, and explicit catch of JSONDecodeError/OSError keep this best-effort and non-fatal. One small consideration: a corrupt UTF-8 file would raise UnicodeDecodeError (not in the catch tuple) and bubble up. If you want full robustness, broaden to (json.JSONDecodeError, OSError, UnicodeDecodeError) so a damaged audit artifact never crashes the governance report.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/compliance.py` around lines 139 - 151, The current inlining of
data_audit_report.json uses a try/except that catches json.JSONDecodeError and
OSError but will still raise on a corrupt UTF-8 file (UnicodeDecodeError);
update the exception tuple in the except block that surrounds the open+json.load
(the block that assigns report["data_audit"]) to include UnicodeDecodeError so
the handler (logger.warning(..., audit_path, exc)) swallows encoding errors as
well and keeps the process non‑fatal for output_dir/audit_path.

forgelm/cli.py (2)

1106-1117: Exit-code choice for missing extras differs from sibling commands.

_run_chat_cmd (line 789–791) maps ImportError to EXIT_TRAINING_ERROR (2), but here the missing-extras case maps to EXIT_CONFIG_ERROR (1). Both are valid public-contract codes, but the inconsistency makes it harder for CI scripts to branch on "missing optional dep". Pick one convention across subcommands and document it in docs/standards/error-handling.md if it isn't already.

As per coding guidelines: "Exit codes are a public contract: 0/1/2/3/4 as per error-handling.md, especially for CLI".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/cli.py` around lines 1106 - 1117, The ImportError handler in the
ingest command currently exits with EXIT_CONFIG_ERROR while _run_chat_cmd maps
ImportError to EXIT_TRAINING_ERROR; make the exit-code convention consistent by
changing the ImportError except block (the "except ImportError as exc:" handler
that uses output_format and logger) to exit with EXIT_TRAINING_ERROR instead of
EXIT_CONFIG_ERROR, and update docs/standards/error-handling.md to document that
missing optional dependency (ImportError) should use EXIT_TRAINING_ERROR across
subcommands.

---

1139-1153: _run_data_audit only catches FileNotFoundError.

audit_dataset calls out_dir.mkdir(...) and open(out_path, "w", ...), both of which can raise OSError (read-only FS, permission denied, ENOSPC). _resolve_input and _read_jsonl_split can also surface non-FileNotFoundError OSError cases. Today those will produce a Python traceback instead of a clean JSON/text error envelope.

♻️ Suggested broaden

-    try:
-        report = audit_dataset(audit_input, output_dir=target)
-    except FileNotFoundError as exc:
+    try:
+        report = audit_dataset(audit_input, output_dir=target)
+    except (FileNotFoundError, OSError, ValueError) as exc:
         if output_format == "json":
             print(json.dumps({"success": False, "error": str(exc)}))
         else:
             logger.error("Audit failed: %s", exc)
         sys.exit(EXIT_CONFIG_ERROR)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/cli.py` around lines 1139 - 1153, The current _run_data_audit
function only catches FileNotFoundError from audit_dataset, leaving other
filesystem-related OSError subclasses (permission errors, ENOSPC, etc.)
unhandled; update the except clause to catch OSError (or add an additional
except OSError as exc) so any filesystem/IO errors from audit_dataset,
_resolve_input, or _read_jsonl_split are handled the same way: when
output_format == "json" print a JSON error envelope with the stringified exc,
otherwise call logger.error("Audit failed: %s", exc), and then
sys.exit(EXIT_CONFIG_ERROR) — reference _run_data_audit, audit_dataset,
output_format, logger, json and EXIT_CONFIG_ERROR to locate the change.

forgelm/data_audit.py (4)

100-100: Use a greedy quantifier for the credit-card pattern.

\b(?:\d[ -]*?){13,19}\b works (regex backtracking will eventually consume separators when the surrounding \b forces it), but the reluctant *? adds avoidable backtracking on long digit-rich text and is what SonarCloud is flagging. A greedy * is functionally equivalent here and faster.

♻️ Proposed change

-    "credit_card": re.compile(r"\b(?:\d[ -]*?){13,19}\b"),
+    "credit_card": re.compile(r"\b(?:\d[ -]*){13,19}\b"),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` at line 100, The credit card regex under "credit_card"
uses a reluctant quantifier (\d[ -]*?) which causes unnecessary backtracking;
update the pattern in forgelm/data_audit.py to use a greedy quantifier (\d[ -]*)
so the entry "credit_card" becomes \b(?:\d[ -]*){13,19}\b (keep the surrounding
word boundaries and same repetition range) to improve performance while
preserving behavior.

---

459-514: _resolve_input complexity warning is fixable by extracting the directory branch.

SonarCloud flags 19 vs. 15. Splitting the directory-scanning logic into _resolve_directory_splits(src) -> (layouts, notes) (returning ({}, [...]) when no canonical files match so the pseudo-split fallback can take over) would cut the nesting and let each branch be tested in isolation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` around lines 459 - 514, The _resolve_input function is
too complex; extract the directory-scanning branch into a new helper
_resolve_directory_splits(src) that encapsulates the logic currently inside the
if src.is_dir() block (use _SPLIT_ALIASES, discovery of canonical/alias files,
building layouts, notes, the pseudo-split fallback and logger.warning), and have
_resolve_input call this helper and return its (layouts, notes) result or
continue to raise FileNotFoundError for missing input; this reduces nesting and
allows unit testing of directory logic in _resolve_directory_splits separately.

---

435-438: Mixed .format() inside a dict literal is easy to misread.

"leaked_rows_in_{}".format(a) works but the surrounding code uses f-strings everywhere; this is the only positional .format call in the file. Also, the if fp_a else 0.0 branch on line 437 is unreachable — the function continues on if not fp_a at line 428.

♻️ Proposed cleanup

             report["pairs"][f"{a}__{b}"] = {
-                "leaked_rows_in_{}".format(a): shared,
-                "leak_rate": round(shared / len(fp_a), 4) if fp_a else 0.0,
+                f"leaked_rows_in_{a}": shared,
+                "leak_rate": round(shared / len(fp_a), 4),
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` around lines 435 - 438, The dict entry creation in
report["pairs"][f"{a}__{b}"] mixes a .format() call with f-strings and includes
an unreachable ternary because fp_a is already guarded by an earlier continue;
change the key name to use an f-string (e.g., f"leaked_rows_in_{a}") for
consistency and remove the unnecessary "if fp_a else 0.0" branch so leak_rate is
computed as round(shared / len(fp_a), 4) (relying on the prior if not fp_a:
continue), updating the code that builds report["pairs"][f"{a}__{b}"]
accordingly.

---

352-415: _audit_split is doing five things; pulling out the per-feature passes would tame the complexity.

SonarCloud flags cognitive complexity 25 vs. 15 budget. The function currently mixes schema/drift, text payload extraction, length stats, language sampling, simhash fingerprinting, and PII counting. Each pass is independent and easy to extract:

• _compute_schema(rows) -> (columns, drift_columns)
• _compute_payloads(rows) -> (payloads, null_or_empty_count)
• _compute_languages(payloads, sample_size=200)
• _compute_fingerprints(split_name, payloads)
• _compute_pii(payloads)

That keeps the public per-split orchestrator readable and gives each helper an independently testable surface.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` around lines 352 - 415, _audit_split currently exceeds
cognitive complexity by mixing schema/drift discovery, payload extraction,
length stats, language sampling, simhashing and PII counting into one large
function; refactor by extracting those independent passes into small helpers and
call them from _audit_split: implement _compute_schema(rows) -> (columns,
drift_columns) to build seen_columns and detect drift vs base_columns,
_compute_payloads(rows) -> (text_payloads, null_or_empty) to run
_extract_text_payload and compute null counts, _compute_languages(text_payloads,
sample_size=200) to sample and return top language counts,
_compute_fingerprints(split_name, text_payloads) to compute simhashes with
progress logging and return fingerprints and simhash distinct count, and
_compute_pii(text_payloads) to aggregate detect_pii results; update _audit_split
to orchestrate these helpers and assemble info, fingerprints and
pii_counts_split accordingly so behavior and returned tuple remain unchanged.

forgelm/ingestion.py (1)

291-414: Consider extracting the per-file processing loop.

SonarCloud flags cognitive complexity 30 (vs. 15 budget) for ingest_path. The body is dominated by the per-file loop (extract → handle errors → chunk → optional PII mask → write). Pulling that into a _process_file(...) helper that returns chunk-count / chars / pii-counts would bring the public function back under budget and make the test surface narrower.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/ingestion.py` around lines 291 - 414, ingest_path is over-complex due
to a large per-file loop; extract that logic into a new helper (e.g.
_process_file) that takes (fpath, strategy, chunk_size, overlap, mask_pii,
detect_pii, out_fh) and returns a small result struct/tuple (chunks_written,
chars_written, per_file_pii_counts, skipped_flag, format_ext) so the main
ingest_path just iterates files, calls _select_extractor then _process_file, and
accumulates chunk_count, total_chars, format_counts and pii_redaction_counts;
move error handling for extractor calls, empty-text checks, chunking via
_strategy_dispatch, optional PII detect_pii/mask_pii logic, and the
out_fh.write(json.dumps(...)) into _process_file to reduce ingest_path cognitive
complexity while preserving existing symbols (ingest_path, _select_extractor,
_strategy_dispatch, detect_pii, mask_pii, IngestionResult).

pyproject.toml (1)

86-92: Pin upper bounds on the new ingestion extras.

The other optional groups consistently cap major versions (e.g. bitsandbytes>=0.42.0,<1.0.0, lm-eval>=0.4.4,<1.0.0). The new group uses open-ended >= only, which means a future pypdf 6.x or python-docx 2.x with breaking API changes would be silently pulled in by pip install 'forgelm[ingestion]' and break the format extractors at runtime. (pypdf 6.x is already publicly available as of April 2026.)

♻️ Proposed bounds

 ingestion = [
-  "pypdf>=4.0.0",
-  "python-docx>=1.0.0",
-  "ebooklib>=0.18",
-  "beautifulsoup4>=4.12.0",
-  "langdetect>=1.0.9",
+  "pypdf>=4.0.0,<6.0.0",
+  "python-docx>=1.0.0,<2.0.0",
+  "ebooklib>=0.18,<1.0.0",
+  "beautifulsoup4>=4.12.0,<5.0.0",
+  "langdetect>=1.0.9,<2.0.0",
 ]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pyproject.toml` around lines 86 - 92, The ingestion extras in pyproject.toml
currently use open-ended >= pins for pypdf, python-docx, ebooklib,
beautifulsoup4, and langdetect; update the ingestion array to add conservative
upper bounds that cap the next major (e.g., pypdf>=4.0.0,<6.0.0,
python-docx>=1.0.0,<2.0.0, ebooklib>=0.18,<1.0.0, beautifulsoup4>=4.12.0,<5.0.0,
langdetect>=1.0.9,<2.0.0) so future breaking major releases won’t be pulled
in—modify the ingestion extras entry in pyproject.toml accordingly.

docs/roadmap/phase-11-data-ingestion.md (1)

3-3: Status banner is fine but the static-analysis hint is worth a glance.

markdownlint flags MD028 (blank line inside the blockquote that follows on line 5) and MD001 (heading jumps from # to ### Tasks: at line 13). Neither blocks anything, but if you're already touching the file you can collapse the blank line and demote ### Tasks: to ## Tasks: to keep the linter quiet.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/roadmap/phase-11-data-ingestion.md` at line 3, The markdown linter
warnings come from a blank line inside the blockquote after the Status banner
and an incorrectly leveled heading; remove the empty line inside the blockquote
(collapse the blockquote so there is no blank line after the Status line) and
change the heading "### Tasks:" to "## Tasks:" so the heading levels do not jump
and MD028/MD001 are resolved.

tests/test_data_audit.py (3)

76-78: Silence SonarCloud type-arg warnings consistently with line 123.

These two calls intentionally exercise the non-string guard in detect_pii, but SonarCloud flags them (failures at L77 and L78). Line 123 already uses # type: ignore[arg-type] for the same scenario on mask_pii — apply the same suppression here for consistency and a clean CI run.

♻️ Proposed change

     def test_returns_empty_for_non_string(self):
-        assert detect_pii(None) == {}
-        assert detect_pii(42) == {}
+        assert detect_pii(None) == {}  # type: ignore[arg-type]
+        assert detect_pii(42) == {}  # type: ignore[arg-type]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_data_audit.py` around lines 76 - 78, Add the same SonarCloud
type-arg suppression used for mask_pii to these tests: in
test_returns_empty_for_non_string update the two assertions that call
detect_pii(None) and detect_pii(42) to append a "# type: ignore[arg-type]"
comment to each call so the non-string guard is still tested but static type
checkers/CI warnings are suppressed; locate the assertions inside the
test_returns_empty_for_non_string function around detect_pii and add the
comments there.

---

109-116: Drop the redundant local import of _is_credit_card.

_is_credit_card is already imported at the top of the file (line 17), so the inline from forgelm.data_audit import _is_credit_card here is dead. The explanatory docstring above is fine to keep.

♻️ Proposed change

     def test_luhn_helper_distinguishes_valid_from_invalid(self):
         # The masker may also redact long digit runs as candidate phone
         # numbers (false positives are intentional per the module docstring).
         # We assert at the helper level instead, which is unambiguous.
-        from forgelm.data_audit import _is_credit_card
-
         assert _is_credit_card("4111111111111111") is True
         assert _is_credit_card("4111111111111112") is False

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_data_audit.py` around lines 109 - 116, Remove the redundant inline
import in the test method test_luhn_helper_distinguishes_valid_from_invalid:
delete the line "from forgelm.data_audit import _is_credit_card" and rely on the
existing module-level import of _is_credit_card; leave the surrounding docstring
and the two assert lines unchanged so the test still calls the helper
_is_credit_card("4111111111111111") and _is_credit_card("4111111111111112").

---

257-266: caplog.at_level("WARNING") context is set up but never asserted on.

The PR comment for Bug 26 says the operator must be warned about pseudo-splits, but this test only verifies the report.notes blob — nothing checks that a WARNING-level log record was actually emitted. Either assert on caplog.records (the stronger guarantee, since notes and logs can drift apart) or drop the caplog fixture entirely to keep the test honest.

♻️ Proposed change (assert the warning was logged)

-    def test_pseudo_split_fallback_warns(self, tmp_path, caplog):
+    def test_pseudo_split_fallback_warns(self, tmp_path, caplog):
         # Bug 26: when no canonical split files exist, every .jsonl becomes
         # its own pseudo-split BUT the operator must be warned that
         # cross-split leakage analysis is meaningless in that case.
         _write_jsonl(tmp_path / "alpha.jsonl", [{"text": "x"}])
         _write_jsonl(tmp_path / "beta.jsonl", [{"text": "y"}])
         with caplog.at_level("WARNING"):
             report = audit_dataset(str(tmp_path))
         assert "alpha" in report.splits and "beta" in report.splits
         assert any("pseudo-split" in n for n in report.notes)
+        assert any(
+            r.levelname == "WARNING" and "pseudo-split" in r.getMessage()
+            for r in caplog.records
+        )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_data_audit.py` around lines 257 - 266, The test
test_pseudo_split_fallback_warns sets up caplog but never asserts a log was
emitted; update the test to assert that a WARNING-level log record about
pseudo-splits was emitted when calling audit_dataset (e.g., inspect
caplog.records or caplog.text for a message containing "pseudo-split" or
similar) in addition to the existing report.notes checks, or remove the caplog
usage if you prefer not to assert logs; focus your change inside
test_pseudo_split_fallback_warns and reference caplog, audit_dataset, and
report.notes when adding the assertion.

🤖 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/guides/data_audit.md`:
- Line 58: The fenced code block starting before the JSON object that contains
"cross_split_overlap" is missing a language identifier; update the opening
triple-backtick fence to include "json" so markdownlint MD040 is satisfied
(i.e., change ``` to ```json immediately before the object that begins with
"cross_split_overlap"). Ensure the rest of the block remains unchanged and that
the closing ``` remains present.
- Around line 21-24: The documentation currently contradicts itself about what
`--output-format json` prints; reconcile both descriptions so they match: decide
whether `--output-format json` writes the full JSON report to stdout or only a
summary (while always writing the full `data_audit_report.json` to `--output`),
then update the earlier paragraph (mentions `--output`,
`data_audit_report.json`, and `--output-format json`) and the later Note (the
explicit Note that references `--output-format json`, top-level metrics + report
path, and `data_audit_report.json`) to state the chosen behavior consistently
and clearly for CI users.
- Around line 1-7: Create a Turkish mirror of the user-facing guide by adding a
new file named data_audit-tr.md that is a full translation of data_audit.md:
translate the title and body into Turkish but preserve and not translate
code/CLI examples such as `forgelm --data-audit` and artifact names like
`data_audit_report.json`, keep references to trainer concepts like `output_dir`,
and ensure frontmatter/metadata (if any) matches the original; finally, register
the new Turkish page in the docs navigation/sidebar so it appears alongside the
English guide.

In `@docs/guides/ingestion.md`:
- Around line 116-122: The docs currently state encrypted PDFs surface a
ValueError to the operator, but the actual behavior in PdfExtractor and tests
(see TestPdfExtractor::test_encrypted_pdf_raises_clear_error) is to catch the
failure per-file, log a warning, and append the file to files_skipped rather
than exiting; update the wording in docs/guides/ingestion.md to describe this
per-file handling (warning + files_skipped) and include the suggested
external-decrypt command as remediation for skipped files.
- Around line 1-10: Add a Turkish mirror of the user-facing guide by creating a
new file named docs/guides/ingestion-tr.md that is a bilingual mirror of
docs/guides/ingestion.md: copy the content and translate all user-facing text
into Turkish, preserve any Markdown structure, links (adjust localized link
targets if needed), headings, code blocks and metadata, and ensure the new file
matches the same frontmatter/formatting conventions as other -tr guides so it
appears alongside ingestion.md in the docs build.

In `@docs/reference/usage.md`:
- Around line 165-168: The closing trailer sentence "This uses the `synthetic`
config section to generate training data from a teacher model before training
begins. See the [Configuration Guide](configuration.md) for all synthetic data
options." is orphaned and must be moved so it immediately follows the Synthetic
Data Generation section (the paragraph that begins with the forgelm --config
example), not after the Document Ingestion / Dataset Audit blocks; relocate that
exact sentence to sit directly after the synthetic example/trailer in the
Synthetic Data Generation section (or hoist it up before inserting the Document
Ingestion and Dataset Audit sections) so the Document Ingestion (v0.5.0+) and
Dataset Audit paragraphs appear after that trailer and read correctly.
- Around line 130-166: Add Turkish translations of the two missing sections
titled "Document Ingestion (v0.5.0+)" and "Dataset Audit (v0.5.0+)" into the
Turkish mirror (usage-tr) so it matches the English usage reference; include the
same command examples (single file, recursive, sliding-window, PII mask)
translated text for descriptions, keep the version note "(v0.5.0+)", preserve
links to the corresponding guides (ingestion and data_audit) and the note about
the trainer inlining data_audit findings under the data_audit key so the
bilingual docs remain in sync.

In `@docs/roadmap.md`:
- Line 20: Update the date for the v0.4.0 roadmap entry where the string "v0.4.0
— Post-Training Completion (2026-04-26)" appears so it reads "v0.4.0 —
Post-Training Completion (2026-04-25)" to match CHANGELOG.md; make the identical
replacement in the translated roadmap entry that contains the same v0.4.0
string.

In `@docs/roadmap/phase-11-5-backlog.md`:
- Around line 11-12: The count in the table header (| # | Item | Why deferred |
Effort | Impact |) and the PR summary disagree: update the table rows or the PR
prose so they match; either add the two missing rows to the table to reach "14
deferred follow-ups" or change the PR summary to "12 deferred follow-ups", and
if two items indeed landed mid-cycle, list them under the "Recently landed"
section (mention their titles/IDs) so the document and PR description are
consistent.

In `@docs/roadmap/phase-11-data-ingestion.md`:
- Around line 15-16: Update the roadmap entry to match the shipped ingestion
output: replace the SFT-style {"messages":[...]} schema with the actual JSONL
record shape emitted by forgelm.ingestion (records of the form {"text":
"<chunk>"}), keep the same mention of supported input formats and chunking
strategies (sliding/paragraph/semantic), and note the v0.5.0 behavior and
optional dependency group (forgelm[ingestion]); also update related docs
(releases and reference/usage) to use the same {"text": ...} schema so loaders
aren’t written against a nonexistent messages format.

In `@forgelm/ingestion.py`:
- Around line 374-380: The current code increments pii_redaction_counts by
running detect_pii(payload) which independently applies all _PII_PATTERNS and
can double-count spans that match multiple patterns while mask_pii mutates the
text in a single pass, causing counts to drift; change mask_pii to perform the
single-pass redaction and also return a per-type count (e.g., return
(masked_text, counts_dict)), then replace the detect_pii(payload) usage in this
block with the counts returned from mask_pii so pii_redaction_counts is
incremented from the actual redactions, and update any call sites of mask_pii to
accept the new return shape (or alternatively make mask_pii accept a flag to
return counts) while preserving existing behavior for other callers.

In `@tests/test_ingestion.py`:
- Around line 257-274: Update the docs text in guides/ingestion.md so it matches
the actual behavior asserted by test_encrypted_pdf_raises_clear_error: instead
of saying a ValueError is raised, state that encrypted PDFs are logged as a
warning and skipped during batch ingestion (files_skipped increments) and ensure
the warning message recommends the external decrypt path to the operator;
reference the ingest_path flow and the test name to locate the behavior to
document.

---

Outside diff comments:
In `@docs/standards/architecture.md`:
- Line 71: Update the stale sentence "Seventeen single-file modules." to
accurately reflect the current module count (at least 18) or, better, remove the
hard-coded number and replace it with a dynamic phrasing like "multiple
single-file modules" or "N single-file modules" that is maintained alongside the
topology table; change the exact text token "Seventeen single-file modules." in
the architecture prose so it matches the modules listed (including ingestion.py,
data_audit.py, and inference.py) and ensure the guidance about splitting modules
into module_name/ packages (and keeping public API at forgelm.module_name.X)
remains unchanged.

In `@README.md`:
- Line 241: The "tests/" line in README.md contains a stale test summary ("# 430
passed (+34 skipped) across 30 test files") that doesn't match the PR/test run
(718 passed, 11 skipped) — update or remove this count to avoid future rot:
either replace the existing summary text next to the "tests/" entry with the
current v0.5.0rc1 results (718 passed, 11 skipped) or remove the numeric summary
entirely so the "tests/" bullet remains without hard-coded counts; edit the line
that begins with "tests/" in README.md to apply the change.

---

Duplicate comments:
In `@docs/roadmap-tr.md`:
- Line 20: The v0.4.0 entry in docs/roadmap-tr.md currently shows the date
2026-04-26 which drifts from CHANGELOG.md and the EN roadmap; update the date
string for the `v0.4.0` entry to 2026-04-25 so the Turkish mirror (`v0.4.0` —
Post-Training Tamamlama) matches the canonical sources.

---

Nitpick comments:
In `@docs/roadmap/phase-11-data-ingestion.md`:
- Line 3: The markdown linter warnings come from a blank line inside the
blockquote after the Status banner and an incorrectly leveled heading; remove
the empty line inside the blockquote (collapse the blockquote so there is no
blank line after the Status line) and change the heading "### Tasks:" to "##
Tasks:" so the heading levels do not jump and MD028/MD001 are resolved.

In `@forgelm/cli.py`:
- Around line 1106-1117: The ImportError handler in the ingest command currently
exits with EXIT_CONFIG_ERROR while _run_chat_cmd maps ImportError to
EXIT_TRAINING_ERROR; make the exit-code convention consistent by changing the
ImportError except block (the "except ImportError as exc:" handler that uses
output_format and logger) to exit with EXIT_TRAINING_ERROR instead of
EXIT_CONFIG_ERROR, and update docs/standards/error-handling.md to document that
missing optional dependency (ImportError) should use EXIT_TRAINING_ERROR across
subcommands.
- Around line 1139-1153: The current _run_data_audit function only catches
FileNotFoundError from audit_dataset, leaving other filesystem-related OSError
subclasses (permission errors, ENOSPC, etc.) unhandled; update the except clause
to catch OSError (or add an additional except OSError as exc) so any
filesystem/IO errors from audit_dataset, _resolve_input, or _read_jsonl_split
are handled the same way: when output_format == "json" print a JSON error
envelope with the stringified exc, otherwise call logger.error("Audit failed:
%s", exc), and then sys.exit(EXIT_CONFIG_ERROR) — reference _run_data_audit,
audit_dataset, output_format, logger, json and EXIT_CONFIG_ERROR to locate the
change.

In `@forgelm/compliance.py`:
- Around line 139-151: The current inlining of data_audit_report.json uses a
try/except that catches json.JSONDecodeError and OSError but will still raise on
a corrupt UTF-8 file (UnicodeDecodeError); update the exception tuple in the
except block that surrounds the open+json.load (the block that assigns
report["data_audit"]) to include UnicodeDecodeError so the handler
(logger.warning(..., audit_path, exc)) swallows encoding errors as well and
keeps the process non‑fatal for output_dir/audit_path.

In `@forgelm/data_audit.py`:
- Line 100: The credit card regex under "credit_card" uses a reluctant
quantifier (\d[ -]*?) which causes unnecessary backtracking; update the pattern
in forgelm/data_audit.py to use a greedy quantifier (\d[ -]*) so the entry
"credit_card" becomes \b(?:\d[ -]*){13,19}\b (keep the surrounding word
boundaries and same repetition range) to improve performance while preserving
behavior.
- Around line 459-514: The _resolve_input function is too complex; extract the
directory-scanning branch into a new helper _resolve_directory_splits(src) that
encapsulates the logic currently inside the if src.is_dir() block (use
_SPLIT_ALIASES, discovery of canonical/alias files, building layouts, notes, the
pseudo-split fallback and logger.warning), and have _resolve_input call this
helper and return its (layouts, notes) result or continue to raise
FileNotFoundError for missing input; this reduces nesting and allows unit
testing of directory logic in _resolve_directory_splits separately.
- Around line 435-438: The dict entry creation in report["pairs"][f"{a}__{b}"]
mixes a .format() call with f-strings and includes an unreachable ternary
because fp_a is already guarded by an earlier continue; change the key name to
use an f-string (e.g., f"leaked_rows_in_{a}") for consistency and remove the
unnecessary "if fp_a else 0.0" branch so leak_rate is computed as round(shared /
len(fp_a), 4) (relying on the prior if not fp_a: continue), updating the code
that builds report["pairs"][f"{a}__{b}"] accordingly.
- Around line 352-415: _audit_split currently exceeds cognitive complexity by
mixing schema/drift discovery, payload extraction, length stats, language
sampling, simhashing and PII counting into one large function; refactor by
extracting those independent passes into small helpers and call them from
_audit_split: implement _compute_schema(rows) -> (columns, drift_columns) to
build seen_columns and detect drift vs base_columns, _compute_payloads(rows) ->
(text_payloads, null_or_empty) to run _extract_text_payload and compute null
counts, _compute_languages(text_payloads, sample_size=200) to sample and return
top language counts, _compute_fingerprints(split_name, text_payloads) to compute
simhashes with progress logging and return fingerprints and simhash distinct
count, and _compute_pii(text_payloads) to aggregate detect_pii results; update
_audit_split to orchestrate these helpers and assemble info, fingerprints and
pii_counts_split accordingly so behavior and returned tuple remain unchanged.

In `@forgelm/ingestion.py`:
- Around line 291-414: ingest_path is over-complex due to a large per-file loop;
extract that logic into a new helper (e.g. _process_file) that takes (fpath,
strategy, chunk_size, overlap, mask_pii, detect_pii, out_fh) and returns a small
result struct/tuple (chunks_written, chars_written, per_file_pii_counts,
skipped_flag, format_ext) so the main ingest_path just iterates files, calls
_select_extractor then _process_file, and accumulates chunk_count, total_chars,
format_counts and pii_redaction_counts; move error handling for extractor calls,
empty-text checks, chunking via _strategy_dispatch, optional PII
detect_pii/mask_pii logic, and the out_fh.write(json.dumps(...)) into
_process_file to reduce ingest_path cognitive complexity while preserving
existing symbols (ingest_path, _select_extractor, _strategy_dispatch,
detect_pii, mask_pii, IngestionResult).

In `@pyproject.toml`:
- Around line 86-92: The ingestion extras in pyproject.toml currently use
open-ended >= pins for pypdf, python-docx, ebooklib, beautifulsoup4, and
langdetect; update the ingestion array to add conservative upper bounds that cap
the next major (e.g., pypdf>=4.0.0,<6.0.0, python-docx>=1.0.0,<2.0.0,
ebooklib>=0.18,<1.0.0, beautifulsoup4>=4.12.0,<5.0.0, langdetect>=1.0.9,<2.0.0)
so future breaking major releases won’t be pulled in—modify the ingestion extras
entry in pyproject.toml accordingly.

In `@tests/test_data_audit.py`:
- Around line 76-78: Add the same SonarCloud type-arg suppression used for
mask_pii to these tests: in test_returns_empty_for_non_string update the two
assertions that call detect_pii(None) and detect_pii(42) to append a "# type:
ignore[arg-type]" comment to each call so the non-string guard is still tested
but static type checkers/CI warnings are suppressed; locate the assertions
inside the test_returns_empty_for_non_string function around detect_pii and add
the comments there.
- Around line 109-116: Remove the redundant inline import in the test method
test_luhn_helper_distinguishes_valid_from_invalid: delete the line "from
forgelm.data_audit import _is_credit_card" and rely on the existing module-level
import of _is_credit_card; leave the surrounding docstring and the two assert
lines unchanged so the test still calls the helper
_is_credit_card("4111111111111111") and _is_credit_card("4111111111111112").
- Around line 257-266: The test test_pseudo_split_fallback_warns sets up caplog
but never asserts a log was emitted; update the test to assert that a
WARNING-level log record about pseudo-splits was emitted when calling
audit_dataset (e.g., inspect caplog.records or caplog.text for a message
containing "pseudo-split" or similar) in addition to the existing report.notes
checks, or remove the caplog usage if you prefer not to assert logs; focus your
change inside test_pseudo_split_fallback_warns and reference caplog,
audit_dataset, and report.notes when adding the assertion.

🪄 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: 51515584-5e3c-4707-8459-61905d832f97

📥 Commits

Reviewing files that changed from the base of the PR and between 45713dc and 0889fd6.

📒 Files selected for processing (25)

• .github/workflows/nightly.yml
• CHANGELOG.md
• README.md
• docs/guides/data_audit.md
• docs/guides/ingestion.md
• docs/guides/quickstart.md
• docs/qms/sop_data_management.md
• docs/reference/usage.md
• docs/roadmap-tr.md
• docs/roadmap.md
• docs/roadmap/phase-11-5-backlog.md
• docs/roadmap/phase-11-data-ingestion.md
• docs/roadmap/phase-12-quickstart.md
• docs/roadmap/releases.md
• docs/standards/architecture.md
• forgelm/__init__.py
• forgelm/cli.py
• forgelm/compliance.py
• forgelm/data_audit.py
• forgelm/ingestion.py
• forgelm/wizard.py
• pyproject.toml
• tests/test_data_audit.py
• tests/test_ingestion.py
• tests/test_wizard_byod.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Tag fenced block language for markdownlint MD040.

The fenced block at line 58 has no language identifier — markdownlint flags it. Use json to match the rest of the file.

📝 Proposed fix

-```
+```json
 {
   "cross_split_overlap": {

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 58-58: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/guides/data_audit.md` at line 58, The fenced code block starting before
the JSON object that contains "cross_split_overlap" is missing a language
identifier; update the opening triple-backtick fence to include "json" so
markdownlint MD040 is satisfied (i.e., change ``` to ```json immediately before
the object that begins with "cross_split_overlap"). Ensure the rest of the block
remains unchanged and that the closing ``` remains present.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -t f 'ingestion.*\.md$' docs/

Repository: cemililik/ForgeLM

Length of output: 126

---

🏁 Script executed:

#!/bin/bash
# List all markdown files in docs/guides/
echo "=== All markdown files in docs/guides/ ==="
fd -t f '\.md$' docs/guides/

echo ""
echo "=== Check for Turkish mirror patterns (-tr.md) ==="
fd -t f '\-tr\.md$' docs/guides/

echo ""
echo "=== Check entire docs directory for Turkish mirrors ==="
fd -t f '\-tr\.md$' docs/

Repository: cemililik/ForgeLM

Length of output: 668

---

Missing Turkish mirror for user-facing guide.

docs/guides/ingestion.md is missing a docs/guides/ingestion-tr.md companion. Per the project guideline, user-facing docs under docs/guides/ must ship as bilingual mirror documents.

Note: All files in docs/guides/ currently lack Turkish mirrors (not just this file), while docs/reference/ guides have implemented them. Ensure consistency across the documentation structure.

🧰 Tools

🪛 LanguageTool

[locale-violation] ~6-~6: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...forgelm --data-audit afterwards to surface > length-distribution / lang...

(AFTERWARDS_US)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/guides/ingestion.md` around lines 1 - 10, Add a Turkish mirror of the
user-facing guide by creating a new file named docs/guides/ingestion-tr.md that
is a bilingual mirror of docs/guides/ingestion.md: copy the content and
translate all user-facing text into Turkish, preserve any Markdown structure,
links (adjust localized link targets if needed), headings, code blocks and
metadata, and ensure the new file matches the same frontmatter/formatting
conventions as other -tr guides so it appears alongside ingestion.md in the docs
build.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Date for v0.4.0 disagrees with CHANGELOG.

This line says v0.4.0 — Post-Training Completion (2026-04-26), but CHANGELOG.md line 105 records v0.4.0 as 2026-04-25. v0.4.5 is the 2026-04-26 release. Same fix needed in docs/roadmap-tr.md line 20.

📝 Proposed fix

-**Previous:** `v0.4.0` — Post-Training Completion (2026-04-26). Inference primitives, interactive chat REPL, GGUF export, VRAM fit advisor, and deployment config generation.
+**Previous:** `v0.4.0` — Post-Training Completion (2026-04-25). Inference primitives, interactive chat REPL, GGUF export, VRAM fit advisor, and deployment config generation.

📝 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

	**Previous:** `v0.4.0` — Post-Training Completion (2026-04-26). Inference primitives, interactive chat REPL, GGUF export, VRAM fit advisor, and deployment config generation.
	**Previous:** `v0.4.0` — Post-Training Completion (2026-04-25). Inference primitives, interactive chat REPL, GGUF export, VRAM fit advisor, and deployment config generation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/roadmap.md` at line 20, Update the date for the v0.4.0 roadmap entry
where the string "v0.4.0 — Post-Training Completion (2026-04-26)" appears so it
reads "v0.4.0 — Post-Training Completion (2026-04-25)" to match CHANGELOG.md;
make the identical replacement in the translated roadmap entry that contains the
same v0.4.0 string.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

docs/guides/data_audit-tr.md (1)

61-70: ⚠️ Potential issue | 🟡 Minor

Same cross_split_overlap schema mismatch as the EN guide.

The TR mirror inherits the same incorrect example shape. Apply the matching fix here so the bilingual pair stays in sync (see the EN comment for the corrected JSON).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/guides/data_audit-tr.md` around lines 61 - 70, The TR guide's JSON
example for "cross_split_overlap" uses the incorrect schema for "pairs" — update
the snippet so it matches the corrected schema used in the EN guide: keep
"hamming_threshold" and the same semantic values but replace the current "pairs"
shape with the corrected structure from the EN guide (apply the exact corrected
JSON for "pairs" as in the EN comment) so "cross_split_overlap",
"hamming_threshold" and "pairs" are consistent between languages.

🧹 Nitpick comments (5)

forgelm/cli.py (1)

1151-1151: Tiny cleanup: FileNotFoundError is a subclass of OSError.

SonarCloud's hint here is valid — listing both in the except tuple is redundant since FileNotFoundError is already an OSError. Drop the subclass for clarity:

-    except (FileNotFoundError, OSError) as exc:
+    except OSError as exc:

Behavior is unchanged; the comment about PermissionError / ENOSPC / IsADirectoryError already covers this correctly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/cli.py` at line 1151, The except clause in the error handling block
currently catches both FileNotFoundError and OSError redundantly; update the
handler in the relevant try/except (the line reading "except (FileNotFoundError,
OSError) as exc") to only catch OSError (i.e., remove FileNotFoundError from the
tuple) so the behavior remains the same but the code is clearer; leave the
existing comment about PermissionError/ENOSPC/IsADirectoryError unchanged.

forgelm/data_audit.py (1)

521-545: matched_b is built but never read — drop it (or use it to halve the second pass).

Inside _cross_split_overlap, matched_b accumulates every fp_b fingerprint that triggered a hit on the first sweep, but leaked_in_b is then computed by an independent any(hamming_distance(...) <= threshold for other in fp_a_set) over fp_b. The accumulator is dead code — the inline comment even acknowledges "matched_b as a starting point but a single pass over b is needed", but nothing actually consumes it.

Also note: fp_a_set = set(fp_a) doesn't accelerate the inner loop because hamming_distance still requires iterating elements (no O(1) lookup), so building the set is pure overhead.

♻️ Suggested simplification

-            fp_a_set = set(fp_a)
-            fp_b_set = set(fp_b)
-            # Single pass over fp_a captures both directions:
-            # rows in `a` whose nearest neighbour in `b` is within threshold,
-            # and the per-row matched fingerprints lift contributes to the
-            # `b`-side count.
-            leaked_in_a = 0
-            matched_b: set = set()
-            for fp in fp_a:
-                for other in fp_b_set:
-                    if hamming_distance(fp, other) <= threshold:
-                        leaked_in_a += 1
-                        matched_b.add(other)
-                        break
-            # Now count distinct b-rows whose nearest neighbour in `a` is
-            # close. We have `matched_b` as a starting point but a single
-            # pass over `b` is needed to find rows that match any `a`-row,
-            # not just the first `a`-row that triggered a match.
-            leaked_in_b = sum(1 for fp in fp_b if any(hamming_distance(fp, other) <= threshold for other in fp_a_set))
+            leaked_in_a = sum(
+                1 for fp in fp_a if any(hamming_distance(fp, other) <= threshold for other in fp_b)
+            )
+            leaked_in_b = sum(
+                1 for fp in fp_b if any(hamming_distance(fp, other) <= threshold for other in fp_a)
+            )

This drops the unused accumulator and the no-op set(...) calls without changing the reported metric.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/data_audit.py` around lines 521 - 545, The matched_b accumulator and
the fp_a_set/fp_b_set set conversions in _cross_split_overlap are
dead/inefficient: remove matched_b and the set(...) allocations, compute
leaked_in_a by iterating fp_a and checking any(hamming_distance(fp, other) <=
threshold for other in fp_b) and compute leaked_in_b symmetrically by iterating
fp_b and checking against fp_a; keep using leaked_in_a, leaked_in_b, threshold,
hamming_distance and the existing report["pairs"][f"{a}__{b}"] population but
drop matched_b, fp_a_set and fp_b_set to simplify and avoid unnecessary work.

docs/guides/ingestion.md (1)

99-110: Tag the two unfenced code blocks for markdownlint MD040.

The directory-tree fence at line 99 and the CLI-usage fence at line 161 lack a language identifier — markdownlint flags both, and the rest of the file uses explicit fences (e.g. line 161 in data_audit-tr.md uses text).

📝 Proposed fixes

-```
+```text
 ./policies/
 ├── 2024_q1.pdf

-```
+```text
 forgelm ingest INPUT_PATH \
   --output FILE \

Also applies to: 159-171

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/guides/ingestion.md` around lines 99 - 110, Two code fences lack
language identifiers: the directory-tree block that begins with "./policies/"
and the CLI example that starts with "forgelm ingest ./policies/ --recursive
--output data/all_policies.jsonl"; add a language tag (use "text") to both
opening ``` fences so markdownlint MD040 is satisfied, and mirror the existing
style used elsewhere (e.g., the fenced example in data_audit-tr.md) to keep
consistency.

forgelm/ingestion.py (2)

308-434: Optional: extract the per-file processing block to tame cognitive complexity.

ingest_path packs file iteration, extractor selection, exception triage, chunking, PII masking, JSONL writing, and counters into one body — SonarCloud reports cognitive complexity 30. Pulling the inner loop into a small helper (e.g. _process_file(fpath, out_fh, …) returning chunks/chars/pii_counts) would let the entry point read top-down without changing behavior. Not blocking; flagging since the function is the primary touchpoint for future ingestion features (semantic chunking, additional formats).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/ingestion.py` around lines 308 - 434, The ingest_path function is too
cognitively complex; extract the inner per-file work into a helper like
_process_file(fpath, out_fh, strategy, chunk_size, overlap, mask_pii) that
performs extractor selection, exception handling, skipping logic, chunk
generation, PII masking and JSONL writes, and returns counts (chunks_written,
chars_written, file_processed_flag, file_skipped_flag, per_file_format,
per_file_pii_counts); then simplify ingest_path to iterate files, call
_process_file for each, and aggregate returned counters (chunk_count,
total_chars, files_processed/skipped, format_counts, pii_redaction_counts,
notes) so the high-level flow reads top-to-bottom without changing behavior or
side effects.

---

371-378: Confirmed: ImportError re-raise ordering is correct.

Worth a one-line comment here since ImportError is a subclass of Exception and a future reader could be tempted to consolidate the two except clauses. Keeping except ImportError: raise ahead of the broad except Exception is what guarantees missing optional extras surface loudly instead of being silently downgraded to "skipped file". A short comment ("ImportError must propagate — missing extras are not a per-file skip") would prevent regressions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/ingestion.py` around lines 371 - 378, Add a one-line comment above
the ImportError except branch in the try/except around extractor(fpath)
explaining that ImportError must propagate so missing optional extras surface
(i.e., "ImportError must propagate — missing extras are not a per-file skip"),
and keep the existing ordering with except ImportError: raise before the broader
except Exception to prevent swallowing optional-dependency errors.

🤖 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/guides/data_audit.md`:
- Around line 61-70: Update the example JSON for the "cross_split_overlap"
payload so each pair contains the four directional fields the tests expect:
leaked_rows_in_train, leaked_rows_in_test, leak_rate_train, and leak_rate_test
(instead of the single "leak_rate"); specifically replace the current pair
object under "pairs" for "train__test" with entries for both directions and
their respective leak counts/rates, and apply the same change to the TR mirror
of the guide.

In `@forgelm/cli.py`:
- Line 238: The ingest subparser help string and the top-level epilog need to
include Markdown (.md) to match SUPPORTED_EXTENSIONS; update the help= value
that currently reads "Convert raw documents (PDF / DOCX / EPUB / TXT) into
SFT-ready JSONL." to include "Markdown (.md)" (or ".md") and likewise edit the
epilog text so Markdown is listed among supported input types; reference the
existing SUPPORTED_EXTENSIONS constant to ensure wording exactly matches the
supported types.

---

Duplicate comments:
In `@docs/guides/data_audit-tr.md`:
- Around line 61-70: The TR guide's JSON example for "cross_split_overlap" uses
the incorrect schema for "pairs" — update the snippet so it matches the
corrected schema used in the EN guide: keep "hamming_threshold" and the same
semantic values but replace the current "pairs" shape with the corrected
structure from the EN guide (apply the exact corrected JSON for "pairs" as in
the EN comment) so "cross_split_overlap", "hamming_threshold" and "pairs" are
consistent between languages.

---

Nitpick comments:
In `@docs/guides/ingestion.md`:
- Around line 99-110: Two code fences lack language identifiers: the
directory-tree block that begins with "./policies/" and the CLI example that
starts with "forgelm ingest ./policies/ --recursive --output
data/all_policies.jsonl"; add a language tag (use "text") to both opening ```
fences so markdownlint MD040 is satisfied, and mirror the existing style used
elsewhere (e.g., the fenced example in data_audit-tr.md) to keep consistency.

In `@forgelm/cli.py`:
- Line 1151: The except clause in the error handling block currently catches
both FileNotFoundError and OSError redundantly; update the handler in the
relevant try/except (the line reading "except (FileNotFoundError, OSError) as
exc") to only catch OSError (i.e., remove FileNotFoundError from the tuple) so
the behavior remains the same but the code is clearer; leave the existing
comment about PermissionError/ENOSPC/IsADirectoryError unchanged.

In `@forgelm/data_audit.py`:
- Around line 521-545: The matched_b accumulator and the fp_a_set/fp_b_set set
conversions in _cross_split_overlap are dead/inefficient: remove matched_b and
the set(...) allocations, compute leaked_in_a by iterating fp_a and checking
any(hamming_distance(fp, other) <= threshold for other in fp_b) and compute
leaked_in_b symmetrically by iterating fp_b and checking against fp_a; keep
using leaked_in_a, leaked_in_b, threshold, hamming_distance and the existing
report["pairs"][f"{a}__{b}"] population but drop matched_b, fp_a_set and
fp_b_set to simplify and avoid unnecessary work.

In `@forgelm/ingestion.py`:
- Around line 308-434: The ingest_path function is too cognitively complex;
extract the inner per-file work into a helper like _process_file(fpath, out_fh,
strategy, chunk_size, overlap, mask_pii) that performs extractor selection,
exception handling, skipping logic, chunk generation, PII masking and JSONL
writes, and returns counts (chunks_written, chars_written, file_processed_flag,
file_skipped_flag, per_file_format, per_file_pii_counts); then simplify
ingest_path to iterate files, call _process_file for each, and aggregate
returned counters (chunk_count, total_chars, files_processed/skipped,
format_counts, pii_redaction_counts, notes) so the high-level flow reads
top-to-bottom without changing behavior or side effects.
- Around line 371-378: Add a one-line comment above the ImportError except
branch in the try/except around extractor(fpath) explaining that ImportError
must propagate so missing optional extras surface (i.e., "ImportError must
propagate — missing extras are not a per-file skip"), and keep the existing
ordering with except ImportError: raise before the broader except Exception to
prevent swallowing optional-dependency errors.

🪄 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: 2a49dab8-70b0-4cf1-9a6c-70adaa461cdd

📥 Commits

Reviewing files that changed from the base of the PR and between 0889fd6 and 6121c9a.

📒 Files selected for processing (19)

• 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/phase-11-5-backlog.md
• docs/roadmap/phase-11-data-ingestion.md
• docs/standards/architecture.md
• forgelm/cli.py
• forgelm/compliance.py
• forgelm/data_audit.py
• forgelm/ingestion.py
• pyproject.toml
• tests/test_compliance.py
• tests/test_data_audit.py
• tests/test_ingestion.py

✅ Files skipped from review due to trivial changes (3)

• docs/roadmap/phase-11-5-backlog.md
• docs/reference/usage.md
• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (3)

• docs/standards/architecture.md
• pyproject.toml
• docs/roadmap/phase-11-data-ingestion.md

[cemililik]

@coderabbitai review

[coderabbitai]

✅ 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]

Actionable comments posted: 2

🧹 Nitpick comments (2)

forgelm/ingestion.py (1)

386-393: Document the additional overlap <= chunk_size // 2 bound.

The docstring says only overlap must be < chunk_size, but _chunk_sliding also rejects overlap > chunk_size // 2 at lines 207-211 to prevent quadratic chunk explosion. Library users hitting that limit via ingest_path() get a ValueError that isn't reflected in the docstring contract.

📝 Proposed docstring update

         chunk_size: Soft size cap per chunk (characters).
-        overlap: Overlap window for the sliding strategy. Must be < ``chunk_size``.
+        overlap: Overlap window for the sliding strategy. Must satisfy
+            ``0 <= overlap <= chunk_size // 2`` (the upper half-cap prevents
+            pathological chunk explosion; e.g. ``overlap=199`` with
+            ``chunk_size=200`` would emit roughly one chunk per character).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@forgelm/ingestion.py` around lines 386 - 393, Update the ingest_path()
docstring to include the additional constraint enforced by _chunk_sliding:
besides overlap < chunk_size, document that overlap must also be <= chunk_size
// 2 to avoid quadratic chunk explosion; mention that violating this will raise
a ValueError from _chunk_sliding so callers should validate overlap accordingly
and include the rationale (prevent quadratic growth) and the exact bound
(overlap <= chunk_size // 2) alongside the existing description of overlap and
chunk_size.

tests/test_ingestion.py (1)

58-65: Stale comment — early-exit suppresses the 4th chunk.

The comment claims 4 chunks ("abcd", "cdef", "efgh", "gh") but _chunk_sliding's early-exit (lines 224-225 of ingestion.py) returns after "efgh" since start (4) + chunk_size (4) >= len(text) (8). The test still passes because only the first three are asserted, but the comment will mislead the next maintainer into expecting a runt "gh" tail.

📝 Proposed comment fix

         text = "abcdefgh"
         chunks = list(_chunk_sliding(text, chunk_size=4, overlap=2))
-        # step = 2 → "abcd", "cdef", "efgh", "gh"
+        # step = 2 → "abcd", "cdef", "efgh".
+        # The early-exit in _chunk_sliding suppresses the runt "gh" tail
+        # that would otherwise be a pure prefix of the prior overlap window.
         assert chunks[0] == "abcd"
         assert chunks[1] == "cdef"
         assert chunks[2] == "efgh"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_ingestion.py` around lines 58 - 65, The test comment incorrectly
states that _chunk_sliding will produce four chunks ("abcd", "cdef", "efgh",
"gh") even though _chunk_sliding early-exits and only yields three; update the
comment in test_overlap_preserves_context to reflect the actual output from
_chunk_sliding (e.g., step = 2 → "abcd", "cdef", "efgh") or alternatively change
the test inputs (chunk_size/overlap or text length) if you intend to assert a
final short tail, and reference the _chunk_sliding function when making this
change so the comment and assertions match the actual 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/guides/data_audit.md`:
- Around line 168-173: The CLI synopsis code fence in docs/guides/data_audit.md
is unlabeled (MD040); update the fenced block to use the text language tag so it
matches the Turkish mirror and satisfies markdownlint: replace the opening "```"
with "```text" for the forgelm --data-audit PATH block (the block currently
shown between lines 168–173) so the fence is explicitly labeled as text and
mirrors docs/guides/data_audit-tr.md.

In `@docs/guides/ingestion.md`:
- Around line 161-171: The CLI usage fenced block for the "forgelm ingest"
command is missing a Markdown language tag; update the code fence that contains
the forgelm ingest usage to use the text tag (i.e., ```text) so it matches the
labeled-fence convention used elsewhere (e.g., the data_audit-tr guide) and
resolves markdownlint MD040.

---

Nitpick comments:
In `@forgelm/ingestion.py`:
- Around line 386-393: Update the ingest_path() docstring to include the
additional constraint enforced by _chunk_sliding: besides overlap < chunk_size,
document that overlap must also be <= chunk_size // 2 to avoid quadratic chunk
explosion; mention that violating this will raise a ValueError from
_chunk_sliding so callers should validate overlap accordingly and include the
rationale (prevent quadratic growth) and the exact bound (overlap <= chunk_size
// 2) alongside the existing description of overlap and chunk_size.

In `@tests/test_ingestion.py`:
- Around line 58-65: The test comment incorrectly states that _chunk_sliding
will produce four chunks ("abcd", "cdef", "efgh", "gh") even though
_chunk_sliding early-exits and only yields three; update the comment in
test_overlap_preserves_context to reflect the actual output from _chunk_sliding
(e.g., step = 2 → "abcd", "cdef", "efgh") or alternatively change the test
inputs (chunk_size/overlap or text length) if you intend to assert a final short
tail, and reference the _chunk_sliding function when making this change so the
comment and assertions match the actual 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: b4cf26ce-aa65-463f-a853-33393d0dbb12

📥 Commits

Reviewing files that changed from the base of the PR and between 6121c9a and ebfcd3d.

📒 Files selected for processing (9)

• docs/guides/data_audit-tr.md
• docs/guides/data_audit.md
• docs/guides/ingestion.md
• forgelm/cli.py
• forgelm/compliance.py
• forgelm/data_audit.py
• forgelm/ingestion.py
• tests/test_data_audit.py
• tests/test_ingestion.py

🚧 Files skipped from review as they are similar to previous changes (1)

• forgelm/data_audit.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unlabeled fence — diverges from the Turkish mirror.

The CLI synopsis block at lines 168-173 has no language tag, which markdownlint flags (MD040). The Turkish mirror at docs/guides/data_audit-tr.md lines 170-175 already uses ```text for the same block, so there's a small bilingual divergence here. As per coding guidelines, "User-facing docs must be bilingual (English and Turkish) as mirror documents".

📝 Proposed fix

-```
+```text
 forgelm --data-audit PATH \
   [--output DIR] \
   [--output-format {text,json}] \
   [--quiet | --log-level {DEBUG,INFO,WARNING,ERROR}]

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/guides/data_audit.md around lines 168 - 173, The CLI synopsis code
fence in docs/guides/data_audit.md is unlabeled (MD040); update the fenced block
to use the text language tag so it matches the Turkish mirror and satisfies
markdownlint: replace the opening "" with "text" for the forgelm
--data-audit PATH block (the block currently shown between lines 168–173) so the
fence is explicitly labeled as text and mirrors docs/guides/data_audit-tr.md.

</details>

<!-- fingerprinting:phantom:medusa:nectarine:608628d8-10bc-4ea6-9d79-3a4d3c8888d0 -->

<!-- d98c2f50 -->

<!-- This is an auto-generated comment by CodeRabbit -->

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud