Skip to content

docs: rewrite generated docstrings in pipeline.py - #140

Merged
Paul-Kyle merged 4 commits into
phasespace-labs:mainfrom
JodVarun:docs/rewrite-pipeline-docstrings
Aug 16, 2026
Merged

docs: rewrite generated docstrings in pipeline.py#140
Paul-Kyle merged 4 commits into
phasespace-labs:mainfrom
JodVarun:docs/rewrite-pipeline-docstrings

Conversation

@JodVarun

@JodVarun JodVarun commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #136

Rewrote machine-generated docstrings and inline comments in palinode/ingest/pipeline.py to natural English, matching the style established in #131 per issue #130.

@Paul-Kyle

Copy link
Copy Markdown
Member

Close — the comments are handled exactly right, and I want to flag that first because it is the
instruction most people skim: you kept "Cap for very large PDFs" and "Simple readability: strip HTML
tags" and dropped only the generated tails, which is what the issue asked for.

Two things before I merge.

Three lines from the issue list are still untouched, all Returns: lines — current 160, 199 and
251:

  • str | None: The resulting context pathway saving to memory if perfectly transcribed.
  • str | None: Active reference resulting memory path.
  • str | None: Successful parsed response local filepath.

And four of the rewritten lines don't match what the code does. I would rather raise these than
merge them, because the whole point of the issue is that a confident wrong docstring costs a reader
more than an obviously empty one:

  • ingest_url_file Returns (:224) — "Returns target URL extracted from shortcut file." It
    returns ingest_url(...), so the value is the saved research-file path, or None. This is the one
    I would most like fixed: it is new text and it is confidently wrong.
  • ingest_audio (:153) — "Stream audio chunks." It opens the file and posts it once as
    multipart; nothing is chunked.
  • ingest_text (:192) — "Determine if the input string is a URL." That is the branch test, not
    the function: it ingests the text, handing off to URL ingestion when the content looks like a URL.
  • write_research_file (:291) — "Generate YAML frontmatter." It writes the research file and
    returns its path; the frontmatter is a step inside that.

No rush, and none of this is a rewrite — it is seven lines. Push when you have them and I will merge
on green.

@JodVarun

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback! I've updated the 3 untouched Returns: lines and corrected the 4 docstrings to accurately reflect function behavior. Pushed the update!

@Paul-Kyle

Copy link
Copy Markdown
Member

All seven addressed — merging. Apologies for the overnight wait on the corrected push.

The one I most wanted right is ingest_url_file's return: it now says the saved research-file path
rather than the extracted URL, which is what return ingest_url(...) actually gives you. A wrong
docstring costs a reader more than a meaningless one, so that correction is the difference between
this file being fixed and being differently broken.

This is your first contribution here and it took a review round to land — that is completely normal,
and the second pass was clean. Thanks for the patience.

@Paul-Kyle
Paul-Kyle merged commit eb4e0de into phasespace-labs:main Aug 16, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Finish the generated-docstring cleanup in ingest/pipeline.py

2 participants