feat: add KB child topics resource - #285
Open
Ruari-Phipps wants to merge 6 commits into
Open
Conversation
Adds a ChildTopic resource for variant-scoped knowledge base topics, stored at topics/<variant_name>/<topic_name>.yaml alongside the flat base topics. ChildTopic subclasses Topic to reuse YAML serialisation, pretty-printing and reference validation, overriding only what differs: the projection source (childOverwrites.knowledgeBase), the variant-nested file path, discovery, and the command type. Commands route through create/update/delete_child_topic so they act on the child overwrites collection rather than the base topics. The variant is inferred from the enclosing folder rather than stored in YAML, mirroring how FlowStep resolves its parent flow. Validation checks that the variant exists and that names are unique within the base topics plus that variant's child topics; the same name may be reused across variants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The guide described a registration flow that no longer exists: adding a _read_<type>_from_projection method to a SyncClientHandler in platform_api.py and hand-editing RESOURCE_NAME_TO_CLASS in project.py. Registration is now driven by the @register_resource decorator, and each resource class owns its own from_projection classmethod. Also documents the parent-scoped resource pattern, the find_new_kept_deleted name-recovery branch that cleaned-filename resources must opt into, and drops the venv activation step that conflicts with uv run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The variant folder was written with clean_name(lowercase=False), producing topics/Variant_1/topic_1.yaml while every other name in a topic path is lowercased. A variant declared as "Variant 1" therefore never matched its folder on disk, leaving variant_name unset, and file_path then crashed on None while diffing. Lowercase the variant folder and normalise both sides of the comparison, so "Variant 1" resolves to topics/variant_1/. When a variant still cannot be resolved, keep the folder as read from disk so file_path remains usable during discovery; validate() continues to reject it, since variant_id is left unset. file_path now raises a readable error rather than a TypeError when there is no variant at all. Also drop the non-ASCII fixture content and read the fixture JSON as UTF-8. The fixture was the only non-ASCII file in the repo and was decoded with the platform default encoding on Windows, corrupting the accented characters and making the resource look modified on every run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…teps Child topic names were checked only against the base topics and the child topics in the same variant. The platform requires a name to be unique across the base topics and every child topic in every variant, so a cross-variant duplicate passed locally and was rejected on push. Check every topic and child topic regardless of variant, and name the clashing variant in the error. FlowStep and FunctionStep had the same latent crash that was just fixed for child topics: get_flow_id_from_flow_name returns no flow when a step's flow config is missing or unreadable, and file_path then dereferenced the unset flow name, raising AttributeError while diffing. Fall back to the folder as read from disk so the path stays usable, and raise a readable error when there is no flow at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Contributor
Coverage Report
Changed file coverage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
ChildTopicresource for variant-scoped knowledge base topics, stored atchild_topics/<variant_name>/<topic_name>.yaml. It subclassesTopicto reuse serialisation and reference validation, overriding only the parts that genuinely differ. Also fixes a pre-existing crash in flow steps that was uncovered while building it.Motivation
Child topics (the platform's
childOverwrites.knowledgeBasecollection) had no representation in the ADK, so variant-scoped topics could not be pulled, diffed or pushed from a local project.Despite the platform naming, these are not overrides of a base topic. Confirmed against the platform backend: a child topic is assigned its own unique ID independent of any base topic, and there is no ID-based link field between the two. They are modelled here as what they are: independent, variant-scoped topics.
Changes
Child topics
ChildTopicinsrc/poly/resources/child_topic.py, registered aschild_topics, subclassingTopicchild_topics/<variant>/tree, kept separate from the base topics intopics/so discovery of the two can never overlapchildOverwrites.knowledgeBaseprojection, resolving variant names fromvariantManagementFlowStepresolves its parent flowcommand_type = "child_topic"so create/update/delete act on the child overwrites collection instead of the base topicsThe uniqueness rule matches the server rather than being stricter or looser than it:
getAllActiveTopicsconcatenatesknowledgeBase.topicswithchildOverwrites.knowledgeBase.topicsacross all variants, andCreateChildTopicHandlerinherits that check fromCreateTopicHandler. Enforcing it locally turns what would be a push-time rejection into a message naming the clashing file.Flow steps — pre-existing crash
FlowStepandFunctionStepresolve their flow viaget_flow_id_from_flow_name, which returns nothing when a step's flow config is missing, renamed or unparseable.file_paththen dereferenced the unset flow name, so any orphaned step crashedpoly diff/statuswithAttributeError: 'NoneType' object has no attribute 'lower'.Both now fall back to the folder as read from disk, so the path stays usable and the real problem is reported by validation instead of a stack trace. This bug predates the branch;
ChildTopicinitially inherited it by copying theFlowSteppattern.Test fixtures
Docs
resource-scaffolderagent guide, which documented a registration flow (_read_<type>_from_projectiononSyncClientHandler, hand-editingRESOURCE_NAME_TO_CLASS) that no longer exists — registration is now the@register_resourcedecorator plus afrom_projectionclassmethod on the resourceTest strategy
poly <command>)Checklist
ruff check .andruff format --check .passpytestpassespolyCLI interface (or migration path documented)