Skip to content

Integrate Portal reports into the Claude skill and MCP guidance [REPORT-95] - #17

Merged
dougmartin merged 13 commits into
mainfrom
REPORT-95-portal-reports-guidance
Sep 11, 2026
Merged

dougmartin merged 13 commits into
mainfrom
REPORT-95-portal-reports-guidance

Conversation

@dougmartin

@dougmartin dougmartin commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

cc-data has been able to create Portal report runs, download them and join them to student answers since REPORT-94, but Claude could not reliably drive any of it, because the guidance it reads was written for the Athena-only world and had only been patched where individual stories happened to touch it. A researcher asking Claude for class-level results got steered down the Athena path, which is slower, needs a report authored first, and returns a frozen snapshot.

The gap, measured rather than assumed

Each surface was rendered and probed, rather than read:

Fact Claude needs Skill MCP instructions
The phrase "Portal report" absent present
student-id-mapping slug absent absent
student-metadata slug absent absent
--refresh absent absent
student_id_mapping / student_metadata views present present
Re-pull rather than duplicate present present

The ticket's own background turned out to be stale, so the spec was written against the code instead. It said the researcher guide "asserts the opposite in two places" and that the two dimension views are "minimal stubs". REPORT-94 had already removed both strings in 564da81 and landed the two views, which are the longest entries in the views catalog. That relocated the work rather than removing it: the guide was in good shape, and the guidance was where the holes were.

What this adds

The shared core (rendered by both surfaces) learns the Athena/Portal distinction, the live-versus-snapshot model as a decision rule rather than a fact, the seven report slugs the code knows, and a six-step recipe for pulling a cohort's work without authoring an Athena report. The recipe's materialize step is a pointer, not a restatement: REPORT-113 owns that rule, and REPORT-115 templates its CLUE workflow on this recipe.

Command spellings cannot go in the core, because TestCoreNamesNoCommand rejects any "cc-data " string there, so the rule goes in the core and the spelling goes in skill_header.md, with the MCP surface carrying the parameter on tools.md. That is the same split the auth remedy already uses. A tool's Description is registered with the server and is not part of guidance.Instructions(), so a fact placed only in a description does not reach the MCP guidance at all; tools.md gains refresh for that reason.

Four new guards, and what each one catches

  • Report slugs: every slug the guidance names must exist in dataset.slugToType or duck.dimensionViews[].slug. The whole rendered core is scanned for the slug shape, not just the catalog section, so a typo in the view entries or the recipe is caught too. One direction only, deliberately: a slug the server offers and this repo has no constant for is not a defect the guidance should be blocked on.
  • report_type vocabularies, both directions. There are two of them, and conflating them is what made the original requirement wrong. A run carries [answers log usage] and nothing else, because a Portal run's report_type is null on the wire; the reports union additionally admits portal and recovered, both assigned locally. Adding portal to a sentence about runs would have told the model a Portal run carries report_type: portal while this story's own reports_list description teaches that execution is the discriminator. The gap was never a missing enum value, it was a missing discriminator.
  • Command spellings: every cc-data ... the guidance spells out resolves to a real command, across both rendered surfaces. Names only. --portal is not a cobra-required flag but a runtime fallback to default_portal, so there is nothing declarative to compare a flag against. The guard has to check cobra.Find's leftover args rather than only its error, or a renamed subcommand resolves to its parent and the guard passes; two of three mutations survived until that was fixed.
  • Tool descriptions: each of six descriptions must carry the one named Portal fact it is responsible for. The existing drift guard compares tool names and never reads a description, so "mention Portal runs where relevant" was a requirement that could not fail.

ParseCatalog's identifier pattern had to be widened to accept hyphens first. Without that, a slug catalog parses as empty and its guard reports success while checking nothing. The four shipped catalogs parse identically before and after, and the existing bidirectional guards would fail on any name the widened pattern gained or lost, so no new test was added for them.

Deliberately not done

The Portal offers five aggregate metrics reports whose slugs are in no Go inventory and which no endpoint enumerates, so three of the five are not in this repo to copy. The catalog says it is not exhaustive, in one sentence, and REPORT-130 is filed to expose the v1 report catalog from the server so nothing has to hardcode slugs at all. Tree.api_report_slugs/0 already computes exactly that list server-side and already gates report creation.

