Repository navigation
docs: remove phantom parameters from two public docstrings (#2392) - #2493
RerankerGuo wants to merge 2 commits into
Conversation
🤖 Open Code ReviewTarget: PR #2493 ✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s). Generated by cloud-assistant via Open Code Review. |
✅ Automated Test Results: PASSEDAll tests passed (2/2 executed). memos_python_core/changed-repo-python: 2/2. Duration: 9s Branch: |
|
Fixed in 22ecd8f. Build failure. OpenCodeReview findings 1 and 2. |
22ecd8f to
c8c683f
Compare
✅ Automated Test Results: PASSEDAll tests passed (2/2 executed). memos_python_core/changed-repo-python: 2/2. Duration: 10s Branch: |
…#2392) Two public docstrings documented parameters that the AST no longer exposes: 1. `RedisPersistentUserManager.__init__` listed `user_id (str, optional):` as the first Args entry, but the signature is `(host, port, password, db, decode_responses)` — `user_id` is a leftover from an earlier API. Removed the phantom line. 2. `concat_original_source` listed `merge_field (List[str]): List of fields to merge`, which is actually a local variable inside the function, and omitted the real `rerank_source` parameter (the comma-separated metadata field names that the function actually accepts). Rewrote the docstring with both `graph_results` and `rerank_source` documented, with their actual semantics. To prevent the same kind of drift from sneaking back in, this commit also adds tests/test_docstring_signature_parity.py: it parses each function's source file, extracts the parameter list from the AST, extracts the `Args:` block parameter names from the docstring, and asserts the two sets match. The check works around the `@require_python_package` decorator that rewrites `__code__`, so it sees the real `__init__` signature rather than the wrapper's. Docs-only functional change; the parity test is the regression guard. Fixes MemTensor#2392.
ruff format rejected the over-long re.search line in test_docstring_signature_parity.py, failing every build-matrix job at the Ruff checks step; the file is now formatted. An AST-lookup miss previously degraded to an empty signature set, which made both parity diffs empty by construction so the assertions could never fail. Raise AssertionError instead so the 'fail loudly' contract in the module docstring holds. The module-scope fallback branch in _function_def_in_class was unreachable (the helper is only called for dotted qualnames) and is removed.
c8c683f to
46c9b9f
Compare
Summary
Two public docstrings documented parameters that the AST no longer exposes.
AST-verified mismatches
RedisPersistentUserManager.__init__(host, port, password, db, decode_responses)— the docstring’sArgssection started withuser_id (str, optional): User ID. Defaults to root., butuser_idis not a parameter (it is a leftover from an earlier API). Dropped.concat_original_source(graph_results, rerank_source)— the docstring documentedmerge_field (List[str]), which is actually a local variable inside the function, and omitted the realrerank_sourceparameter (the comma-separated metadata field names that the function actually accepts). Rewrote the docstring with both parameters documented, with their actual semantics.Regression guard
To prevent the same kind of drift from sneaking back in, this commit also adds
tests/test_docstring_signature_parity.py. It parses each function’s source file (so it sees the real signature even when the function is wrapped by@require_python_package), extracts the parameter list from the AST, extracts theArgs:block parameter names from the docstring, and asserts the two sets match. Any future divergence in either direction fails the test loudly.Verification
Ruff:
ruff format --checkandruff checkboth clean on touched files (0 new findings vsmain).Out of scope
Closes #2392