From 6839fdcbb883806276fad5acdd586c9a7951ea7d Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Tue, 21 Jul 2026 23:30:43 -0700 Subject: [PATCH 01/10] chore: added journal.md and ensured app ran --- journal.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 journal.md diff --git a/journal.md b/journal.md new file mode 100644 index 000000000..199928ddc --- /dev/null +++ b/journal.md @@ -0,0 +1,16 @@ +## Week 7 — Issue selection + +**Issue link:** (https://github.com/ascherj/pathreview/issues/149) + +**Issue title:** Structural chunker silently drops documents that contain no headings + +**Tier:** [X] Tier 1 [ ] Tier 2 [ ] Tier 3 + +**Problem summary:** +StructuralChunker is not supposed to return an empty list for any document without a heading. It is essentially excluding the entire document from the RAG index instead of trying another strategy. A successful fix would have the chunker either return an error message, alerting the user of what is happening, or it should try another method for chunking. + +**Branch name:** fix/149-structural-chunker-silently-drops-documents + +**Setup confirmation:** [X] App runs locally at localhost:5173 + +**Cohort ledger:** [X] Issue added to cohort ledger From fda70ef0529f521fc5c0327fe77be2007465826b Mon Sep 17 00:00:00 2001 From: John Pham <92967370+JohnPhm@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:22:46 -0700 Subject: [PATCH 02/10] Fix: Rename journal.md to JOURNAL.md --- journal.md => JOURNAL.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename journal.md => JOURNAL.md (100%) diff --git a/journal.md b/JOURNAL.md similarity index 100% rename from journal.md rename to JOURNAL.md From 596ae9085d680d3e36ba29ee2898d902e6df1561 Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sun, 2 Aug 2026 16:25:44 -0700 Subject: [PATCH 03/10] doc: updated JOURNAL.md to include information on reproducing error --- JOURNAL.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/JOURNAL.md b/JOURNAL.md index 199928ddc..2459dd224 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -14,3 +14,17 @@ StructuralChunker is not supposed to return an empty list for any document witho **Setup confirmation:** [X] App runs locally at localhost:5173 **Cohort ledger:** [X] Issue added to cohort ledger + +## Week 8 — Reproduction & solution planning + +**Reproduction commit link:** [link to commit documenting the reproduced issue] + +**Reproduction summary:** +[1–2 sentences: How did you reproduce the issue? What did you observe?] + +**PLAN.md link:** [link to PLAN.md in your fork] + +**Walkthrough video (recommended):** [link to your Loom video, ≤2 min — recommended, not graded] + +**Blockers or open questions:** +[Anything you're still uncertain about going into Week 9, or leave blank] From c365e22c1a92590de989216cae2b763a77fe7b4a Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sun, 2 Aug 2026 16:37:43 -0700 Subject: [PATCH 04/10] doc: added issue reproduction documentation (JOURNAL.md) and solution planning (PLAN.md) --- JOURNAL.md | 5 +++-- PLAN.md | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 PLAN.md diff --git a/JOURNAL.md b/JOURNAL.md index 2459dd224..e09872469 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -20,11 +20,12 @@ StructuralChunker is not supposed to return an empty list for any document witho **Reproduction commit link:** [link to commit documenting the reproduced issue] **Reproduction summary:** -[1–2 sentences: How did you reproduce the issue? What did you observe?] +[1–2 sentences: How did you reproduce the issue? What did you observe?] +To reproduce the issue, I used one of the included unit tests within the pathreview project. Under the tests subfolder (pathreview/tests/unit/test_structural_chunker.py) and with the virtual environment activated, I ran the command "python3 -m pytest tests/unit/test_structural_chunker.py::TestStructuralChunker::test_document_with_no_headings -v", which specifically uses the test_document_with_no_headings function in the structural_chunker python file. Running the test function, it returns an AssertionError (assert 0>= 1). **PLAN.md link:** [link to PLAN.md in your fork] **Walkthrough video (recommended):** [link to your Loom video, ≤2 min — recommended, not graded] **Blockers or open questions:** -[Anything you're still uncertain about going into Week 9, or leave blank] +Going into week 9, I am still confused on how to write and modify unit tests to confirm that my fixes to the structural chunker works correctly. diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 000000000..80e3035c5 --- /dev/null +++ b/PLAN.md @@ -0,0 +1,24 @@ +## Solution plan + +**Issue:** Structural chunker silently drops documents that contain no headings #149 + +### Understand +What is the root cause of this issue? What behavior is expected vs. actual? +The root cause of this issue + +### Map +Which files, functions, or modules are involved? +List the specific files you expect to touch. + +### Plan +What are the steps to fix this issue? +Break it into 3–5 concrete sub-tasks. + +### Inputs & outputs +What does your fix take as input? What should it produce or change? + +### Risks & unknowns +What could go wrong? What are you still unsure about? + +### Edge cases +What inputs or states should your fix handle gracefully? \ No newline at end of file From 25c6573a87821d553ded675f2f662e74135b9a15 Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sun, 2 Aug 2026 21:38:14 -0700 Subject: [PATCH 05/10] doc: updated JOURNAL.md and PLAN.md to include a solution plan --- JOURNAL.md | 4 ++-- PLAN.md | 20 +++++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index e09872469..e28301f41 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -17,13 +17,13 @@ StructuralChunker is not supposed to return an empty list for any document witho ## Week 8 — Reproduction & solution planning -**Reproduction commit link:** [link to commit documenting the reproduced issue] +**Reproduction commit link:** https://github.com/ascherj/pathreview/commit/c365e22c1a92590de989216cae2b763a77fe7b4a **Reproduction summary:** [1–2 sentences: How did you reproduce the issue? What did you observe?] To reproduce the issue, I used one of the included unit tests within the pathreview project. Under the tests subfolder (pathreview/tests/unit/test_structural_chunker.py) and with the virtual environment activated, I ran the command "python3 -m pytest tests/unit/test_structural_chunker.py::TestStructuralChunker::test_document_with_no_headings -v", which specifically uses the test_document_with_no_headings function in the structural_chunker python file. Running the test function, it returns an AssertionError (assert 0>= 1). -**PLAN.md link:** [link to PLAN.md in your fork] +**PLAN.md link:** https://github.com/JohnPhm/pathreview/blob/fix/149-structural-chunker-silently-drops-documents/PLAN.md **Walkthrough video (recommended):** [link to your Loom video, ≤2 min — recommended, not graded] diff --git a/PLAN.md b/PLAN.md index 80e3035c5..850ea2c3d 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1,24 +1,38 @@ ## Solution plan -**Issue:** Structural chunker silently drops documents that contain no headings #149 +**Issue:** Structural chunker silently drops documents that contain no headings #149 ### Understand What is the root cause of this issue? What behavior is expected vs. actual? -The root cause of this issue + +The root cause of this issue is that the content is only collected if a heading is presented. Content is only collected once a heading is on the stack and if there is no heading on the stack, then no content is ever collected. The expected behavior of this should have skipped text before the first heading instead of skipping the entire document. ### Map Which files, functions, or modules are involved? List the specific files you expect to touch. +The files, functions, or modules that are involved with the structural_chunker.py file include semantic_chunker.py, pipeline.py, strategy_selector.py, and the function itself _extract_sections() in strucutural_chunker.py. The testing file test_structural_chunker.py is also used to test the function for its correctness. + ### Plan What are the steps to fix this issue? Break it into 3–5 concrete sub-tasks. +1. Confirm that the issue exists and that the tests fail before fixing. +2. Fix the main issue with the content guard within structural_chunker.py. +3. Fix the mid-loop save so that text before the first heading is not silently dropped and so it becomes its own section. +4. Verify that test_structural_chunker.py functions correctly using the tests; test_semantic_chunker.py, test_heading_path_format, test_empty_sections_handled, test_heading_not_in_middle_of_content. + ### Inputs & outputs What does your fix take as input? What should it produce or change? +The fix itself doesn't change the input of the chunker. StructuralChunker still takes a raw document and metadata as its input. The function _extract_sections() should produce list[dict], with each dictionary shaped: {"content":str, "path": list[str], "level":int}. + ### Risks & unknowns What could go wrong? What are you still unsure about? +The chunker could create empty chunks from blank preamble or it could emit content twice. I am still unsure if the proposed fix is able to handle all of the edge cases. + ### Edge cases -What inputs or states should your fix handle gracefully? \ No newline at end of file +What inputs or states should your fix handle gracefully? + +My fix should be able to gracefully handle a completely empty document that has a heading, documents with repeated headings, and documents that start with a heading. Furthermore, it should be able to handle a document that has pre-heading content as just blank lines. \ No newline at end of file From efe2e21316bad58916576f0cab9a6c9a630d5be1 Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sat, 8 Aug 2026 22:52:41 -0700 Subject: [PATCH 06/10] fix:fixed ruff errors in structural_chunker.py and test_structural_chunker.py, edited JOURNAL.md --- JOURNAL.md | 31 +++++++++++ ingestion/chunking/structural_chunker.py | 67 +++++++++++++++--------- tests/unit/test_structural_chunker.py | 11 ++-- 3 files changed, 80 insertions(+), 29 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index e28301f41..fb61bc367 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -29,3 +29,34 @@ To reproduce the issue, I used one of the included unit tests within the pathrev **Blockers or open questions:** Going into week 9, I am still confused on how to write and modify unit tests to confirm that my fixes to the structural chunker works correctly. + +## Week 9 — Solution building & PR submission + +### Check-in 1 (mid-week) + +**Current progress:** +So far, I have been able to confirm that the issue exists and am able to reproduce the issue. + +**Next steps:** +For the rest of the week, I will be working on the implementation of the issue fix. + +**Blockers:** +Going from planning to implementation is taking longer than I expected. + +--- + +### Check-in 2 (end of week) + +**PR link:** [link to your submitted pull request] + +**Branch:** fix/149-structural-chunker-silently-drops-documents + +**What you built:** +[1–3 sentences summarizing what your fix does and how it works] + +**Tests added or updated:** +[Which test files did you touch? What do they cover?] + +**Self-review confirmation:** [ ] make check passes [ ] make test-unit passes + +**Draft PR feedback received from:** [name or Slack handle, or "none"] diff --git a/ingestion/chunking/structural_chunker.py b/ingestion/chunking/structural_chunker.py index d5bcf0530..ba32c5f20 100644 --- a/ingestion/chunking/structural_chunker.py +++ b/ingestion/chunking/structural_chunker.py @@ -49,22 +49,26 @@ def chunk(self, text: str, metadata: dict) -> list[Chunk]: if section_tokens > self.SECTION_TOKEN_LIMIT: # Sub-chunk using semantic chunker section_metadata = metadata.copy() - section_metadata.update({ - "heading_path": heading_path, - "heading_level": section["level"], - }) + section_metadata.update( + { + "heading_path": heading_path, + "heading_level": section["level"], + } + ) sub_chunks = self.semantic_chunker.chunk(section_text, section_metadata) chunks.extend(sub_chunks) else: # Single chunk for this section section_metadata = metadata.copy() - section_metadata.update({ - "heading_path": heading_path, - "heading_level": section["level"], - "chunk_index": len(chunks), - "char_start": 0, - "char_end": len(section_text), - }) + section_metadata.update( + { + "heading_path": heading_path, + "heading_level": section["level"], + "chunk_index": len(chunks), + "char_start": 0, + "char_end": len(section_text), + } + ) chunks.append(Chunk(text=section_text, metadata=section_metadata)) return chunks @@ -79,7 +83,7 @@ def _extract_sections(self, text: str) -> list[dict]: sections = [] heading_stack = [] # Stack of (level, heading_text) current_section_lines = [] - current_level = 0 + # current_level = 0 for line in lines: heading_match = re.match(r"^(#{1,6})\s+(.+)$", line) @@ -88,11 +92,13 @@ def _extract_sections(self, text: str) -> list[dict]: # Save previous section if exists if current_section_lines: if heading_stack: - sections.append({ - "content": "\n".join(current_section_lines).strip(), - "path": [h[1] for h in heading_stack], - "level": heading_stack[-1][0] if heading_stack else 0, - }) + sections.append( + { + "content": "\n".join(current_section_lines).strip(), + "path": [h[1] for h in heading_stack], + "level": heading_stack[-1][0] if heading_stack else 0, + } + ) current_section_lines = [] # Process new heading @@ -104,19 +110,28 @@ def _extract_sections(self, text: str) -> list[dict]: heading_stack.pop() heading_stack.append((heading_level, heading_text)) - current_level = heading_level + # current_level = heading_level else: # Regular content line - if heading_stack or current_section_lines: # Only collect if we have a heading - current_section_lines.append(line) + # if heading_stack or current_section_lines: # Only collect if we have a heading + current_section_lines.append(line) # Save final section - if current_section_lines and heading_stack: - sections.append({ - "content": "\n".join(current_section_lines).strip(), - "path": [h[1] for h in heading_stack], - "level": heading_stack[-1][0] if heading_stack else 0, - }) + # if current_section_lines and heading_stack: + # sections.append({ + # "content": "\n".join(current_section_lines).strip(), + # "path": [h[1] for h in heading_stack], + # "level": heading_stack[-1][0] if heading_stack else 0, + # }) + content = "\n".join(current_section_lines).strip() + if content: + sections.append( + { + "content": content, + "path": [h[1] for h in heading_stack], # [] when no headings + "level": heading_stack[-1][0] if heading_stack else 0, # 0 when no headings + } + ) return sections diff --git a/tests/unit/test_structural_chunker.py b/tests/unit/test_structural_chunker.py index a0b5d0d5d..0d038c6ba 100644 --- a/tests/unit/test_structural_chunker.py +++ b/tests/unit/test_structural_chunker.py @@ -2,8 +2,8 @@ import pytest -from ingestion.chunking.structural_chunker import StructuralChunker from ingestion.chunking.base import Chunk +from ingestion.chunking.structural_chunker import StructuralChunker @pytest.mark.unit @@ -82,12 +82,16 @@ def test_heading_path_format(self, chunker): # Should have " > " as separator if it has parent found_path = True assert isinstance(path, str) + assert found_path, "No chunk had a heading_path containing 'Child'" def test_large_section_sub_chunked(self, chunker): """Test large section (> 800 tokens) gets sub-chunked.""" # Create a large section - large_section = """# Large Section -""" + "This is a paragraph with lots of content. " * 50 + large_section = ( + """# Large Section +""" + + "This is a paragraph with lots of content. " * 50 + ) result = chunker.chunk(large_section, {}) @@ -172,6 +176,7 @@ def test_heading_path_breadcrumb(self, chunker): # Should contain the hierarchy if "Installation" in path or "Prerequisites" in path: found_full_path = True + assert found_full_path, "No chunk had a breadcrumb with Installation/Prerequisites" def test_chunks_have_text_content(self, chunker): """Test that all chunks have text content.""" From a3b9163a67797f0ecec133e9cb11d80bcd81a730 Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sun, 9 Aug 2026 00:43:25 -0700 Subject: [PATCH 07/10] doc:updated JOURNAL.md to include link for PR --- JOURNAL.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index fb61bc367..70e5a96b4 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -47,16 +47,17 @@ Going from planning to implementation is taking longer than I expected. ### Check-in 2 (end of week) -**PR link:** [link to your submitted pull request] +**PR link:** https://github.com/ascherj/pathreview/pull/1014 **Branch:** fix/149-structural-chunker-silently-drops-documents **What you built:** -[1–3 sentences summarizing what your fix does and how it works] +For structural_chunker.py, the fix introduced was on the content branch and the final save. I removed both gates so that the lines are always collected. Furthermore, the current_level variable was removed as it was not being used. +For test_structural_chunker.py, the functions test_heading_path_format and test_heading_path_breadcrumb both had a boolean flag inside a loop that never got checked. This meant that both passed even if chunk() returned nothing. This was resolved by adding a trailing assertion to both of the functions. **Tests added or updated:** -[Which test files did you touch? What do they cover?] +The files that I touched include structural_chunker.py and test_structural_chunker.py. These two files are the ones that contain the actual error itself. Structural_chunker.py is the file that works as a chunker for RAG systems. The test_structural_chunker.py file works to test the functionality of each function found in the structural_chunker.py file. -**Self-review confirmation:** [ ] make check passes [ ] make test-unit passes +**Self-review confirmation:** [X] make check passes [X] make test-unit passes -**Draft PR feedback received from:** [name or Slack handle, or "none"] +**Draft PR feedback received from:** None \ No newline at end of file From 7f1d0ddeec7938ab53d36ee0db22b782570073f8 Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sun, 9 Aug 2026 19:25:32 -0700 Subject: [PATCH 08/10] doc: updated JOURNAL.md with more information --- JOURNAL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index 70e5a96b4..8797dae1f 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -35,13 +35,13 @@ Going into week 9, I am still confused on how to write and modify unit tests to ### Check-in 1 (mid-week) **Current progress:** -So far, I have been able to confirm that the issue exists and am able to reproduce the issue. +So far, I have been able to confirm that the issue exists and am able to reproduce the issue. This was done using the test_structural_chunker.py testing file, which resulted in the message "assert 0 >= 1" and lets us know that the functionality of structural chunker is incorrect. **Next steps:** -For the rest of the week, I will be working on the implementation of the issue fix. +For the rest of the week, I will be working on the implementation of the issue fix as well as the documentation and reasoning behind the implementation. **Blockers:** -Going from planning to implementation is taking longer than I expected. +Going from planning to implementation is taking longer than I expected. Furthermore, there are many errors that occur from using make lint, more specifically, there are 77 errors. --- From a001b24baea2dc5bc377bc514697c7bd6375525e Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Sun, 9 Aug 2026 19:37:26 -0700 Subject: [PATCH 09/10] doc:updated JOURNAL.md to include more information about the tests --- JOURNAL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/JOURNAL.md b/JOURNAL.md index 8797dae1f..2232e292b 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -56,7 +56,7 @@ For structural_chunker.py, the fix introduced was on the content branch and the For test_structural_chunker.py, the functions test_heading_path_format and test_heading_path_breadcrumb both had a boolean flag inside a loop that never got checked. This meant that both passed even if chunk() returned nothing. This was resolved by adding a trailing assertion to both of the functions. **Tests added or updated:** -The files that I touched include structural_chunker.py and test_structural_chunker.py. These two files are the ones that contain the actual error itself. Structural_chunker.py is the file that works as a chunker for RAG systems. The test_structural_chunker.py file works to test the functionality of each function found in the structural_chunker.py file. +The files that I touched include structural_chunker.py and test_structural_chunker.py. These two files are the ones that contain the actual error itself. Structural_chunker.py is the file that works as a chunker for RAG systems. The test_structural_chunker.py file works to test the functionality of each function found in the structural_chunker.py file. More specifically, the function test_document_with_no_headings() tests the implementation of the new code as it presents the chunker with a heading-less document. If the test passes, then it means that our logic and implementation was correct. **Self-review confirmation:** [X] make check passes [X] make test-unit passes From 694fbcead86238b4ce2b2feb45a4189b1c3124d1 Mon Sep 17 00:00:00 2001 From: JohnPhm Date: Tue, 11 Aug 2026 16:29:28 -0700 Subject: [PATCH 10/10] doc:updated JOURNAL.md to include reflection --- JOURNAL.md | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/JOURNAL.md b/JOURNAL.md index 2232e292b..0d7508dfb 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -60,4 +60,45 @@ The files that I touched include structural_chunker.py and test_structural_chunk **Self-review confirmation:** [X] make check passes [X] make test-unit passes -**Draft PR feedback received from:** None \ No newline at end of file +**Draft PR feedback received from:** None + +## Week 10 — Iteration & reflection + +### Reviewer feedback + +**Feedback received:** [ ] Yes [X] No — still awaiting review + +**Summary of feedback:** +I am still waiting for feedback of my pull request. + +**How you responded:** +[What changes did you make, or what did you reply? If no feedback, +leave blank.] +Since I did not technically receive any feedback about my PR on GitHub itself, I based my changes on the feedback that I received from the CodePath graders. The main feedback that I received about my PR was certain areas were lacking detail, such as the testing methodology and reasons why the test should be included/used. I added more detailing, explaining what the tests did and why I included/noted them in the PR. Furthermore, I also explained why I changed each file related to the issue and noted the reason why the issue occurred and why the solution worked. + +--- + +### Reflection + +**What was harder than you expected?** +[Be specific — what part of the process, codebase, or workflow +surprised you?] +For me, the parts that were harder than I expected were contributing to GitHub using the git commands in the terminal and attempting to fix the errors found when using 'make check'. I ran into multiple issues trying to connect my local machine to my GitHub as it kept asking me for reauthentication since my token expired. I had to generate a new one and I used Claude to guide me through the process as this was the first time I have encountered this error. On the other hand, I used the command 'make check' and there were 77 errors that resulted from the command. For a while, I thought that I needed to correct these errors before committing and pushing to GitHub but in retrospect, I realized this too far down the line. My section of interest did not come into contact with the majority of the errors so I did not have to fix the errors myself. Only the errors that concerned the area around my issue would need to be fixed. This made it difficult at the beginning but I realized that it was unncessary to fix. + +**What did you learn about working in a large codebase?** +[What's different about contributing to someone else's production code +vs. building your own project?] +I learned that building my own project and contributing to someone else's production code requires you to follow the guidelines that are established so that everyone contributing to that codebase is able to easily understand the changes. If there are coding conventions and contribution guidelines laid out, then it is best if I follow it to not risk my contributions being rejected. + +**How did AI tools help — and where did they fall short?** +[Where was AI assistance most useful this module? Where did you need +to go beyond what AI could give you?] +For this module, I used Claude to assist me whenever I encountered an error. There were times when the token for authentication expired and I had to browse through the GitHub settings to generate a new token and reauthenticate it. Claude helped walked me through this process and explain what and why it happened. The areas where Claude fell short include suggestions for the first implementation of structural_chunker.py. I had to read through the structural_chunker.py file itself and provided extra context to Claude so that it can provide further fixes to the new implementation. + +**What would you do differently if you started over?** +[Issue selection, planning, implementation, or process — anything +you'd change?] +If I started over, I would focus more on the process of iteratively fixing and committing each implementation. During the course of this module and the previous module, I was able to apply the correct implementation to alleviate the issue but I kept forgetting to regularly commit my changes and documenting why I included each change. This made it difficult as each time I took a break and came back, I felt momentarily lost as I had nothing to refer to in terms of my progress. + +**What are you most proud of from this module?** +The thing that I am most proud of from this module would be becoming proficient in using Git to stage and commit code directly from the VSCode terminal while also maintaining the standard of the commits. \ No newline at end of file