slugToType gains a comment saying what it is and what nothing reconciles it against. The repo already refused a slug map once for this reason: deriving a report's type from execution rather than a slug lookup is what lets a Portal report added to the server later be recognized without a cc-data release.

Verification

13 files, +537/-30. 8 new tests; every one was verified to go red under a deliberate break. go build, go vet, gofmt -l and go test ./... are clean on the head commit.

Measured on the head commit: core 17,003 bytes, skill 20,291, MCP instructions 20,539, which is +2,669 / +3,385 / +2,782 against main. The skill grows most because it alone carries the command spellings the core is forbidden to name. The full recipe was drafted both ways before being written: the minimal decision rule was +765 bytes against the full recipe's +1,532, which is not where a context budget is won or lost.

The spec is closed at specs/REPORT-95-portal-reports-guidance.md.

Requirements and implementation plan for closing the gap between what cc-data
can do with Portal reports and what the guidance tells Claude about them. The
tool can create Portal runs, download them and join them to student answers;
the shared core both surfaces render was written for the Athena-only world and
has only been patched where individual stories touched it, so a researcher
asking for class-level results gets steered down the Athena path, which is
slower, needs a report authored first, and returns a frozen snapshot.

Measured rather than assumed: the phrase "Portal report" is absent from the
skill surface, neither surface names a single report slug, and neither mentions
--refresh, so the first step of the documented workflow is unexecutable from the
guidance alone.

What the plan builds, in five steps:

- Widening ParseCatalog's identifier pattern to accept hyphens. It lands alone
  because it changes a regex three shipped guards depend on, and every report
  slug carries a hyphen, so without it a slug catalog parses as empty and its
  guard passes while checking nothing.

- Two report_type guards, each against the code list that defines the sentence
  it checks. The run vocabulary and the download vocabulary are different sets,
  and conflating them is what an earlier draft of this spec did.

- The core gaining the Portal family, the live-versus-snapshot rule as a
  decision rule, a guarded slug catalog, the two dimension entries' missing
  facts, and the end-to-end recipe from creating a Student ID Mapping run to
  the final join.

- --refresh named on each surface, with six MCP descriptions gaining one named
  fact each, asserted by tests, since the drift guard checks tool names and
  never reads description text.

- A consistency pass over the researcher guide, whose expected outcome is a
  small diff or none, since REPORT-94 already gave it a full Portal treatment.

Decisions a reviewer needs. The run sentence at core.md:24 keeps its list and
gains the fact it lacks, that a Portal run has no report_type and execution is
what identifies it: a Portal run's report_type is null on the wire, pinned by
portalRunWire and stated as design in the API type, so adding portal to that
list would contradict the discriminator the same story teaches on reports_list.
portal and recovered are values cc-data assigns locally, and they go on the
downloads entry where they belong.

The slug catalog says in one sentence that it is not exhaustive, because the
Portal offers five aggregate metrics reports whose slugs are in no Go inventory
and which no endpoint enumerates. The guard runs one direction only, and what
it cannot prove is written beside slugToType rather than in the test: it holds
the guidance against cc-data's own copy of the slug list, never against the
server's. REPORT-130 was filed to replace that copy with a catalog read at
runtime, which is the same reasoning the fetch path already applied when it
derived a report's type from execution rather than a slug map.

The recipe names the joins but points at the rules that govern them rather than
restating them, for the withheld join key and for when materializing is worth
it. A correctness rule stated twice in one file with nothing holding the copies
together is how the two drift.

Command spellings cannot go in the core, which TestCoreNamesNoCommand enforces,
so the rule goes in the core and the --refresh spelling in the skill header,
matching the split the auth remedy already uses. The recipe was drafted into the
real file and run against that guard before being specified.

Stacked on REPORT-113, because the recipe points at materialize prose that
story adds to the same file. Rebase onto main before retargeting the PR when it
merges.
leadingNames excluded hyphens, so a section of report slugs parsed to nothing
and ParseCatalog reported success, which would have left a guard over it
checking nothing at all. Every report slug carries a hyphen, so the slug catalog
this story adds is unreachable without the change.

The four shipped catalogs are unaffected, and that needs no new test: the view,
tool, identity-column and researcher-guide guards already compare each catalog
against its code-derived set in both directions, so a name the widened pattern
gained or lost fails one of them. Measured either way, all four parse to the
same names as before. What does need a test is the failure the widening exists
to prevent, and it goes red against the old pattern.
…T-95]

