Recover GraphRAG extraction and keep corpus operations consistent - #93
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 012a9ea5c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue | ||
| fingerprint = await graph_schema_input_fingerprint(corpus, cfg) | ||
| postgres = PostgresClient(cfg.indexing.postgres_url) | ||
| await postgres.connect() |
There was a problem hiding this comment.
When PostgreSQL is unreachable during the UI's automatic saved-schema restoration, this connect() exception is not passed through raise_postgres_unavailable_if_applicable, so the new endpoint returns a generic 500 instead of the structured, retryable dependency-unavailable 503 used by the other index read boundaries. Catch transport/Postgres failures around both corpus/config loading and proposal retrieval, and declare the 503 response contract on this route.
Useful? React with 👍 / 👎.
| continue | ||
| if _semantic_kg_usage_run_matches(run, corpus_id=corpus_id, alias=alias, schema_hash=schema_hash): | ||
| candidates.append(run) | ||
| return max(candidates, key=lambda run: (run.started_at, run.run_id), default=None) |
There was a problem hiding this comment.
When the newest matching run has only failed, cached, conflicting, or not-yet-ingested native rows, this eagerly selected run makes _semantic_kg_usage_sample return None and prevents an older matching run with valid successful requests from contributing any output-token estimate. The resulting quote incorrectly reports that no matching settled sample exists; candidates need to be tried newest-to-oldest until one yields usable evidence.
Useful? React with 👍 / 👎.
012a9ea to
ac4f83a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac4f83a711
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async def capture_source(path: Path) -> SourceSnapshot: | ||
| snapshot = await asyncio.to_thread(SourceSnapshot.capture, path, max_bytes=max_indexable_bytes) | ||
| assert source_snapshots is not None | ||
| source_snapshots.enter_context(snapshot) |
There was a problem hiding this comment.
For semantic corpora whose aggregate source size exceeds the LXC's temporary-disk capacity, registering every snapshot on the run-wide ExitStack keeps a complete second copy of all files until _run_index_body finishes, even though a snapshot is no longer read after that file's document, chunks, checkpoint, and graph have been written. This can exhaust /tmp and fail otherwise valid large indexing runs; scope each snapshot to its file and release it after the retained extraction/checkpoint work for that file has drained.
Useful? React with 👍 / 👎.
| _examples_digest=extraction_digest(examples), | ||
| _template_digest=extraction_digest(prompt_template.template), | ||
| ) | ||
| self._files[file_path] = file |
There was a problem hiding this comment.
For semantic runs near the supported 40,000–100,000 chunk ceiling, this run-wide map retains every ExtractionFileCheckpoint, including _official_chunks containing serialized full chunk text, until final promotion. Consequently memory grows with the entire corpus even after file_graph_chunks.clear() releases the normal per-file batch, which can cause large runs to be killed after successfully checkpointing substantial work; replace completed entries with lightweight file-completion metadata or discard their bulky input fields after mark_written().
Useful? React with 👍 / 👎.
Fence durable chunk checkpoints, release completed file resources, restore truthful graph progress and saved schemas, and qualify extraction estimates. Refresh corpus controls and guard cleanup atomically. Repair mobile panes and generated documentation wrappers. @codex review
DMontgomery40
changed the title
ac4f83a to
e99ee66
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e99ee66868
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await asyncio.to_thread( | ||
| persist_graph_progress, self.path, repo_id=self.state.repo_id, | ||
| run_id=self.state.run_id, extraction=self.state.extraction.model_copy(deep=True), | ||
| ) |
There was a problem hiding this comment.
For large semantic runs, this persists the entire run summary once for every chunk transition: each reused chunk emits at least selected/admitted/reused, while each fresh chunk also emits dispatching. Since persist_graph_progress atomically rewrites the JSON and fsyncs both the file and directory, the supported default 40,000-chunk run performs roughly 120,000–160,000 summary rewrites, and close() blocks promotion until that queue fully drains. This can make otherwise completed large runs spend hours on progress I/O; apply every event in order in memory, but coalesce or checkpoint the latest sequence at a bounded interval plus final drain.
Useful? React with 👍 / 👎.
1 participant
Failed semantic index replacements discarded completed extraction work and reported whole-file failures as measured attempts. This change durably checkpoints validated official chunk graphs in Postgres and reuses them only when source bytes, provenance, approved schema, prompt, model route and recipe match exactly. Existing generation and provenance checks still decide whether a replacement can be published. Cancellation drains active writers before releasing ownership; completed files release their snapshots, payloads and successful tasks. Corrupt checkpoints fail without a silent paid retry.
Run history now persists measured admission, durable success, reuse, failure and unfinished counts while preserving accounting. The Indexing page restores a saved current schema through a typed read-only request, respects unapplied edits, and keeps technical details behind disclosure controls. Semantic estimates use the actual extraction contract and qualified native usage; missing evidence remains unavailable. Both saved-schema reads map database outages to the shared retryable 503.
Corpus controls refresh after indexing and when reopened. Cleanup requires a successful fresh registry read and an atomic
only_unindexedcondition under the existing database lock. If indexing finishes during cleanup, the API returns typed 409 and the UI preserves that corpus. Conditional cleanup also resolves configuration without inserting or migrating stored settings or warming caches; ordinary configuration reads and explicit manual deletion retain their existing behavior. Mobile Graph and Source panes, drawers and error footers remain reachable across narrow layouts and desktop resize boundaries.The documentation generator and normal autopilot remove recognized whole-page Markdown wrappers before strict build and publication. Repair preserves internal code fences, rejects unfamiliar page shapes before writing, and is available without a provider call. Generated pages remain bot-owned.
Validation on LXC100: 700 extraction/storage/progress/lifecycle cases, 142 outage/schema/estimate cases, 112 corpus deletion, dependency and configuration cases, and 67 documentation cases passed. Real HTTP, PostgreSQL locks, commit cancellation and cross-store assertions cover the failure families. Six independent browser regressions reproduced stale corpus state and unsafe cleanup before passing with the correction. The 32-case graph ordering suite and four unchanged Chat registry cases passed, alongside the 17-case schema matrix and mobile pane/drawer/error-footer/resize coverage. Whole-server Ruff/mypy, generated types/contracts, 454 configuration keys, 435 capability rows, banned-pattern and docs-ownership checks pass. Independent Sol xhigh reviews cover each material slice. Full CI on the previous head passed 3,947 tests; this updated head still requires CI, deployment and actual operator acceptance. The approved Apollo replacement follows deployment; historical lost extraction outputs cannot be recovered by these checkpoints.