docs: add a docs-writing skill and fix structured-results against it (RES-1495) - #185
Conversation
The four existing docs skills all check prose without ever teaching it: docs-drift verifies claims, docs-coverage finds gaps, docs-autofill drafts unattended, mkdocs-material handles the build. Nothing said what a good page looks like, so every draft rediscovered the house voice from scratch. docs-writing holds the reader and the jargon budget, three page genres with their opening patterns and length bounds, four non-negotiables every page carries, eight voice rules each anchored to a before/after pair taken from targets.md, tuning.md and getting-started.md, the page-level mechanics, and a reviewer loop that gates a draft: three narrow sonnet lenses plus the opus content-evaluator, findings validated before they are applied, an explicit blocking list so the loop terminates, and a three-iteration cap that ships a [BLOCKED] PR rather than looping. CLAUDE.md hands off to it as step 3 of the docs checklist and gives up the two rules it now owns. The three rules that stay there stay because they bind in a docstring, a commit message or a PR body, where no docs skill is loaded. docs-autofill reads it before drafting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ilently loses The page showed three scorer functions and never called evaluatorq, so a reader landing from search had no way to run one. It also printed EvaluationResultCellValue in a signature without defining it, and described the terminal placeholder as a fact rather than as data they would not get back. The first example is now a complete program — job, scorer, Evaluator, run — that executes offline with no API key, followed by the read-back path, since the terminal never shows the sub-scores. Both guards in that loop are explained rather than presented as tidiness: a float evaluator and a scorer that raised both put a non-cell in the same slot, and the isinstance check is what keeps the loop alive when the reader adds their second evaluator. The type is spelled out with its three silent edges: bool coerces to 1, a list raises, a nested int reads back as a float, and a third level of nesting raises. A new section states the trade the page's own framing had been hiding — sub-scores are never aggregated, so a cell buys cohesion and costs you the per-key averages that separate evaluators would give. The tracing paragraph now names evaluator_type as the switch it turns on. Without it an evaluator gets exactly one span copy of the score, in an attribute Orq ingestion drops whole past 512 characters rather than truncating, and nothing logs the loss. Every code block on the page was executed in document order, and the evaluator_type gate and both read-back guards were verified against the running library rather than read off the source. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 3 of the review loop caught two claims the round 2 rewrite introduced, both verified against the library rather than argued about. The nesting rule was wrong in both directions. A sub-score dict may itself hold dicts of scalars, so two nested levels are legal and only the third raises. As written a reader who nested twice expected a validation error and would not have tried it, and one who nested three deep got an unexplained one. The explanation of the two `or []` guards described a case that does not occur. A job that raises still produces a JobResult, with an empty evaluator_scores list; job_results is None only when the datapoint fails before its jobs run at all. The code was right and the prose taught the wrong model of when results go missing. Also narrowed two overstatements: the blob-storage copy escapes the 512-character attribute cap rather than being uncapped outright, and pass_ is the only aggregated signal a structured evaluator contributes to a CI gate, not to the summary table, which shows the placeholder either way. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Round 3 of the reviewer loop ran after this PR opened and found two claims the round 2 rewrite had introduced. Both are now fixed in a follow-up commit, and both were verified against the running library rather than read off the source. The nesting rule was wrong in both directions. A sub-score dict may itself hold dicts of scalars, so The explanation of the two Two overstatements narrowed while there: the blob-storage span copy escapes the 512-character attribute cap rather than being uncapped outright, and That is three rounds, which is the cap the skill sets. Worth noting for anyone evaluating the loop itself: each round found a defect introduced by the previous round's fix, and every one of them was a false statement about behaviour rather than a matter of taste. A single-pass review would have shipped all three. |
Five example docstrings told the reader to run a path that does not exist — the files moved under examples/lib/ and the usage lines stayed behind, so anyone copying one got a file-not-found. These lines are the first thing a reader tries, and they are rendered verbatim onto the generated example pages, so the broken path was published rather than merely stale in the repo. The corrected command was executed for the rubric example rather than eyeballed, and every examples/**.py path referenced anywhere in examples/ or docs/ now resolves to a real file. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The example usage paths are fixed here after all, so the "Not fixed here" section of the PR description no longer applies. It was five files rather than the three I first reported. The three Verified two ways: the corrected command was executed for the rubric example rather than eyeballed, and a sweep confirms every Left alone deliberately: these five lines use bare |
These five usage lines invoked bare python while the rest of the tree uses uv run python. In a uv project the bare form reaches whichever interpreter is first on PATH, which is usually one without evaluatorq installed, so a reader copying the line gets an ImportError immediately after the path was fixed to point at a real file. The two lines carrying an env-var prefix keep it ahead of the runner, which is the form that works. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pasted Linear URL rots the moment a workspace or slug changes, and it is not what anyone greps, quotes in Slack or types into search — the id is. Linear already links both directions from the id plus the PR attachment, so the URL buys nothing and outlives its own correctness. The rule states the cost as well as the form: dropping the Closes line is what stops a merge from closing the ticket, so whoever drops it moves the ticket themselves. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
currentlycodinng
left a comment
There was a problem hiding this comment.
Ran every technical claim on the page against the library rather than reading it. Eight of nine hold. One does not, inline below.
Everything else checks out, confirmed by execution: True coerces to 1 as an int, False to 0. A list raises ValidationError. Nested int reads back 2.0. Both isinstance guards discriminate exactly as L89 describes, a 5-evaluator run put an EvaluationResultCell in one slot, three floats in three, and '' with error='scorer exploded' in the raised one, and a job that raised gave evaluator_scores=[]. Aggregation: three float evaluators rendered three averaged rows, the structured cell rendered [structured]. Tracing: without evaluator_type the span carried only orq.explanation and orq.score; a 10-key rubric with long names serialized to 539 characters, over the 512 cap; adding evaluator_type="python_eval" produced the second copy at orq.evaluation.output with all 10 keys intact, and spans.py:213 truncates the explanation but deliberately leaves orq.score alone, so the page is naming a real exposure. The first code block ran with ORQ_API_KEY and OPENAI_API_KEY both unset and produced the table shown, and the read-back block appended after it printed rubric rubric 0.44. mkdocs build --strict is clean, page is in mkdocs.yml nav and llmstxt sections. CI 13/13.
Two things outside this diff, so noting here instead of inline. The PR says every examples/**.py path referenced under examples/ or docs/ now resolves. One doesn't: examples/lib/integrations/langchain/langgraph_dataset_eval.py, referenced at examples/README.md:30 and :55. It's absent at the merge base too so this PR didn't break it, but the claim as written is wrong and the file is one line away from the five you did fix. Separately, examples/pairwise/bt_sigma_ranking.py:21 still invokes bare python rather than uv run python, the same defect the five corrected.
On the skill itself: coherent, and the voice anchors are real text. Checked targets.md:3, tuning.md:3, tuning.md:15 and targets.md:14 against the current files, all verbatim. Every page the genre table names exists. The CLAUDE.md handoff is what it says: 2 rules moved and both landed at SKILL.md:78-79, 3 stay and all three genuinely bind in docstrings and commit messages. One citation is off though: SKILL.md:58-60 attributes the gotcha-placement example to getting-started.md, but that uv add/uv init text lives in docs/index.md:17 and docs/faq.md:27; getting-started.md:8-11 has a different caveat under the install command. Not blocking, the pattern holds, just the pointer.
Also CLAUDE.md picks up a new rule about naming tickets by id in the PR title, unrelated to docs-writing and not mentioned in the description. Fine rule, just flagging it's a stowaway.
Fix L15 and I'm happy with this.
… python The nesting clause undershot by a layer. A sub-score dict may hold dicts of scalars, so three dict layers are accepted and only the fourth raises; the sentence pointed back at the two-layer example and called the third an error. Verified at all four depths against the running model. This is the second correction to the same sentence: the previous one repaired the first half and left the trailing clause anchored to the wrong example. The voice section cited getting-started.md for the gotcha-placement example, but the uv add / uv init text quoted there lives in index.md and faq.md, and getting-started.md carries a different install caveat. index.md joins the exemplar list. bt_sigma_ranking.py invoked bare python, the same defect corrected in the other five usage lines. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Responses to the four points outside the diff. Three adopted, one deferred to you.
The The false sweep claim — adopted, and the cause is worth naming: the sweep ran with
On the stowaway — correct, and it is now called out in the PR description under its own heading rather than sitting in the diff unmentioned. Happy to pull it into its own PR if you would prefer the docs change stand alone. Thanks for re-running the claims against the library instead of reading them; the 539-character rubric is a better demonstration of the 512 cap than anything on the page. |
examples/README.md listed langgraph_dataset_eval.py in its tree and gave a command to run it. The file has never existed: PR #54 added four files under langchain/ and a README describing five, and nothing since created the fifth. It was not lost in a move, so there is nothing to restore. Every examples/**.py path referenced in any tracked file now resolves. The earlier version of that check searched only *.py files for references, so it could not see a dead link in a README, which is how this survived the first sweep. A dataset-based LangGraph example may still be worth having. That is a new example to write, not a reference to repair. Refs RES-1495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Followed up on So there was no lost file to weigh against tidiness, and both references are removed. Every The |
currentlycodinng
left a comment
There was a problem hiding this comment.
Re-checked against 5f6f8ef, not the commit titles.
L15: fixed. Now reads "one dict layer beyond that raises" with both the accepted and rejected examples spelled out. Matches what I got running it: {"a": {"b": {"c": 2}}} accepted, one level deeper raises.
examples/README.md: fixed. The langgraph_dataset_eval.py reference is gone from both the tree diagram and the usage block, so the claim about every referenced path resolving is now true.
bt_sigma_ranking.py:21: fixed, uv run python.
SKILL.md voice-anchor citation: fixed the right way. Rather than moving the getting-started.md attribution, it added index.md (and the same text is on faq.md:27) to the anchor list, and that uv add/uv init gotcha-placement example genuinely lives on both pages. Checked it against current content, verbatim.
Approving.
Why
The repo has four docs skills and all of them check prose without ever teaching it.
docs-driftverifies claims,docs-coveragefinds gaps,docs-autofilldrafts unattended,mkdocs-materialhandles the build. Nothing said what a good page looks like, so every draft rediscovered the house voice from scratch — anddocs-autofillin particular writes with no human in the loop and no voice guidance at all.The skill
.claude/skills/docs-writing/SKILL.mdholds:guides/targets.md,tuning.mdandguides/getting-started.mdrather than invented.mkdocs.ymlentries a new page needs, naming a current model.content-evaluatoragent, every finding validated before it is applied, an explicit blocking list so the loop terminates, and a three-iteration cap that ships a[BLOCKED]PR rather than looping.CLAUDE.mdhands off to it as step 3 of the docs checklist and gives up the two rules the skill now owns. Three rules stay inCLAUDE.mddeliberately — they bind in a docstring, a commit message or a PR body, where no docs skill is loaded, so moving them would have silently stopped them binding.The page
docs/structured-results.mdwas the first page run through the loop, over three rounds. What it found and the page now fixes:evaluatorq, defined a job, and called neither. A reader landing from search had three scorer functions and no way to run one. The first example is now a complete program that executes offline with no API key.isinstancecheck is what keeps the loop alive when the reader adds their second evaluator.EvaluationResultCellValueappeared in a signature and was never defined. It is now spelled out with its silent edges:boolcoerces to1, a list raises, anintbelow the top level reads back as afloat, and a dict nested deeper than two levels raises.[structured]placeholder. That is now a section, and it is the reason the scope line at the top says what it says.evaluator_typeas the switch that decides — without it an evaluator gets exactly one span copy of the score, in an attribute Orq ingestion drops whole past 512 characters rather than truncating, with nothing logged.Every code block on the page was executed in document order. The
evaluator_typegate, both read-back guards, thebool/list/nesting coercions, and the[structured]placeholder were each verified against the running library rather than read off the source.The example usage lines
Five example docstrings told the reader to run a path that does not exist — the files moved under
examples/lib/and the usage lines stayed behind. These lines render verbatim onto the generated example pages, so the broken path was published, not merely stale in the repo. The same five also invoked barepython, which in a uv project reaches whichever interpreter is first onPATH— usually one without evaluatorq installed — so a reader would have hit anImportErrorimmediately after the path was fixed. Both are corrected: real paths,uv run python, env-var prefixes kept ahead of the runner.examples/README.mdalso listedlanggraph_dataset_eval.pyin its tree and gave a command to run it. That file has never existed — PR #54 added four files underlangchain/and a README describing five — so both references are removed rather than repaired. A dataset-based LangGraph example may still be worth having, but that is one to write, not a reference to fix.Verified two ways rather than by eye: the documented command was executed for the rubric example, and every
examples/**.pypath referenced in any tracked file now resolves. The first version of that sweep searched only*.pyfiles for references, so it could not see a dead link in a README — which is how the README one survived it.What the dry run says about the loop itself
Worth recording, since it is the argument for the design:
evaluator_typeis set and none of the page's evaluators set it. Round 3 then caught two more: the nesting depth was wrong in both directions, and theor []explanation described a failure case that does not occur. All three were false statements about behaviour, not matters of taste — a single-pass review ships all three.Use it whentable that would have partitioned nothing), two were downgraded as overstated, one upgraded. A reviewer-inventedevaluator_typevalue was replaced with the codebase's own vocabulary.Checks
uv run --group docs mkdocs build --strict— clean, exit 0.ORQ_API_KEYunset.src/.One more CLAUDE.md rule
CLAUDE.mdgains a rule requiring the ticket to be named by its id in the PR title and by no URL anywhere —docs: … (RES-1495). The id is what a reader greps, quotes in Slack and types into search, and a pasted Linear URL rots the moment a workspace or slug changes while the id never does. Linear links both directions from the id plus the PR attachment, so the URL buys nothing.It is here rather than in its own PR because this PR is what exposed the problem, and because the rule states a cost that only shows up when you follow it: dropping the
Closes <id>line is what stops a merge from closing the ticket, so whoever drops it moves the ticket by hand. This PR is the first case — RES-1495 is closed manually.🤖 Generated with Claude Code