Skip to content

fix(harness): allow product learnings to be edited - #179

Open
imran-ishaq wants to merge 1 commit into
theam:mainfrom
imran-ishaq:fix/product-learning-edits
Open

fix(harness): allow product learnings to be edited#179
imran-ishaq wants to merge 1 commit into
theam:mainfrom
imran-ishaq:fix/product-learning-edits

Conversation

@imran-ishaq

@imran-ishaq imran-ishaq commented Aug 18, 2026

Copy link
Copy Markdown

Summary

This PR fixes Product KB edits for existing L learning entries. The Product workspace already treats learnings as normal editable artifacts, but the backend validator rejects them in Product spaces because L is only declared in the research chain. As a result, a user can open and edit a learning page in the UI, but saving fails with unknown_artifact_type.

The fix aligns the Product artifact-chain contract with the UI by adding L as a supported Product KB type. Regression coverage proves both the harness validator and the API PATCH route accept an existing Product-space learning entry.

Expected user impact:

  • Existing Product-space learning pages can be edited successfully.
  • The API no longer rejects content-only edits to L pages with unknown_artifact_type.
  • Validation remains strict because the fix makes L an explicit Product type instead of bypassing unknown-type checks.

Fixes #36

Test plan

  • Harness test accepts product learnings passes
  • API test edits existing product learning entries passes
  • In Product, open an existing learning page, edit the body, save, refresh — no unknown_artifact_type

Product KB spaces already show agent-written L pages as editable, but
the chain validator rejected them as unknown. Declare Learning on the
product chain so PATCH saves succeed.
@ophiocus

Copy link
Copy Markdown

Not blocking this — just linking it up. I'd been reproducing #36 separately and posted what I found on the issue: the trigger turns out to be the space config rather than a writer, since chainFromConfig defaults to the research chain and PUT /kb/space swaps it without revalidating. Which means the same thing can strand H, E, F, CR, SR — and S, D, T, V, R in the other direction.

Declaring L is still a real question in its own right (the one wrinkle is that L is already Literature in the research chain, so it's a naming call), and it's orthogonal to the validation side. I have a patch for that half if it's useful; it doesn't touch chain.ts, so it wouldn't conflict with this.

@imran-ishaq
imran-ishaq marked this pull request as ready for review August 20, 2026 14:21
@imran-ishaq

Copy link
Copy Markdown
Author

Not blocking this — just linking it up. I'd been reproducing #36 separately and posted what I found on the issue: the trigger turns out to be the space config rather than a writer, since chainFromConfig defaults to the research chain and PUT /kb/space swaps it without revalidating. Which means the same thing can strand H, E, F, CR, SR — and S, D, T, V, R in the other direction.

Declaring L is still a real question in its own right (the one wrinkle is that L is already Literature in the research chain, so it's a naming call), and it's orthogonal to the validation side. I have a patch for that half if it's useful; it doesn't touch chain.ts, so it wouldn't conflict with this.

Agreed - this matches what I was seeing too. The orphan comes from the config swap/research default, not the writers: chainFromConfig falls through to research, and PUT /kb/space can swap the chain without rechecking stored pages. Same thing can strand H/E/F/CR/SR, and S/D/T/V/R the other way.

I’d rather refuse a chain change that would leave existing types illegal, and default product spaces to chain: "product". Soft-allowing unknown types on PATCH feels like recovery, not the real fix. Product Learning vs Literature L is a separate naming call either way.

Please open your patch as a PR alongside this one - it shouldn’t conflict, and both can be reviewed and merged.

@ophiocus

Copy link
Copy Markdown

Opened it as #195.

I took a couple of days before posting — I wanted to run my own findings against the
code rather than ship the patch I had in my pocket. That one was the soft-allow you
described (keep stored entries of an undeclared type editable), and you're right that it
is recovery, not the fix: it keeps pages saveable, but every later validation has to
know about the exception. So the PR does what you said instead: the PUT refuses a
config whose chain would leave a stored entry undeclared, with a 409 that names the
chain and the entries, and the stored config stays as it was. It holds end to end on
the API, not just in the harness.

It doesn't touch chain.ts, so it sits beside this one; declaring L stays yours.
The other half you raised — defaulting product spaces to chain: "product" — I agree
with and deliberately left out: it's a behaviour change for every new project, and it
has to be persisted at creation time rather than at resolution time, or existing
{} spaces get re-chained retroactively, which is the exact stranding the PR refuses.
Happy to open that as its own PR if the maintainers want it.

@adrian-lorenzo adrian-lorenzo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution!

Adding L to the Product chain makes sense, but this leaves the repository with conflicting definitions of that chain: the runtime validator calls L “Learning,” while the seeded product-chain contract and the Product KB documentation do not declare it. The web also labels every L entry as a learning even though the Research chain defines L as Literature. Please update these representations together and add coverage that the UI label follows the selected chain, so L has one clear meaning within each workspace.

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.

KB: editing agent-written pages 400s when their type is not in the space chain config (unknown_artifact_type)

3 participants