There are two and they are not the same set. A run carries answers, log or
usage, and nothing else: a Portal run's report_type is null on the wire, which
is why execution identifies one. The reports union additionally admits portal
and recovered, both assigned locally, the first derived from a run's execution
and the second by a reindex that cannot classify a CSV. IsAllowedReportType was
a switch, which offers a guard nothing to read, so the allowlist becomes a slice
the predicate consults and each vocabulary gets an accessor.

The run vocabulary is now guarded against the sentence that states it, both
directions: a value the guidance names must be accepted by the code, and a value
the code accepts must be named or carry an exemption with a reason. The
exemption set ships empty and exists so a value added later has to make a
decision rather than be forgotten. The guard reads the sentence as written,
since it opens with prose rather than a backticked name and ParseCatalog cannot
help.

The download vocabulary's guard is not here. It was written first and confirmed
to fail for the right reason, that the core states no such sentence yet, and it
lands with the sentence rather than leaving a commit red.

slugToType gains a comment saying what it is: cc-data's own copy of the Athena
slugs, not a roster of what the server offers, and the thing the guidance guard
cannot vouch for.
… [REPORT-95]

The guidance was written for the Athena-only world. It never named the Portal
report family, never gave a single report slug, and never said how to re-read a
live report, so the first step of the documented workflow was unexecutable from
the guidance alone and a researcher asking for class-level results got steered
down the slower path.

Five edits to the shared core, which both surfaces render by construction. The
two families as a decision rule rather than a fact, since what a model needs is
which one to reach for. The run sentence keeping its list and gaining what it
lacked: a Portal run has no report_type, and execution identifies it. The
downloads entry stating its own vocabulary, where portal and recovered belong,
both being values cc-data assigns rather than values a run arrives with. A
guarded slug catalog, which is what makes creating a run possible from the
guidance alone, saying in one sentence that it is not exhaustive because the
Portal offers aggregate reports no Go inventory knows. And the end-to-end
recipe, from creating a mapping run to the final join.

The recipe names operations rather than commands, which is what keeps the
no-commands guard green, and it points at rules rather than restating them: at
the materialize rule for when copying is worth it, and at the mapping view's
entry for what a NULL join key means. A correctness rule stated twice in one
file with nothing holding the copies together is how the two drift.

Two guards close behind the prose. The download vocabulary now has the sentence
its guard was written against, and every documented slug must exist in code. The
slug guard runs one direction only, because the reverse would fail on reports
the server offers and no Go constant names.
The core states the rule, that a Portal report is refreshed by re-pulling the
same run rather than duplicating it, and cannot state the flag, because the
no-commands guard rejects any cc-data spelling there. So the skill header gains
--refresh and the MCP guidance gains refresh, which is the split the auth remedy
already uses.

The tools.md edit is not redundant with the tool description. A tool's
Description is registered with the MCP server and is not part of the rendered
instructions, which are the MCP header plus the core plus tools.md, so the MCP
surface's guidance learns about refresh only if tools.md says so.

Six descriptions gain one named fact each rather than a general instruction to
mention Portal runs: how to tell an Athena run from a Portal one, how a mapping
run's filter is assembled, how a Portal report is re-read, and that a mapping
run id is a valid source for each of the three bulk fetches. The drift guard
cannot hold any of this, because it compares tool names and never reads a
description, so each is asserted directly.

The both-surfaces test covers only the per-surface halves. The rule itself lives
in the core, which both surfaces render by construction, so asserting it on each
would be true by construction and would test nothing.
…EPORT-95]

The expected outcome here was a small diff or none, and it was small: REPORT-94
already gave the guide a full Portal treatment, so it already named --refresh
and both mapping slugs. Two things were left.

The guide's run-creation section offered only an Athena example, so a researcher
reading it would not learn that a whole cohort can be pulled without authoring an
Athena report at all. It now points at the mapping path the guide already
describes further down. The same section said a Portal report is re-read with
get report, without the flag that does it, which the section two hundred lines
later does name.

The logs entry names three slugs inline, and now that the core defines two report
families it has to sit visibly under one. It says they are Athena.
ReportSlugs returns slugToType's keys, which is the Athena half and not every
slug a report can be created from; the call site unions it with the dimension
half. The name promised the whole set.

