fix: use metadata parameter in ChunkingQualityAnalyzer (#65)

[himanshu231204]

Summary

Fixes #65 — ChunkingQualityAnalyzer.analyze() now uses the metadata parameter for more informed chunking quality analysis.

Changes

Core Fix (openagent_eval/diagnosis/chunking.py)

• Extract metadata values: chunk_size and chunk_overlap are now read from the metadata dict
• chunk_size_deviation check: Flags chunks that deviate >50% from expected size
• excessive_overlap check: Flags character overlap >3x the expected overlap
• Adaptive empty chunk threshold: Uses chunk_size / 10 when chunk_size is provided instead of the default 50 chars
• Added _char_overlap() helper: Computes suffix/prefix character overlap between texts

Tests ( ests/unit/test_diagnosis/test_chunking.py)

• 9 new tests in TestMetadataUsage covering all metadata scenarios
• 5 new tests in TestCharOverlap for the new helper method
• All backward compatible — metadata=None or {} works exactly as before

Test Results

29/29 chunking tests passed 76/76 full diagnosis test suite passed ruff check: clean

Metadata Keys Supported

Key	Type	Description
chunk_size	loat	Expected chunk size in characters
chunk_overlap	loat	Expected overlap between chunks in characters

Full Changelog: main...fix/65-chunking-metadata-unused

[github-actions]

Author identity unknown

*** Please tell me who you are.

Run

git config --global user.email "you@example.com"
git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: empty ident name (for runner@runnervm3jd5f.xariblbjf3ru1nzambwxmzlesd.gx.internal.cloudapp.net) not allowed

opencode session  |  github run

[github-actions]

Review of PR #65: fix: use metadata parameter in ChunkingQualityAnalyzer

---

Summary

Adds metadata dict support to ChunkingQualityAnalyzer.analyze() so that chunk_size and chunk_overlap values drive more informed analysis — chunk size deviation, excessive character overlap, and adaptive empty chunk thresholds. Also adds a _char_overlap() helper and 14 new tests.

---

Issues Found

[BUG] expected_overlap=0 causes false positives for ANY overlap

File: chunking.py:129 | Severity: HIGH

expected_overlap * 3 = 0 when expected_overlap is 0, so char_overlap > 0 flags even 1-2 character accidental matches. A user passing chunk_overlap=0 ("tolerate no overlap") will get noise from trivial substring overlaps.

Suggested fix: Guard the comparison:

if expected_overlap > 0 and char_overlap > expected_overlap * 3:

---

[BUG] _char_overlap checks both directions, not matching its docstring

File: chunking.py:331-335 | Severity: MEDIUM

The docstring says "longest overlapping suffix/prefix" but the implementation checks both A-suffix/B-prefix AND B-suffix/A-prefix. For ordered chunks, the reverse direction (B ends with start of A) is unusual and can produce unexpected matches. This is especially problematic since there's no documented order guarantee for the contexts list.

Suggested fix: Either update the docstring to

opencode session  |  github run

[github-actions]

Summary

This PR wires the metadata parameter in ChunkingQualityAnalyzer.analyze() to drive three new checks: chunk size deviation (against chunk_size), excessive character overlap (against chunk_overlap), and adaptive empty-chunk thresholds. Adds a _char_overlap() helper and 14 tests. Backward compatible.

---

Issues Found

[BUG] expected_overlap=0 causes false positives for any overlap

File: chunking.py:129 | Severity: HIGH

if char_overlap > expected_overlap * 3:

When expected_overlap is 0, the multiplier produces 0 * 3 = 0, so any char_overlap > 0 (even 1–2 accidental characters) triggers a false positive. A user explicitly setting chunk_overlap=0 ("tolerate zero overlap") gets noise.

Fix: Guard the comparison:

if expected_overlap > 0 and char_overlap > expected_overlap * 3:

---

[BUG] _char_overlap docstring doesn't match behavior

File: chunking.py:326-328 | Severity: MEDIUM

Docstring says "longest suffix of text_a that matches a prefix of text_b" but the implementation also checks the reverse (text_b.endswith(text_a[:length]) at line 334). For ordered contexts (i < j pairs), finding that chunk B ends with content from the start of chunk A is unusual and could produce surprising matches.

Fix: Either remove the reverse check (line 334) or update the docstring to describe the bi-directional behavior.

---

[BUG] Empty chunk detection disabled when chunk_size < 10

File: chunking.py:240-243 | Severity: MEDIUM

min_threshold = (
    expected_chunk_size // 10
    if expected_chunk_size is not None
    else DEFAULT_MIN_CHUNK_LENGTH
)

Integer division: chunk_size=5 → 5 // 10 = 0. Then len(stripped) < 0 is always False, silently disabling empty chunk detection.

Fix: Guard with a minimum:

min_threshold = max(expected_chunk_size // 10, 1) if expected_chunk_size is not None else DEFAULT_MIN_CHUNK_LENGTH

(Or use expected_chunk_size > 0 gate like in _check_size_consistency.)

---

[STYLE] Magic number 3 should be a named constant

File: chunking.py:129 | Severity: LOW

expected_overlap * 3 hardcodes the "excessive" multiplier. Extract to EXCESSIVE_OVERLAP_MULTIPLIER = 3 alongside the other module-level defaults.

---

[STYLE] Type annotation mismatch on metadata parameter

File: chunking.py:54 | Severity: LOW

metadata: dict[str, float] | None = None,

The values are typed as float, but immediately cast to int at lines 76/79. Either type as dict[str, Any] or accept int in the annotation.

---

[TEST] Weak assertion in test_metadata_chunk_size_multiple_chunks

File: test_chunking.py:149 | Severity: LOW

assert len(size_issues) > 0

Both chunks deviate from the expected size of 20 (deviation of 70% and 280% respectively), so exactly 2 should be found. A more precise assertion (assert len(size_issues) == 2) would catch regressions.

---

Positive Notes

• Backward compatibility done right — metadata=None and metadata={} are explicitly tested to produce identical results to the old path.
• Clean extraction pattern — metadata values are extracted once at the top of analyze() and threaded through as optional parameters, keeping method signatures explicit.
• Comprehensive test coverage — 14 new tests covering every metadata scenario including edge cases (identical texts, empty texts, single chunk, both keys simultaneously).
• Well-structured helper — _char_overlap() is a clean, static method with O(n·m) worst-case but correct early-return for the common case.
• Good docstrings throughout — Args, returns, and usage examples are clear and consistent with project conventions.

---

Verdict

REQUEST CHANGES — The expected_overlap=0 false positive bug (HIGH severity) must be fixed before merging. The docstring mismatch and magic number are advisory but should ideally be addressed in the same changeset.

opencode session  |  github run

[github-actions]

🎉 Congratulations @himanshu231204!

Your pull request has been successfully merged into main. 🚀

Thank you for contributing to OpenAgentHQ and helping improve the project.

We truly appreciate your contribution and hope to see you back with more amazing PRs!

Happy Open Sourcing! ❤️