fix: address Copilot review on utils-split PR

[denis-samatov]

Summary

This PR addresses the automated Copilot review comments left on the feature/utils-split PR.

Changes

pageindex/core/pdf.py

• Fix KeyError crash: get_page_tokens() now wraps tiktoken.encoding_for_model() in try/except KeyError and falls back to cl100k_base — consistent with the same fallback logic already in core/llm.py's count_tokens()

tests/conftest.py

• Fix broken sys.path: Replaced ../src with .. (project root). The repo has no src/ directory, so the previous path was silently adding a non-existent location to sys.path, masking potential import failures

pageindex/utils.py

• Restore backward compatibility: Added explicit wrappers for the removed legacy symbols llm_completion and llm_acompletion. Without these, any code importing from pageindex.utils would raise NameError at runtime. The wrappers delegate to the new ChatGPT_API and ChatGPT_API_async in core/llm, with a thread-executor fallback for the async path

Testing

All 11 unit tests pass:

11 passed in 0.51s

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[Copilot]

Pull request overview

This PR follows up on the earlier feature/utils-split work by wiring pageindex to the new core/* modules, restoring legacy pageindex.utils symbols, and adding/adjusting tests + config loading to match the refactor.

Changes:

• Re-export core modules via pageindex/utils.py and add legacy llm_completion / llm_acompletion wrappers.
• Move page_index.py off from .utils import * onto explicit core.* imports and update call sites accordingly.
• Add core/ modules (llm, pdf, tree, logging), introduce Pydantic-based config loading, and add unit tests.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
pageindex/utils.py	Re-exports core modules and adds backward-compat wrappers for removed legacy symbols.
pageindex/page_index.py	Switches to explicit imports from core/* and updates LLM call sites and option handling.
pageindex/core/llm.py	Adds OpenAI client helpers, token counting, and JSON extraction utilities.
pageindex/core/pdf.py	Adds PDF helpers and per-page token counting with a tiktoken fallback.
pageindex/core/tree.py	Adds tree/structure utilities and summary generation helpers.
pageindex/core/logging.py	Adds JSON file logger used by indexing pipeline.
pageindex/config.py	Introduces Pydantic-backed PageIndexConfig + loader.
config.yaml	Adds repo-level default configuration values.
pyproject.toml	Declares package metadata + pinned dependencies.
tests/conftest.py	Fixes sys.path injection to project root for tests.
tests/test_tree.py	Adds tests for new tree utilities.
tests/test_llm.py	Adds tests for JSON extraction and token counting.
tests/test_config.py	Adds tests for config loading and validation behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[JiwaniZakir]

The default values in config.yaml and PageIndexConfig in pageindex/config.py are inconsistent in several places: toc_check_page_num is 20 vs. 3, max_page_num_each_node is 10 vs. 5, max_token_num_each_node is 20000 vs. 4000, and both if_add_doc_description and if_add_node_text are false in the YAML but True in the Pydantic model. Since ConfigLoader falls back to the Pydantic defaults when no config file is found (e.g., during testing or when the package is installed without the repo's config.yaml), this creates two distinct behavioral profiles depending on the deployment context — likely unintentional. The Pydantic field defaults should either match config.yaml exactly, or config.yaml should be the sole source of truth and the Pydantic defaults should be treated as last-resort fallbacks with that clearly documented.

Additionally, in ConfigLoader.load, filtering out None values from user_dict (if v is not None) silently prevents a caller from explicitly clearing a field that has a non-None default (e.g., setting api_key=None to override a key loaded from the YAML). Consider distinguishing between "not provided" and "explicitly set to None," possibly by using MISSING sentinel values or model_fields_set from Pydantic v2.