The plan now also records two things the implementation settled: the download
vocabulary's guard ships with the sentence it checks rather than with its
sibling, so no commit lands red, and the accessors are exported for the same
reason StaticViewNames and IdentityColumnNames already are.
skill_header.md named no cc-data reports subcommand at all, so a CLI-driving
model would read the new slug catalog, read the recipe's opening step of
creating a student-id-mapping run and assembling its filter, and have no command
for either. Steps 1 and 4 of the six-step recipe were unexecutable on that
surface, which is this story's own defect one level up: knowing the string and
not the verb is as unexecutable as not knowing the string. The MCP surface
needed nothing, since tools.md already names both tools.

The header gains reports list, filter-options, create and duplicate, every
spelling checked against its own --help. That caught one of mine: reports
duplicate requires --portal and the first draft omitted it, which is exactly the
failure the slug guard exists to prevent, one layer out. A test holds the skill
surface to the two verbs the recipe needs, and asserts nothing about the MCP
side, because the shipped tool guard already fails if either tool leaves the
catalog, checked by removing one.

Three fixes from the review pass. The recipe's materialize step used a section
title as a bare noun phrase, so "when Materializing a dataset says it is worth
it" had no coherent subject; it now names the section, as the step below it
already did for the view entry. The researcher guide names the Report types
section rather than pointing "below", which survives reordering. And the
slugToType comment drops its closing ticket reference: the constraint it states
is complete without one, and no other comment in the tree cites a ticket.

The spec records the gap, the decision behind the ticket-free comment, and the
finished surface sizes, which had been pinned to a commit two merges behind.
…RT-95]

The guidance spells out cc-data commands because the core is forbidden to, and
nothing compared those spellings against the commands that exist. A renamed or
removed subcommand would leave the skill telling a researcher to run something
that is gone. The guard walks every backticked invocation in both rendered
surfaces and asserts it resolves.

cobra.Find alone is not enough, and the first version of this guard passed on
two of three deliberate breaks because of it: Find returns the deepest command
it matched plus the args it could not consume, and errors only when the first
word is unknown, so `reports clone` resolves to `reports` with a leftover. The
leftovers are the signal.

Names only, deliberately. Flags cannot be guarded the same way: --portal is not
a cobra-required flag but a runtime fallback to default_portal, so there is
nothing declarative to compare against.

That same fact corrects the four lines this branch added. They carried
--portal, on the mistaken belief that omitting it was an error; it is needed
only when no default portal is configured, which the rest of the file already
handles by showing what is unconditionally required and nothing more. The flag
is named once as a condition instead of four times as a requirement.
Replace the requirements and implementation files with a single closed
spec holding the overview, the verified gap table, the 22 requirements,
the technical notes and the decisions that shaped the work.

The decisions worth carrying forward: the slug guard runs one direction
only, because a slug the server offers and the code lacks is not a
defect the guidance should block on; the core states the run vocabulary
and the downloads entry states the download one, since the two differ
and conflating them was the original requirement's error; and the
guidance names the seven slugs it can prove while saying the list is
not exhaustive, with the server-side catalog that removes the drift
class filed as REPORT-130.

Copilot AI 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.

🟡 Changes recommended

Guidance inconsistencies and incomplete slug validation can still mislead users or permit documentation drift.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Portal-report workflows to shared Claude skill and MCP guidance, backed by drift guards.

Changes:

  • Documents Portal/Athena behavior, report slugs, refresh semantics, and cohort workflow.
  • Expands MCP tool descriptions and researcher guidance.
  • Adds guards for slugs, report types, commands, and descriptions.
