Skip to content

Docs: dev-setup instructions leave a contributor with no test runner, and CHANGELOG is missing four merged PRs - #236

Closed
jaylfc wants to merge 1 commit into
masterfrom
exec/tsk-twljv4
Closed

Docs: dev-setup instructions leave a contributor with no test runner, and CHANGELOG is missing four merged PRs#236
jaylfc wants to merge 1 commit into
masterfrom
exec/tsk-twljv4

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 9, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Docs: dev-setup instructions leave a contributor with no test runner, and CHANGELOG is missing four merged PRs

Autonomous build of board card tsk-twljv4.

REVIEW WARNING (automated): this card's text asks for tests, but the diff changes no test file. Either the acceptance criteria are unmet or the card needs correcting. Do not merge without resolving this.

README.md now has an explicit Dev setup section with the working uv
commands (uv sync, uv run pytest). The old pip install -e . instructions
are kept for runtime use, with a plain warning that they give you no test
runner. pip is not documented as a dev path because it is not available
in this environment and PEP 735 --group support could not be verified.

CHANGELOG.md Unreleased gains short entries for A2A envelope refs+blocks
(#212), GET /version capabilities (#213), CollectionStore _db.connect
helper (#215), and the PEP 735 dev dependency group (#219).

Fixes the two doc-staleness problems found in the 2026-07-28 audit.

Files:
CHANGELOG.md | 8 ++++++++
README.md | 24 +++++++++++++++++++++++-
2 files changed, 31 insertions(+), 1 deletion(-)

Summary by CodeRabbit

  • New Features

    • Added support for optional A2A message references and blocks, including body validation and a 64 KB serialized-message limit.
    • Added capability reporting to the /version endpoint and included the same information in /health.
  • Documentation

    • Clarified runtime-only and development installation options.
    • Added uv-based development setup and verification steps.
    • Documented that pip is not the recommended development installation path.

…#215, #219

README.md now has an explicit Dev setup section with the working uv
commands (uv sync, uv run pytest). The old pip install -e . instructions
are kept for runtime use, with a plain warning that they give you no test
runner. pip is not documented as a dev path because it is not available
in this environment and PEP 735 --group support could not be verified.

CHANGELOG.md Unreleased gains short entries for A2A envelope refs+blocks
(#212), GET /version capabilities (#213), CollectionStore _db.connect
helper (#215), and the PEP 735 dev dependency group (#219).

Fixes the two doc-staleness problems found in the 2026-07-28 audit.
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request updates the changelog and README. The documentation covers recent API and dependency changes and defines a uv-based development setup with import checks and test execution.

Changes

Documentation updates

Layer / File(s) Summary
Release note entries
CHANGELOG.md
The changelog records A2A message field rules, capabilities reporting, database connection handling, and development dependency changes.
Development installation guidance
README.md
The README distinguishes runtime-only editable installs from the supported uv development setup, including import checks and test execution.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies both main changes: development setup instructions and four missing changelog entries.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch exec/tsk-twljv4

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

Comment thread CHANGELOG.md

Dev and test dependencies moved into a PEP 735 `[dependency-groups] dev` group, so a bare `uv sync` from a clean checkout installs pytest, pytest-asyncio, and the auth extras needed by the test suite with no flags (PR #219).

`GET /version` with a capability list (new `taosmd.capabilities` module). The server now publishes what the running build actually supports, because neither a status code nor a version number could answer that. `taosmd serve` renders the dashboard SPA on unknown non-API paths, so `GET /collections` returns `200 text/html` on a build with no collections code, and an integrator who "verified" a route by checking for a 200 got a confident yes from a server that could not do the thing (this really happened, against the wrong service). Semver does not close the gap either: features land continuously between bumps, and a production box sat a month stale without anyone noticing even though `GET /health` already reported a version. `/version` returns `{"version", "commit", "commit_source", "built_at", "built_at_source", "capabilities"}` and `GET /health` gains the same `capabilities` list alongside its existing `status` and `version` keys, which are unchanged (taOS and the dashboard consume both). Both endpoints are unauthenticated by design, joining `/health` in `_PUBLIC_PATHS`, so monitoring and drift probes keep working on a token-secured box; they expose build identity and capability identifiers only (no paths, no tokens, no configuration). Capabilities are **stable contract identifiers with an explicit version suffix** (`collections.v1`, `grants.v1`, `temporal.v1`, `a2a.v1`, `tasks.v1`, `ingest.v1`, `search.v1`, `graph.v1`, `shelves.v1`), not feature names: a breaking change to a wire contract becomes `collections.v2`, so a client pinned to `collections.v1` sees the capability disappear (a visible break it can act on) rather than `collections` silently meaning something new; additive changes keep the identifier. The list is derived at request time by probing the running build (each identifier is declared next to the module and symbols that implement it, and is advertised only if they resolve), so deleting or renaming an implementation deletes the claim instead of leaving a stale boast, and a divergence test asserts every declared capability's routes exist in the real dispatcher. The commit sha is resolved once at first call and cached, never per request and never by shelling out: `git rev-parse` in a request path can block on a lock or a slow filesystem, so the git plumbing is read directly from the filesystem (`.git/HEAD` -> loose ref or `packed-refs`, including the `gitdir:` indirection used by worktrees and submodules), with an optional packaged `taosmd/_build_info.py` stamp taking precedence for wheel and container builds. Every step degrades to `null` rather than raising, so a pip install with no checkout and no stamp still gets a working endpoint.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Duplicate CHANGELOG entry for PR #213

The new concise entry at line 7 (GET /version endpoint with a contract-identifier capabilities list) describes the same change as the existing verbose entry starting at this line (GET /version with a capability list). Both reference PR #213.

The old entry should have been removed when the new concise entries were inserted. As written, the Unreleased section contains two different versions of the same release note, which will produce duplicate or conflicting entries when the changelog is cut.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
WARNING 1
Issue Details (click to expand)

WARNING

File Line Issue
CHANGELOG.md 13 Duplicate entry for PR #213 — the new concise entry at line 7 and the existing verbose entry at line 13 both describe the same GET /version capabilities change. The old entry should have been removed when the new concise entries were inserted.
Files Reviewed (2 files)
  • CHANGELOG.md - 1 issue
  • README.md

Fix these issues in Kilo Cloud


Reviewed by step-3.7-flash · Input: 63K · Output: 5K · Cached: 401.3K

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Line 7: Remove the duplicate /version capabilities release note from
CHANGELOG.md, preserving a single canonical entry and merging any unique useful
details into it before deleting the repetition.

In `@README.md`:
- Line 65: Update the editable-install warning in the README’s Install paragraph
to accurately state that pip install -e . succeeds but does not install the PEP
735 dev dependency group, test dependencies, or a test runner; remove the claim
that the failure is silent.
- Around line 118-120: Update the README setup documentation to remove the
temporary explanation about pip availability, the current environment, and
unverified PEP 735 support. State only the stable supported setup contract,
identifying uv sync as the supported short path.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 53dab621-8e46-47d2-91c8-376e454bd526

📥 Commits

Reviewing files that changed from the base of the PR and between 25b35af and 2290a02.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • README.md

Comment thread CHANGELOG.md

A2A message envelope: `POST /a2a/send` now accepts optional `refs` and `blocks` fields. `refs` is a list of up to 8 structured reference objects (kind, title, uri, etc.), and `blocks` is a list of arbitrary objects for rich content. The total serialized message is capped at 64KB. `body` remains required when `blocks` is absent, and is required alongside `blocks` when blocks are present (PR #212).

`GET /version` endpoint with a contract-identifier capabilities list (new `taosmd.capabilities` module). The server publishes what the running build actually supports, because neither a status code nor a version number could answer that question reliably. `GET /health` also gains the same capabilities list. Capabilities are stable contract identifiers with an explicit version suffix (e.g. `collections.v1`), not feature names, so a breaking change becomes `collections.v2` and a client pinned to `v1` sees the capability disappear rather than silently meaning something new. The list is derived at request time by probing the running build (PR #213).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the duplicate /version release note.

This entry repeats the /version and capabilities change already documented in Line [13] of CHANGELOG.md. Keep one canonical entry, or merge the additional detail into this entry and remove the duplicate.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CHANGELOG.md` at line 7, Remove the duplicate /version capabilities release
note from CHANGELOG.md, preserving a single canonical entry and merging any
unique useful details into it before deleting the repetition.

Comment thread README.md
### One-Line Setup (manual)

> **Install:** `pip install taosmd` (add the MCP server with `pip install "taosmd[mcp]"`). For a source/dev install instead, `git clone` then `pip install -e .`. The one-line bootstrap below additionally installs Ollama and downloads the embedding and LLM models; it is newer and still being validated across clean machines, so please report issues.
> **Install:** `pip install taosmd` (add the MCP server with `pip install "taosmd[mcp]"`). For a source install instead, `git clone` then `pip install -e .`. Warning: `pip install -e .` installs the runtime only and gives you no test runner. The failure is silent, so a reader will otherwise assume their setup worked. See the Dev setup section below for the working commands. The one-line bootstrap below additionally installs Ollama and downloads the embedding and LLM models; it is newer and still being validated across clean machines, so please report issues.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Confirm the dependency-group configuration and the related README wording.
rg -n -A12 -B2 '^\[dependency-groups\]' pyproject.toml
rg -n -C2 'The failure is silent|pip install -e \.|uv sync' README.md

Repository: jaylfc/taosmd

Length of output: 2173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== editable install installable metadata =="
sed -n '1,140p' pyproject.toml

echo
echo "== references to dependency groups / optional extras =="
rg -n 'dependency-groups|group=|optional-dependencies|uv (install|sync)|pip install \.|-e|pytest|test runner' README.md pyproject.toml

echo
echo "== package files =="
git ls-files | sed -n '/pyproject.toml/,+100p' | head -80

Repository: jaylfc/taosmd

Length of output: 19083


Describe the editable install result accurately.

pip install -e . installs the package successfully, but it does not install the PEP 735 dev dependency group or test tooling. Replace “The failure is silent” with wording such as “The install succeeds, but it does not install the test dependencies or a test runner.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 65, Update the editable-install warning in the README’s
Install paragraph to accurately state that pip install -e . succeeds but does
not install the PEP 735 dev dependency group, test dependencies, or a test
runner; remove the claim that the failure is silent.

Comment thread README.md
Comment on lines +118 to +120
`pip` is not documented as a working dev-setup path here because it is not
available in this environment and PEP 735 `--group` support could not be
verified. `uv sync` is the supported short path.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Confirm that the supported README path matches the repository configuration.
rg -n -A12 -B2 '^\[dependency-groups\]' pyproject.toml
rg -n -C3 'not available in this environment|PEP 735|uv sync' README.md

Repository: jaylfc/taosmd

Length of output: 1281


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the dependency configuration and README dev-setup section around the reported lines.
cat -n pyproject.toml | sed -n '1,70p'
printf '\n--- README dev-setup section ---\n'
cat -n README.md | sed -n '96,128p'

Repository: jaylfc/taosmd

Length of output: 4020


Remove the temporary environment explanation from the public docs.

“This environment” describes a local verification limitation, not a stable project requirement. State the supported contract directly. For example:

Suggested wording
-`pip` is not documented as a working dev-setup path here because it is not
-available in this environment and PEP 735 `--group` support could not be
-verified. `uv sync` is the supported short path.
+`uv sync` is the supported development-install path. It installs the project
+and its development dependency group.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
`pip` is not documented as a working dev-setup path here because it is not
available in this environment and PEP 735 `--group` support could not be
verified. `uv sync` is the supported short path.
`uv sync` is the supported development-install path. It installs the project
and its development dependency group.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 118 - 120, Update the README setup documentation to
remove the temporary explanation about pip availability, the current
environment, and unverified PEP 735 support. State only the stable supported
setup contract, identifying uv sync as the supported short path.

@jaylfc

jaylfc commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

Reviewed. Apologies for the delay: this has been open since 9 August with every check green and no comment from me. I found it by enumerating the open PRs rather than trusting my own notes, which had it listed nowhere.

The README half is a real gap correctly filled. One blocker in the CHANGELOG half, and one sentence to cut.

BLOCKER: the CHANGELOG now describes GET /version twice

The new short entry for PR #213 is inserted six lines above the existing, far more detailed entry for the same change, both under ## Unreleased.

PR branch, Unreleased section:
   7: `GET /version` endpoint with a contract-identifier capabilities list (new `taosmd.capabilities` module)...
  13: `GET /version` with a capability list (new `taosmd.capabilities` module). The server now publishes...

master, same query:
   5: `GET /version` with a capability list (new `taosmd.capabilities` module). The server now publishes...

One on master, two on the branch. The existing entry already covers everything the new one says and considerably more (the SPA-200 trap, why semver does not close the gap, the v1/v2 break semantics, the commit-sha resolution). Drop the new /version paragraph and keep the original.

Worth a glance at the other three while you are in there. #212, #215 and #219 read as genuinely missing from master rather than restatements, and #215's claim is one I independently confirmed today from the other direction: _db.connect really is the convention, which is why I flagged a store on another PR for bypassing it. So those three are earning their place; only /version is the duplicate.

CUT THIS SENTENCE: it documents the author's environment, not the software

`pip` is not documented as a working dev-setup path here because it is not
available in this environment and PEP 735 `--group` support could not be
verified. `uv sync` is the supported short path.

A reader has no idea what "this environment" is and no reason to care; it is the build container, not theirs. Worse, it reads as "pip does not work for dev setup", when what actually happened is that pip was not tried. Those are very different claims to leave in a README, and the weaker one is being stated in the stronger one's voice.

pip 25.1 added --group, so pip install -e . --group dev is plausibly the pip path here. I have NOT run it, so I am not asking you to document it on my say-so. Two honest options:

  • verify pip install -e . --group dev on a clean venv and document it alongside uv sync; or
  • just delete the paragraph. uv sync presented as the supported path needs no explanation of what else was not tried.

Either is fine. What should not ship is a README telling readers about a limitation of the machine that generated it.

Verified good, do not change

I checked the claim the whole PR rests on rather than assuming it:

pyproject.toml:
[dependency-groups]
dev = ["pytest", "pytest-asyncio", "pyjwt>=2.8", "cryptography>=42"]

So uv sync genuinely does install a test runner plus the auth extras the suite needs, and uv run pytest is a working path from a clean checkout. Master's README has no dev-setup section at all (checked), so this is filling a real hole rather than restating one.

The pip install -e . warning is accurate and worth keeping, including the point that the failure is silent. That is the actual trap: the install succeeds, and the missing test runner only surfaces later as a confusing error. Saying so at both the install line and the dev section is proportionate rather than repetitive, since a reader may hit either first.

Drop the duplicate paragraph and resolve the pip sentence and this is good to merge.

@jaylfc

jaylfc commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

One more CHANGELOG item for this PR while you are in that section, so it does not need a second pass.

#240 (deleted-symbols gate, merged today as f6b5a17) has no CHANGELOG entry. I checked master: zero mentions of the gate. It belongs in ## Unreleased alongside the #219 dependency-group entry, since both are contributor-facing rather than API changes.

I deliberately did not commit it straight to master, because this PR is already editing that exact section and a master commit would hand you a conflict for no reason.

The part worth writing down is the waiver, which is currently undiscoverable: a Removes-Intentionally: <symbol>, ... trailer in the PR body suppresses the gate for named symbols, and the workflow has the edited trigger so adding the trailer re-runs the check automatically. Today that is documented only in a comment inside the workflow YAML and a docstring in scripts/check_deleted_symbols.py.

Making the failure message itself say so is carded separately as tsk-gkmvcp and is not your job here. A CHANGELOG line is.

@jaylfc

jaylfc commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

Revision card filed: tsk-xhm3ln. This PR stays open; the card's STEP 0 squash-merges the branch so the work carries forward.

Filed late, and the reason is worth recording rather than glossing. When I enumerated the frozen PR set earlier I built the list from my own running notes instead of from a fresh sweep of open PRs against "has unresolved review findings", so this PR was missed while eight others got cards. The hourly freshness pass caught it. That is the same failure I have written up twice today: an accurate local observation generalised into a list I never re-measured.

Mechanics as with the others: next_card.py:32 excludes any card whose exec/* PR is open, and executor.sh still has no per-card BASE override, so the revision needs a new card. STEP 0 uses git merge --squash because executor.sh:584 bounces branches containing a merge commit.

@jaylfc

jaylfc commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded by #256.

The review of this PR asked for the revision; tsk-xhm3ln produced it as #256 (same two files, +5 doc lines).

Verified before closing: #256 branches off master with its own commits and its changed-file set is a superset of this PR's, so closing this loses no content. The two would also conflict if both landed.

This PR was holding one of the 8 CI throttle slots on jaylfc/taosmd, which sat at 32/8 (locked). Reopen if the supersession is wrong.

@jaylfc jaylfc closed this Aug 17, 2026
jaylfc added a commit that referenced this pull request Aug 17, 2026
…e container, and three merged PRs still unlogged (#256)

* carry forward the tsk-twljv4 work

* fix: deduplicate /version CHANGELOG entry, reword README container sentence, log #240 #243 #245
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.

1 participant