docs: sync documentation with recent changes - #77
Open
nadeem4 wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review window
Prior "docs: sync documentation with recent changes" PR: #60, created 2026-08-28T12:17:56Z. This reviews everything merged into
mainsince then: #62–#75 (excluding #65/#67, which are release-please's ownchore: release mainPRs with no doc-relevant content beyondCHANGELOG.md, which release-please owns).Most of those PRs (#62, #63, #64, #66, #68, #69, #70, #73, #74) already keep README.md/docs/ in sync as part of their own commits — verified by grepping the repo for every package-name reference #62 could have missed and checking each PR's own docs edits against the current code. This PR only covers what PR #75 missed, plus one API behavior change from #74 that had no doc coverage at all, plus a pre-existing nav gap that blocks
mkdocs build --strict.Corrections
docs/architecture/pipeline.md and docs/architecture/failure_recovery.md: both said routing "raises
PipelineError" when no compatible subgraph is found for a datasource. That was the exact bug fix(pipeline): raise a real exception when no subgraph matches #75 fixed —PipelineErroris a pydanticBaseModel, not an exception, so that code actually raisedTypeError: exceptions must derive from BaseException(see the regression test fix(pipeline): raise a real exception when no subgraph matches #75 added,test_scan_layer_router_raise_is_a_real_exception_not_a_type_error). fix(pipeline): raise a real exception when no subgraph matches #75 introducedPipelineExecutionError(anNL2SQLErrorcarrying thePipelineErroron.error) and updateddocs/architecture/subgraphs/main_pipeline_graph.mdin its own commit, but missed these two other files describing the same code path. Verified againstpackages/nl2sql/src/nl2sql/pipeline/routes.pyandpackages/nl2sql/src/nl2sql/common/exceptions.pyonmain.docs/observability/error-handling.md: opened with the blanket claim "NL2SQL represents failures as structured
PipelineErrorobjects and propagates them through state," which is now incomplete — the router path added by fix(pipeline): raise a real exception when no subgraph matches #75 raises an exception instead. Added a paragraph namingPipelineExecutionErrorand howrun_with_graph()'s crash handler folds it back intoGraphState.errorsas anUNKNOWN_ERROR. Verified againstpackages/nl2sql/src/nl2sql/pipeline/runtime.py.docs/api/core/datasource.md:
DatasourceAPI.get_datasource_detailsreturns connection args, but the docs never mentioned that secret-shaped values are masked. fix(demo): make the docker demo datasources reachable #74 addedmask_connection_args()(packages/nl2sql/src/nl2sql/datasources/registry.py), which replaces any connection-arg value whose key containspassword,secret,token, orapi_keywith"***", and wired it intoget_datasource_details(packages/nl2sql/src/nl2sql/api/datasource_api.py:126) as a security fix (a pydanticValidationErrorused to be able to echo a plaintext password). Documented the masking so the public API surface doc matches actual behavior.mkdocs.yml:
docs/getting_started/demo.mdexists and is already linked fromdocs/getting_started/index.md("Demo Data (CLI-first)"), but was never added to thenav:tree, which makesmkdocs build --strictfail with "pages exist... but are not included in the nav configuration." This predates the review window, but it blocks the strict build required by this task, so I registered it under Getting Started to match its existing sibling entries and its own link text inindex.md.Verification
main, not just the PR diffs.nl2sql[/pip install nl2sqlreferences that chore: rename the published distribution to nl2sql-engine #62's own docs commit might have missed — none found; all remaining hits are the import package name (nl2sql) or thepackages/nl2sqlsource path, both correctly unchanged by the PyPI rename.pip install -r requirements-docs.txt && python -m mkdocs build --strict: fails locally with exactly one warning —mkdocs-mermaid2-plugincan't reachunpkg.comto verify the mermaid.js CDN URL.curl -sS "$HTTPS_PROXY/__agentproxy/status"confirmsunpkg.com:443is rejected by this sandbox's outbound network policy (connect_rejected, gateway 403), not by anything in the repo. This is the identical, sandbox-only failure PR docs: sync documentation with recent changes #60 documented and it was not a regression there either — thedocsCI check (real internet access) already passed onmainat feat(cli): add application container to docker demo; make mssql opt-in #59/fix(ci): build the API image from the repository root #70. With the nav fix above, that CDN check is the only warning the strict build produces; every content and nav validation passes.Not touched
CHANGELOG.mdand version numbers (release-please owns those).packages/api/Dockerfile.dev— still flagged from docs: sync documentation with recent changes #60 as broken and out of scope for a docs fix (code issue, not a docs one).doctorCLI command (fix(cli): repair the doctor connectivity check #72 fixed a bug in it, restoring behavior to what limited existing docs already implied) — it isn't documented anywhere indocs/orREADME.md, but that gap predates this review window and fix(cli): repair the doctor connectivity check #72 didn't change what's documented, so it's out of scope here.Generated by Claude Code