File summaries
File Description
specs/REPORT-95-portal-reports-guidance.md Records requirements and decisions.
internal/mcpserver/tools.go Adds Portal facts to tool descriptions.
internal/mcpserver/server_test.go Guards tool-description facts.
internal/guidance/src/tools.md Documents MCP refresh behavior.
internal/guidance/src/skill_header.md Adds report CLI workflow.
internal/guidance/src/core.md Adds shared Portal guidance and recipe.
internal/guidance/guard_test.go Adds vocabulary and slug guards.
internal/guidance/catalog.go Supports hyphenated catalog names.
internal/guidance/catalog_test.go Tests hyphenated parsing.
internal/duck/views.go Exposes dimension-view slugs.
internal/dataset/reporttype.go Exposes report-type and slug inventories.
docs/researcher-guide.md Adds refresh and cohort guidance.
cmd/guidance_commands_test.go Validates documented command names.
Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 4
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/guidance/guard_test.go Outdated
Comment thread internal/guidance/src/core.md
Comment thread internal/guidance/src/core.md Outdated
Comment thread specs/REPORT-95-portal-reports-guidance.md Outdated
The slug guard parsed the `## Report slugs` catalog, so a typo in the
slugs the view entries and the recipe name passed unnoticed, which is
weaker than the guard's own claim. It now scans the rendered core for
the backticked lowercase-hyphenated shape, which today matches the seven
slugs and nothing else, with an empty exemption map for a future
hyphenated identifier that is not a slug.

Two prose corrections come with it. The researcher guide gave a Portal
run `report_type` `portal`, which is the type cc-data assigns to the
download it records; the run itself carries none, and the core now
teaches that `execution` is the discriminator. The core also sent
readers to the guide for the aggregate reports' slugs, which the guide
does not list, so it now says cc-data cannot enumerate them and points
at an existing run instead.
@dougmartin
dougmartin requested a review from emcelroy September 11, 2026 14:22
@dougmartin
dougmartin deleted the branch main September 11, 2026 17:09
@dougmartin dougmartin closed this Sep 11, 2026
@dougmartin dougmartin reopened this Sep 11, 2026
@dougmartin
dougmartin changed the base branch from REPORT-113-dataset-materialization to main September 11, 2026 17:10

@emcelroy emcelroy 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.

Looks good 👍 A Claude review raised the minor issues below that you may want to consider addressing before merge.


1. Label the two Portal slugs. internal/guidance/src/core.md:191-198 marks two entries "Athena" and leaves student-id-mapping and student-metadata unmarked. A reader must infer their family from the recipe's last line. Add "Portal, live" to both.

2. One bullet carries a sentence its neighbour lacks. In the same list, student-id-mapping says a run of it is a valid run id for fetching answers, history and attachments; student-metadata says nothing. Between two entries of the same shape, that reads as a difference. Say it on both, or on neither.

3. The command guard blames the wrong file. cmd/guidance_commands_test.go:41-48 scans the skill and the MCP instructions, but every message says "the skill names". Change it to "the guidance names".

4. RunReportTypes's comment claims too much. internal/dataset/reporttype.go:50-52 says it returns what the server sends. It returns the distinct values of slugToType. Those match today; they are not the same thing. Suggested wording: "RunReportTypes returns the distinct report types in cc-data's known Athena slug map. It does not enumerate or validate the server's vocabulary."

5. Re-wrap one paragraph. internal/guidance/src/core.md:155 is 90 columns and line 142 is left short, so the text was not re-wrapped after the insert.

The catalog's two Portal entries now carry "Portal, live" beside the "Athena" marks on their neighbors, and the sentence saying a run of one is a valid run id for answers, history and attachments moves to the intro so it covers both instead of reading as a difference between them. The researcher guide's matching pair gets the same move.

The command guard's failure messages name the guidance rather than the skill, since it scans both surfaces. RunReportTypes's comment says what it returns, the distinct types of the known slug map, rather than claiming to be the server's vocabulary. The two view bullets that were left ragged by an insert are re-wrapped.
@dougmartin

Copy link
Copy Markdown
Member Author

Thanks. All five addressed in 169fe60.

  1. Both Portal entries in the core catalog now open with "Portal, live", matching the "Athena" marks on the other three.
  2. The run-id sentence moved out of the student-id-mapping bullet into the section intro, stated once for either Portal report. The researcher guide's pair had the same asymmetry, so its version moved into that list's intro too.
  3. The command guard's messages now say "the guidance names", since it scans both surfaces.
  4. RunReportTypes uses your wording: the distinct types in the known Athena slug map, not the server's vocabulary.
  5. The student_id_mapping and student_metadata view bullets were re-flowed at 80 columns, which removes both the 90-column line and the short one.

@dougmartin
dougmartin merged commit 58c9172 into main Sep 11, 2026
10 checks passed
@dougmartin
dougmartin deleted the REPORT-95-portal-reports-guidance branch September 11, 2026 18:48
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.

3 participants