Refactor/code wiki kb - #2328
Open
kissghosts wants to merge 14 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds repository-backed Code Wiki creation, generation, stable page identity, incremental rebuild decisions, publication gating, projection cleanup, repository state reads, read-access enforcement, and API and service test coverage. ChangesCode Wiki lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
backend/app/api/endpoints/knowledge.py (1)
509-542: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve exception chains in re-raised
HTTPExceptions.Ruff (B904) flags the three
exceptblocks in this new endpoint (lines 512-513, 535-540, 542) for re-raising withoutfrom e/from exc. Chaining preserves the original traceback for logs/APM even though the client-facing response is unchanged.♻️ Proposed fix
try: source = SourceRepository.from_url(data.source_type, data.source_url) assert_user_can_read_source(db, current_user.id, source) except SourceAccessDenied as e: - raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) from e ... except IntegrityError: db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=f"Knowledge base with name '{data.name}' already exists in this namespace", - ) + ) from None except ValueError as e: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) from e🤖 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 `@backend/app/api/endpoints/knowledge.py` around lines 509 - 542, Update the exception handlers around SourceRepository.from_url/assert_user_can_read_source and knowledge_orchestrator.create_knowledge_base to chain each re-raised HTTPException from the caught exception using “from e” (or the appropriate exception variable), including the IntegrityError handler after db.rollback. Preserve the existing status codes and client-facing details.Source: Linters/SAST tools
backend/app/schemas/knowledge.py (1)
301-336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider surfacing repository info in the code-wiki response.
KnowledgeBaseResponsenever exposessource(e.g.projectName), so a client creating a code wiki gets no confirmation of which repository was bound — it's persisted in the spec but not returned. Worth exposing at leastproject_name/source_typein a follow-up if a later layer doesn't already surface it.Also applies to: 360-363, 429-430, 458-458
🤖 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 `@backend/app/schemas/knowledge.py` around lines 301 - 336, Update the code-wiki response schema and its construction paths so KnowledgeBaseResponse exposes the persisted repository binding, at minimum project_name and source_type, for code wikis. Ensure the fields are populated from the stored spec/source data across the affected response paths, while preserving existing responses for other knowledge-base types.
🤖 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 `@backend/app/services/knowledge/code_wiki_reconcile.py`:
- Line 81: Add a clear Python type annotation to the deleter parameter in the
affected function signature, matching the callable’s expected shape and the
existing annotation style for neighboring parameters.
In `@backend/app/services/knowledge/code_wiki_source.py`:
- Around line 157-219: Update the provider access-check implementations invoked
by _check_access and assert_user_can_read_source to pass an explicit bounded
timeout to every requests call for GitHub, GitLab, and Gitea. Keep the
synchronous flow intact, using a consistent timeout value or existing timeout
configuration, and ensure no provider request can wait indefinitely.
- Around line 194-211: Update the exception handling around _check_access so
SourceAccessDenied uses a generic client-facing verification message containing
only source.project_name, without interpolating the raw exc; retain exc in
logger.warning for diagnostics and preserve exception chaining.
In `@backend/app/services/knowledge/mermaid_check.py`:
- Around line 148-152: Update the closing-fence condition in the Mermaid
fence-scanning logic to require the closing marker’s length to be at least the
opening fence length, while preserving the matching character and
no-trailing-content checks. Add a regression case covering two triple-backtick
Mermaid examples inside a four-backtick non-Mermaid fence and verify neither is
checked.
- Around line 28-52: Update KNOWN_DIAGRAM_TYPES with Mermaid 11.4.0’s supported
diagram declarations, including kanban and any other declarations missing from
the allow-list, so valid diagrams from the pinned frontend version are accepted.
In `@backend/tests/services/knowledge/test_mermaid_check.py`:
- Around line 13-160: Add the missing return type annotations in
backend/tests/services/knowledge/test_mermaid_check.py: keep _fence annotated as
-> str and update every test_* function shown in the diff to return -> None,
without changing their test behavior.
---
Nitpick comments:
In `@backend/app/api/endpoints/knowledge.py`:
- Around line 509-542: Update the exception handlers around
SourceRepository.from_url/assert_user_can_read_source and
knowledge_orchestrator.create_knowledge_base to chain each re-raised
HTTPException from the caught exception using “from e” (or the appropriate
exception variable), including the IntegrityError handler after db.rollback.
Preserve the existing status codes and client-facing details.
In `@backend/app/schemas/knowledge.py`:
- Around line 301-336: Update the code-wiki response schema and its construction
paths so KnowledgeBaseResponse exposes the persisted repository binding, at
minimum project_name and source_type, for code wikis. Ensure the fields are
populated from the stored spec/source data across the affected response paths,
while preserving existing responses for other knowledge-base types.
🪄 Autofix (Beta)
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: da2e5d10-2073-4195-83d0-c96abd35ce40
📒 Files selected for processing (22)
backend/alembic/versions/20260730_c8d2e3f4a5b6_add_knowledge_content_origin_and_source_ref.pybackend/alembic/versions/20260730_d9e3f4a5b6c7_add_knowledge_document_generation_run.pybackend/app/api/endpoints/knowledge.pybackend/app/models/knowledge.pybackend/app/schemas/kind.pybackend/app/schemas/knowledge.pybackend/app/services/knowledge/code_wiki_generation_state.pybackend/app/services/knowledge/code_wiki_reconcile.pybackend/app/services/knowledge/code_wiki_run_mode.pybackend/app/services/knowledge/code_wiki_source.pybackend/app/services/knowledge/content_scope.pybackend/app/services/knowledge/knowledge_service.pybackend/app/services/knowledge/mermaid_check.pybackend/app/services/knowledge/orchestrator.pybackend/tests/api/test_knowledge_code_wiki.pybackend/tests/services/knowledge/test_code_wiki_generation_state.pybackend/tests/services/knowledge/test_code_wiki_reconcile.pybackend/tests/services/knowledge/test_code_wiki_run_mode.pybackend/tests/services/knowledge/test_code_wiki_source.pybackend/tests/services/knowledge/test_content_scope.pybackend/tests/services/knowledge/test_knowledge_base_kind.pybackend/tests/services/knowledge/test_mermaid_check.py
Comment on lines
+13
to
+160
| assert "unclosed '['" in warnings[0].message | ||
|
|
||
|
|
||
| def test_a_mismatched_closing_bracket_is_reported(): | ||
| markdown = _fence("flowchart TD\n A[Start) --> B") | ||
|
|
||
| warnings = check_mermaid_blocks(markdown) | ||
|
|
||
| assert len(warnings) == 1 | ||
| assert "mismatched" in warnings[0].message | ||
|
|
||
|
|
||
| def test_brackets_inside_quoted_labels_are_not_misread(): | ||
| markdown = _fence('flowchart TD\n A["build (release)"] --> B["ship"]') | ||
|
|
||
| assert check_mermaid_blocks(markdown) == [] | ||
|
|
||
|
|
||
| def test_an_unclosed_quote_is_reported(): | ||
| markdown = _fence('flowchart TD\n A["Start --> B') | ||
|
|
||
| warnings = check_mermaid_blocks(markdown) | ||
|
|
||
| assert len(warnings) == 1 | ||
| assert "unclosed quote" in warnings[0].message | ||
|
|
||
|
|
||
| def test_comments_before_the_diagram_type_are_skipped(): | ||
| markdown = _fence("%% layout notes\nflowchart LR\n A --> B") | ||
|
|
||
| assert check_mermaid_blocks(markdown) == [] | ||
|
|
||
|
|
||
| def test_diagram_type_with_a_direction_or_colon_is_accepted(): | ||
| assert check_mermaid_blocks(_fence("graph LR\n A --> B")) == [] | ||
| assert check_mermaid_blocks(_fence("stateDiagram-v2\n [*] --> Idle")) == [] | ||
| assert check_mermaid_blocks(_fence("pie title Share\n 'a' : 10")) == [] | ||
|
|
||
|
|
||
| def test_a_mermaid_example_quoted_inside_another_fence_is_not_checked(): | ||
| """A code sample showing broken Mermaid is documentation, not a diagram.""" | ||
| markdown = "````markdown\n```mermaid\nflowchat TD\n```\n````" | ||
|
|
||
| assert check_mermaid_blocks(markdown) == [] | ||
|
|
||
|
|
||
| def test_entity_relationship_cardinality_is_not_mistaken_for_a_bracket_error(): | ||
| """ER diagrams write cardinality as ``||--o{``, which only looks unbalanced. | ||
|
|
||
| Reporting it would send the agent off to fix a diagram that already renders. | ||
| """ | ||
| markdown = _fence( | ||
| "erDiagram\n" | ||
| " CUSTOMER ||--o{ ORDER : places\n" | ||
| " ORDER }|..|{ LINE_ITEM : contains" | ||
| ) | ||
|
|
||
| assert check_mermaid_blocks(markdown) == [] | ||
|
|
||
|
|
||
| def test_text_heavy_diagrams_may_contain_stray_brackets_in_labels(): | ||
| assert check_mermaid_blocks(_fence("gantt\n title Release [Q3\n")) == [] | ||
| assert check_mermaid_blocks(_fence("journey\n Browse [start: 3")) == [] | ||
|
|
||
|
|
||
| def test_every_diagram_in_a_page_is_checked(): | ||
| markdown = "\n\n".join( | ||
| [ | ||
| _fence("flowchart TD\n A --> B"), | ||
| _fence("nonsense\n A --> B"), | ||
| _fence("sequenceDiagram\n Alice->>Bob: hi"), | ||
| _fence("erDiagram\n A ||--o{ B : has"), | ||
| ] | ||
| ) | ||
|
|
||
| warnings = check_mermaid_blocks(markdown) | ||
|
|
||
| assert len(warnings) == 1 | ||
| assert "nonsense" in warnings[0].message | ||
|
|
||
|
|
||
| def test_warnings_carry_the_line_of_their_diagram(): | ||
| markdown = "# Title\n\nProse.\n\n" + _fence("flowchat TD\n A --> B") | ||
|
|
||
| warnings = check_mermaid_blocks(markdown) | ||
|
|
||
| assert warnings[0].line == 5 | ||
|
|
||
|
|
||
| def test_warnings_render_as_an_instruction_for_the_agent(): | ||
| warnings = check_mermaid_blocks(_fence("flowchat TD\n A --> B")) | ||
|
|
||
| described = describe_warnings(warnings) | ||
|
|
||
| assert "write the page again" in described | ||
| assert "flowchat" in described | ||
|
|
||
|
|
||
| def test_no_warnings_render_as_nothing(): | ||
| assert describe_warnings([]) == "" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add return type annotations to the helper and tests.
_fence needs -> str; every test_* function needs -> None.
Proposed pattern
-def _fence(body: str, info: str = "mermaid") -> str:
+def _fence(body: str, info: str = "mermaid") -> str:
return f"```{info}\n{body}\n```"
-def test_a_valid_diagram_produces_no_warnings():
+def test_a_valid_diagram_produces_no_warnings() -> None:As per coding guidelines, “Python code must include type hints.”
🤖 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 `@backend/tests/services/knowledge/test_mermaid_check.py` around lines 13 -
160, Add the missing return type annotations in
backend/tests/services/knowledge/test_mermaid_check.py: keep _fence annotated as
-> str and update every test_* function shown in the diff to return -> None,
without changing their test behavior.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/schemas/knowledge.py (1)
159-170: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEnforce the
kb_type/sourceinvariant onKnowledgeBaseCreate.
kb_typeandsource(Lines 159-170) are never cross-validated. Docstrings elsewhere in this PR state the invariant explicitly: a code wiki always binds to a repository, and this binding "is fixed at creation" with "deliberately no code path" to add or remove it later. Right now that invariant is enforced only by the REST endpoint's early rejection ofkb_type == CODE_WIKI(backend/app/api/endpoints/knowledge.py); the orchestrator'screate_knowledge_base, which the same docstring says is shared with MCP tools, acceptskb_typeandsourceindependently with no cross-check, andKnowledgeService.create_knowledge_basepersists whatever combination it receives.A caller reaching the orchestrator directly with
kb_type="code_wiki"and nosourcewould persist a code wiki with no repository binding, breaking downstream assumptions (generation/version-store code that readsspec.source).Add a
model_validator(mode="after")onKnowledgeBaseCreate, following the existing pattern used forvalidate_call_limitsonKnowledgeBaseSpec, enforcingkb_type == CODE_WIKIif and only ifsourceis set. This also closes the related gap where the generic REST endpoint currently just drops a client-suppliedsourceinstead of rejecting it.🛡️ Proposed fix
class KnowledgeBaseCreate(MultimodalAnalysisFieldsMixin): """Schema for creating a knowledge base.""" ... members: Optional[List[InitialMemberCreate]] = Field( None, description="Initial members to add to the knowledge base after creation", ) + `@model_validator`(mode="after") + def validate_code_wiki_source(self): + """A code wiki must have a source; other types must not.""" + is_code_wiki = self.kb_type == KnowledgeBaseType.CODE_WIKI + if is_code_wiki and not self.source: + raise ValueError("A code wiki knowledge base requires a source repository") + if not is_code_wiki and self.source: + raise ValueError( + "Only a code wiki knowledge base may specify a source repository" + ) + return self + `@field_validator`("guided_questions") `@classmethod` def validate_guided_questions(cls, v):🤖 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 `@backend/app/schemas/knowledge.py` around lines 159 - 170, Add a model_validator with mode="after" to the KnowledgeBaseCreate class that enforces the invariant between the kb_type and source fields: when kb_type equals CODE_WIKI, source must be set; when kb_type is not CODE_WIKI, source must be None. Follow the existing validation pattern used for validate_call_limits on KnowledgeBaseSpec. This ensures the invariant is enforced at the schema level rather than only at the REST endpoint, preventing invalid combinations from being persisted when the orchestrator is called directly.
🧹 Nitpick comments (5)
backend/tests/services/knowledge/test_code_wiki_version_store.py (1)
168-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a case for seeding a generation from itself.
seed_from_publishedreturnsskipped_reason="target is the published version"when the two ids match. No test covers that guard. Add one case so a later refactor cannot drop it.💚 Proposed test
+def test_a_generation_is_never_seeded_from_itself(test_db: Session): + published = _generation(test_db) + _page(test_db, published, "index") + + outcome = seed_from_published( + test_db, + target_generation_id=published.id, + published_generation_id=published.id, + ) + + assert not outcome.seeded + assert len(_paths(test_db, published.id)) == 1🤖 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 `@backend/tests/services/knowledge/test_code_wiki_version_store.py` around lines 168 - 176, Add a test alongside test_a_first_run_has_nothing_to_seed_from that calls seed_from_published with identical target_generation_id and published_generation_id, then assert the result is not seeded and has skipped_reason="target is the published version".backend/app/services/knowledge/code_wiki_run_mode.py (1)
7-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify "Both modes" now that three modes are listed.
Lines 9-12 document
SKIP,INCREMENTAL, andFULL. Line 14 then says "Both modes end with a complete snapshot". The claim applies toINCREMENTALandFULLonly, becauseSKIPcreates no version. Name the two modes explicitly.📝 Proposed wording fix
-Both modes end with a complete snapshot, so **publishing does not depend on the mode**. +``INCREMENTAL`` and ``FULL`` both end with a complete snapshot, so **publishing does +not depend on the mode**.🤖 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 `@backend/app/services/knowledge/code_wiki_run_mode.py` around lines 7 - 22, Update the explanatory text around the mode descriptions so the complete-snapshot statement explicitly refers to INCREMENTAL and FULL, excluding SKIP because it creates no version. Preserve the existing publishing behavior and surrounding rationale.backend/app/services/knowledge/code_wiki_version_store.py (1)
190-202: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLoad only the columns needed to match a page path.
The query materializes every
WikiContentrow of the generation, including the fullcontentbody, to readext. For a large wiki this loads the whole snapshot per removal. Selectidandextonly, then delete by id.♻️ Proposed refactor
- for content in ( - db.query(WikiContent).filter(WikiContent.generation_id == generation_id).all() - ): - if collation_key(page_path_of(content)) == wanted: - db.delete(content) - db.flush() + rows = ( + db.query(WikiContent.id, WikiContent.ext) + .filter(WikiContent.generation_id == generation_id) + .all() + ) + for content_id, ext in rows: + if collation_key(str((ext or {}).get(PATH_EXT_KEY, "") or "")) == wanted: + db.query(WikiContent).filter(WikiContent.id == content_id).delete( + synchronize_session=False + ) + db.flush() logger.info( "[code_wiki] removed page '%s' from generation %s", normalized, generation_id, ) return True return False🤖 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 `@backend/app/services/knowledge/code_wiki_version_store.py` around lines 190 - 202, Update the page-removal query in the function containing the generation_id loop to select only WikiContent.id and WikiContent.ext, rather than materializing full WikiContent rows. Match the requested page path using the selected ext value, then delete the matching record by its id and preserve the existing flush, logging, and return behavior.backend/app/services/wiki_service.py (1)
605-659: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the new path-resolution logic into a helper.
The new path validation and matching logic (605-651) adds roughly 55 lines to
save_generation_contents, which was already well over the "functions should remain focused, preferably under 50 lines" guideline before this change. Extracting the per-section path/title resolution (642-651, plus the index-building step) into a dedicated helper (for example_resolve_content_item(section, path, existing_by_path, existing_by_key, existing_by_title)) would keep the new logic testable in isolation and shrink the surrounding method.As per coding guidelines: "functions should remain focused, preferably under 50 lines."
🤖 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 `@backend/app/services/wiki_service.py` around lines 605 - 659, Extract the path normalization, uniqueness validation, existing-content index construction, and per-section matching from save_generation_contents into a focused helper, such as _resolve_content_item or a small resolution helper pair. Pass the section, normalized path, and existing_by_path, existing_by_key, and existing_by_title indexes to the resolver, preserving path-first matching and the legacy type/title fallback; keep save_generation_contents focused on orchestration and updates.Source: Coding guidelines
backend/app/models/knowledge.py (1)
150-157: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a supporting index for the new
origincolumns.The new
origincolumn onKnowledgeDocumentandKnowledgeFolderhas no index anywhere in this changeset, even though it is now filtered on every knowledge-base document listing and every generated-page reconciliation call (via the new content-ownership scope helpers). The migration creates an index for the newwiki_generations.kind_idcolumn but not for eitherorigincolumn, and the ORM models don't declare one either.
backend/app/models/knowledge.py#L150-L157: declare an index forKnowledgeDocument.origin, ideally a composite(kind_id, origin)index matching the actual filter pattern.backend/app/models/knowledge.py#L237-L244: declare the same kind of index forKnowledgeFolder.origin.backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py#L37-L56: add the correspondingop.create_indexcalls for both tables so the database index matches the model declaration.🤖 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 `@backend/app/models/knowledge.py` around lines 150 - 157, Add database indexes for the new origin column across three coordinated sites to support the filtering that occurs on every knowledge-base document listing and generated-page reconciliation call. In backend/app/models/knowledge.py at lines 150-157, declare a composite index on the origin Column for KnowledgeDocument combining kind_id and origin to match the actual filter pattern. In backend/app/models/knowledge.py at lines 237-244, declare the same kind of composite index for the origin Column on KnowledgeFolder. In the migration file backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py at lines 37-56, add corresponding op.create_index calls for both the KnowledgeDocument and KnowledgeFolder origin indexes so the database schema matches the ORM model declarations.
🤖 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
`@backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py`:
- Around line 29-30: In the migration file
20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py, add the
revision and down_revision declarations at the module level if they are missing.
Set revision to "bd9c871a93d2" and down_revision to "f7a8b9c0d1e2" to properly
connect this migration to the Alembic upgrade chain so it integrates correctly
with the existing migration history.
In `@backend/app/services/knowledge/code_wiki_page_path.py`:
- Around line 96-113: Re-validate the leaf in the path-normalization logic after
removing MARKDOWN_SUFFIX, using the same relative-segment rule that rejects "."
and "..". Ensure values such as "..md" and "...md" raise InvalidPagePath before
updating segments or constructing the normalized path, while preserving the
existing empty-filename validation.
In `@backend/app/services/knowledge/code_wiki_version_store.py`:
- Around line 291-303: Update the unsuccessful-generation filter in
backend/app/services/knowledge/code_wiki_version_store.py lines 291-303 to
exclude IN_FLIGHT_STATUSES, so only terminal generations are retained for
deletion. Add a test in
backend/tests/services/knowledge/test_code_wiki_version_store.py lines 360-381
creating an old RUNNING generation and assert apply_retention does not return
its id.
- Around line 68-71: Update _as_naive_utc so aware datetimes are first converted
to UTC, then have their tzinfo removed; preserve naive values unchanged and
continue returning _utcnow() for None. Ensure reclaim_stale_generations and
apply_retention receive correctly normalized UTC-naive timestamps.
- Around line 223-231: Scope wiki generation cleanup to the owning user by
adding a user/project owner parameter to reclaim_stale_generations() and
apply_retention(). Include that parameter in every WikiGeneration query
alongside kind_id and status filters, and update all callers to pass the
appropriate user_id so stale-generation reclamation and retention cannot affect
other users.
In `@backend/app/services/wiki_service.py`:
- Around line 610-616: Update the InvalidPagePath handler in the surrounding
wiki service method to call wiki_db.rollback() before raising the HTTPException,
and explicitly chain the original exception with from exc. Preserve the existing
400 status and detail message.
- Around line 628-651: Restrict the legacy title-based fallback indices in the
section update flow to path-less content only. When building existing_by_key and
existing_by_title, exclude any item for which page_path_of(content) returns a
path; keep existing_by_path unchanged so path-identified writes still resolve
normally.
In `@backend/tests/services/knowledge/test_code_wiki_version_store.py`:
- Around line 309-324: Update
test_the_published_version_survives_a_run_of_failures so each generation created
in the five-iteration loop explicitly uses WikiGenerationStatus.FAILED, matching
the test name and docstring while preserving the existing retention assertions.
---
Outside diff comments:
In `@backend/app/schemas/knowledge.py`:
- Around line 159-170: Add a model_validator with mode="after" to the
KnowledgeBaseCreate class that enforces the invariant between the kb_type and
source fields: when kb_type equals CODE_WIKI, source must be set; when kb_type
is not CODE_WIKI, source must be None. Follow the existing validation pattern
used for validate_call_limits on KnowledgeBaseSpec. This ensures the invariant
is enforced at the schema level rather than only at the REST endpoint,
preventing invalid combinations from being persisted when the orchestrator is
called directly.
---
Nitpick comments:
In `@backend/app/models/knowledge.py`:
- Around line 150-157: Add database indexes for the new origin column across
three coordinated sites to support the filtering that occurs on every
knowledge-base document listing and generated-page reconciliation call. In
backend/app/models/knowledge.py at lines 150-157, declare a composite index on
the origin Column for KnowledgeDocument combining kind_id and origin to match
the actual filter pattern. In backend/app/models/knowledge.py at lines 237-244,
declare the same kind of composite index for the origin Column on
KnowledgeFolder. In the migration file
backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py
at lines 37-56, add corresponding op.create_index calls for both the
KnowledgeDocument and KnowledgeFolder origin indexes so the database schema
matches the ORM model declarations.
In `@backend/app/services/knowledge/code_wiki_run_mode.py`:
- Around line 7-22: Update the explanatory text around the mode descriptions so
the complete-snapshot statement explicitly refers to INCREMENTAL and FULL,
excluding SKIP because it creates no version. Preserve the existing publishing
behavior and surrounding rationale.
In `@backend/app/services/knowledge/code_wiki_version_store.py`:
- Around line 190-202: Update the page-removal query in the function containing
the generation_id loop to select only WikiContent.id and WikiContent.ext, rather
than materializing full WikiContent rows. Match the requested page path using
the selected ext value, then delete the matching record by its id and preserve
the existing flush, logging, and return behavior.
In `@backend/app/services/wiki_service.py`:
- Around line 605-659: Extract the path normalization, uniqueness validation,
existing-content index construction, and per-section matching from
save_generation_contents into a focused helper, such as _resolve_content_item or
a small resolution helper pair. Pass the section, normalized path, and
existing_by_path, existing_by_key, and existing_by_title indexes to the
resolver, preserving path-first matching and the legacy type/title fallback;
keep save_generation_contents focused on orchestration and updates.
In `@backend/tests/services/knowledge/test_code_wiki_version_store.py`:
- Around line 168-176: Add a test alongside
test_a_first_run_has_nothing_to_seed_from that calls seed_from_published with
identical target_generation_id and published_generation_id, then assert the
result is not seeded and has skipped_reason="target is the published version".
🪄 Autofix (Beta)
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: 81aa1185-9f28-4007-90b4-eb57d2e497f8
📒 Files selected for processing (20)
backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.pybackend/app/api/endpoints/knowledge.pybackend/app/models/knowledge.pybackend/app/models/wiki.pybackend/app/schemas/kind.pybackend/app/schemas/knowledge.pybackend/app/schemas/wiki.pybackend/app/services/knowledge/code_wiki_page_path.pybackend/app/services/knowledge/code_wiki_run_mode.pybackend/app/services/knowledge/code_wiki_version_store.pybackend/app/services/knowledge/content_scope.pybackend/app/services/knowledge/knowledge_service.pybackend/app/services/knowledge/orchestrator.pybackend/app/services/wiki_service.pybackend/tests/api/test_knowledge_code_wiki.pybackend/tests/services/knowledge/test_code_wiki_content_write.pybackend/tests/services/knowledge/test_code_wiki_page_path.pybackend/tests/services/knowledge/test_code_wiki_run_mode.pybackend/tests/services/knowledge/test_code_wiki_version_store.pybackend/tests/services/knowledge/test_content_scope.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/services/knowledge/content_scope.py
- backend/tests/services/knowledge/test_code_wiki_run_mode.py
Comment on lines
+223
to
+231
| stale: Sequence[WikiGeneration] = ( | ||
| db.query(WikiGeneration) | ||
| .filter( | ||
| WikiGeneration.kind_id == kind_id, | ||
| WikiGeneration.status.in_(IN_FLIGHT_STATUSES), | ||
| WikiGeneration.updated_at < cutoff, | ||
| ) | ||
| .all() | ||
| ) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how callers resolve kind_id and whether user scoping is applied elsewhere.
rg -n -C 8 'reclaim_stale_generations|apply_retention' backend --glob '!**/tests/**'
rg -n -C 4 'WikiGeneration.user_id' backend/appRepository: wecode-ai/Wegent
Length of output: 7704
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## repo files around wiki"
git ls-files backend/app/services/knowledge backend/app/api/endpoints wiki | sed -n '1,200p'
echo "## WikiGeneration definitions/usages"
rg -n -C 5 'class WikiGeneration|WikiGeneration|kind_id|user_id|namespace|name' backend/app/services/knowledge backend/app/api/endpoints/wiki.py backend/app --glob '!**/tests/**' | sed -n '1,260p'
echo "## service file relevant sections"
sed -n '200,320p' backend/app/services/knowledge/code_wiki_version_store.py | cat -n
echo "## wiki endpoint relevant sections"
sed -n '320,395p' backend/app/api/endpoints/wiki.py | cat -nRepository: wecode-ai/Wegent
Length of output: 33423
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## imports and schedule/retention callouts"
rg -n -C 12 'from.*code_wiki_version_store import|code_wiki_version_store\.(reclaim_stale_generations|apply_retention)|reclaim_stale_generations\(|apply_retention\(' backend --glob '!**/tests/**'
echo "## model files"
git ls-files backend/app/models | sed -n '1,200p'
rg -n -C 8 'class WikiGeneration|kind_id|user_id|Project|ProjectType|WikiProject|WikiGeneration' backend/app/models --glob '*wiki*.py'
echo "## wiki_service relevant generation methods"
sed -n '350,580p' backend/app/services/wiki_service.py | cat -n
sed -n '760,850p' backend/app/services/wiki_service.py | cat -nRepository: wecode-ai/Wegent
Length of output: 27811
Scope wiki generation cleanup by user_id.
WikiGeneration rows are per-user for user_id, but reclaim_stale_generations() and apply_retention() filter only by kind_id and status. If the same knowledge base can be referenced by multiple users, retention can mutate another user’s in-flight or retained generations. Pass the owner or project-scoped user filter into both functions and include it in every generation query.
🤖 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 `@backend/app/services/knowledge/code_wiki_version_store.py` around lines 223 -
231, Scope wiki generation cleanup to the owning user by adding a user/project
owner parameter to reclaim_stale_generations() and apply_retention(). Include
that parameter in every WikiGeneration query alongside kind_id and status
filters, and update all callers to pass the appropriate user_id so
stale-generation reclamation and retention cannot affect other users.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/tests/services/knowledge/test_code_wiki_source.py (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd return type annotations.
Add
-> Noneto the test functions. Add an explicit return type to_granted, such asdict[str, object].As per coding guidelines, “Python code must follow PEP 8, Black with 88-column formatting, isort, and type hints.”
Also applies to: 29-29, 51-51, 72-72, 81-81, 102-102, 121-121, 126-126, 142-142, 150-150, 160-160, 165-165, 175-175, 179-179
🤖 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 `@backend/tests/services/knowledge/test_code_wiki_source.py` at line 25, Add explicit return type annotations to the helper _granted, using dict[str, object], and to every test function in this file, using -> None. Update all referenced functions consistently without changing their behavior.Source: Coding guidelines
backend/app/api/endpoints/knowledge.py (1)
515-542: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
IntegrityError/ValueErrorhandling across two endpoints.The
IntegrityErrorandValueErrorhandling increate_code_wiki(lines 535-542) repeats the same mapping already present increate_knowledge_base(lines 477-487): both mapIntegrityErrorto a 400 "already exists" response andValueErrorto a 400 response withstr(e). Extract a small shared helper (e.g. a context manager or a wrapper function) that both endpoints call, so the mapping is defined once.As per coding guidelines, "Before adding code, search for and reuse existing components, services, utilities, and patterns; extract shared logic instead of duplicating it."
🤖 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 `@backend/app/api/endpoints/knowledge.py` around lines 515 - 542, The duplicate IntegrityError and ValueError-to-HTTPException mappings in create_knowledge_base and create_code_wiki should be centralized. Extract a small shared helper or context manager for this exception handling, then update both endpoint functions to use it while preserving the rollback and existing response details.Source: Coding guidelines
🤖 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 `@backend/app/api/endpoints/knowledge.py`:
- Around line 509-542: Update all exception translations in create_code_wiki to
explicitly chain their original exceptions: add from e to the SourceAccessDenied
and ValueError HTTPException raises, and chain or explicitly suppress the
IntegrityError after db.rollback() according to the desired traceback behavior.
Ensure all three except blocks preserve intentional exception context.
In `@backend/tests/services/knowledge/test_code_wiki_source.py`:
- Line 47: Update the credential setup in both the affected test and
test_gitlab_is_identified_by_project_id: store the mocked credentials, including
the token, in a local credentials variable, return that variable from the
relevant patch, and pass credentials["token"] to the token argument in
assertions or calls instead of a string literal.
- Around line 72-78: Add parameterized cases to
test_missing_credentials_are_denied for get_user_git_info returning {}, {"type":
"github"}, and a record with an empty token, while preserving the existing None
case. Each input must continue asserting SourceAccessDenied with the “No
credentials configured” message through assert_user_can_read_source.
---
Nitpick comments:
In `@backend/app/api/endpoints/knowledge.py`:
- Around line 515-542: The duplicate IntegrityError and
ValueError-to-HTTPException mappings in create_knowledge_base and
create_code_wiki should be centralized. Extract a small shared helper or context
manager for this exception handling, then update both endpoint functions to use
it while preserving the rollback and existing response details.
In `@backend/tests/services/knowledge/test_code_wiki_source.py`:
- Line 25: Add explicit return type annotations to the helper _granted, using
dict[str, object], and to every test function in this file, using -> None.
Update all referenced functions consistently without changing their behavior.
🪄 Autofix (Beta)
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: 2c41b6e0-48cd-4a17-afd2-f28f698f2efe
📒 Files selected for processing (24)
backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.pybackend/app/api/endpoints/knowledge.pybackend/app/models/knowledge.pybackend/app/models/wiki.pybackend/app/schemas/kind.pybackend/app/schemas/knowledge.pybackend/app/schemas/wiki.pybackend/app/services/knowledge/code_wiki_page_path.pybackend/app/services/knowledge/code_wiki_run_mode.pybackend/app/services/knowledge/code_wiki_source.pybackend/app/services/knowledge/code_wiki_version_store.pybackend/app/services/knowledge/content_scope.pybackend/app/services/knowledge/knowledge_service.pybackend/app/services/knowledge/mermaid_check.pybackend/app/services/knowledge/orchestrator.pybackend/app/services/wiki_service.pybackend/tests/api/test_knowledge_code_wiki.pybackend/tests/services/knowledge/test_code_wiki_content_write.pybackend/tests/services/knowledge/test_code_wiki_page_path.pybackend/tests/services/knowledge/test_code_wiki_run_mode.pybackend/tests/services/knowledge/test_code_wiki_source.pybackend/tests/services/knowledge/test_code_wiki_version_store.pybackend/tests/services/knowledge/test_content_scope.pybackend/tests/services/knowledge/test_mermaid_check.py
🚧 Files skipped from review as they are similar to previous changes (21)
- backend/app/services/knowledge/knowledge_service.py
- backend/tests/services/knowledge/test_code_wiki_page_path.py
- backend/app/models/knowledge.py
- backend/tests/api/test_knowledge_code_wiki.py
- backend/tests/services/knowledge/test_code_wiki_version_store.py
- backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py
- backend/app/models/wiki.py
- backend/app/services/knowledge/code_wiki_page_path.py
- backend/tests/services/knowledge/test_code_wiki_content_write.py
- backend/app/services/knowledge/code_wiki_version_store.py
- backend/app/services/knowledge/code_wiki_source.py
- backend/app/schemas/kind.py
- backend/app/services/knowledge/orchestrator.py
- backend/app/services/knowledge/content_scope.py
- backend/tests/services/knowledge/test_code_wiki_run_mode.py
- backend/app/schemas/wiki.py
- backend/tests/services/knowledge/test_mermaid_check.py
- backend/tests/services/knowledge/test_content_scope.py
- backend/app/schemas/knowledge.py
- backend/app/services/knowledge/code_wiki_run_mode.py
- backend/app/services/knowledge/mermaid_check.py
|
|
||
| assert result["has_access"] is True | ||
| provider.check_user_project_access.assert_called_once_with( | ||
| token="t0ken", git_domain="github.com", repo_name="wecode-ai/Wegent" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Resolve the Ruff S106 errors.
Lines 47 and 68 pass a string literal to the security-sensitive token argument. Store the mocked credentials in a local variable, use that variable as the patch return value, and assert with token=credentials["token"].
Proposed fix
def test_access_granted_returns_provider_details():
+ credentials = {"type": "github", "token": "t0ken"}
provider = MagicMock()
provider.check_user_project_access.return_value = _granted()
with (
patch(
"app.services.knowledge.code_wiki_source.get_user_git_info",
- return_value={"type": "github", "token": "t0ken"},
+ return_value=credentials,
),
...
):
result = assert_user_can_read_source(MagicMock(), 1, GITHUB_SOURCE)
provider.check_user_project_access.assert_called_once_with(
- token="t0ken", git_domain="github.com", repo_name="wecode-ai/Wegent"
+ token=credentials["token"],
+ git_domain="github.com",
+ repo_name="wecode-ai/Wegent",
)Apply the same change to test_gitlab_is_identified_by_project_id. Based on static analysis, Ruff reports S106 at these lines.
Also applies to: 68-68
🧰 Tools
🪛 Ruff (0.16.0)
[error] 47-47: Possible hardcoded password assigned to argument: "token"
(S106)
🤖 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 `@backend/tests/services/knowledge/test_code_wiki_source.py` at line 47, Update
the credential setup in both the affected test and
test_gitlab_is_identified_by_project_id: store the mocked credentials, including
the token, in a local credentials variable, return that variable from the
relevant patch, and pass credentials["token"] to the token argument in
assertions or calls instead of a string literal.
Source: Linters/SAST tools
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/repository/github_provider.py (1)
25-29: 🩺 Stability & Availability | 🔵 TrivialVerify the complete access-check deadline.
The new timeout limits individual HTTP requests. It does not limit the complete access-check operation, which performs multiple sequential requests.
backend/app/repository/github_provider.py#L25-L29: Verify the GitHub endpoint budget and use a shared deadline if the complete check must finish within 15 seconds.backend/app/repository/gitlab_provider.py#L23-L27: Verify the GitLab endpoint budget and use a shared deadline if the complete check must finish within 15 seconds.🤖 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 `@backend/app/repository/github_provider.py` around lines 25 - 29, Verify the complete access-check deadline in backend/app/repository/github_provider.py lines 25-29 and backend/app/repository/gitlab_provider.py lines 23-27: inspect the endpoint budgets and update the access-check methods to use one shared 15-second deadline across all sequential requests, rather than applying the timeout independently to each request.
🤖 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 `@backend/app/repository/github_provider.py`:
- Around line 867-871: Validate that the repository’s source_domain matches an
explicitly configured GitHub or GitLab host before any token-check request is
sent. Apply this guard to the GitHub requests around user and repository access
checks, the GitLab access-check requests, and before _get_api_base_url()
constructs custom API URLs; reject unconfigured hosts without sending
decrypt_token.
---
Nitpick comments:
In `@backend/app/repository/github_provider.py`:
- Around line 25-29: Verify the complete access-check deadline in
backend/app/repository/github_provider.py lines 25-29 and
backend/app/repository/gitlab_provider.py lines 23-27: inspect the endpoint
budgets and update the access-check methods to use one shared 15-second deadline
across all sequential requests, rather than applying the timeout independently
to each request.
🪄 Autofix (Beta)
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: f8013f06-1e6d-46d6-8032-d0653a22d707
📒 Files selected for processing (5)
backend/app/repository/github_provider.pybackend/app/repository/gitlab_provider.pybackend/app/services/knowledge/code_wiki_source.pybackend/app/services/knowledge/mermaid_check.pybackend/tests/services/knowledge/test_mermaid_check.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/services/knowledge/code_wiki_source.py
- backend/tests/services/knowledge/test_mermaid_check.py
Comment on lines
+867
to
+871
| user_response = requests.get( | ||
| f"{api_base_url}/user", | ||
| headers=headers, | ||
| timeout=ACCESS_CHECK_TIMEOUT_SECONDS, | ||
| ) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'source\.source_domain|git_domain|check_user_project_access|_get_api_base_url|build_url|allowlist|allowed.*host|https' \
backend/app/services/knowledge/code_wiki_source.py \
backend/app/repository/github_provider.py \
backend/app/repository/gitlab_provider.py || true
rg -n -C 8 'def build_url|build_url\(' backend || trueRepository: wecode-ai/Wegent
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files 'backend/app/services/knowledge/code_wiki_source.py' 'backend/app/repository/github_provider.py' 'backend/app/repository/gitlab_provider.py' 'shared/utils/url_util.py' 'backend/app/models/user.py' | rg '(code_wiki_source\.py|github_provider\.py|gitlab_provider\.py|url_util\.py|user\.py)$' || true
echo
echo "== code_wiki_source.py key sections =="
wc -l backend/app/services/knowledge/code_wiki_source.py
sed -n '1,220p' backend/app/services/knowledge/code_wiki_source.py
echo
sed -n '221,420p' backend/app/services/knowledge/code_wiki_source.py
echo
echo "== url_util.py =="
wc -l shared/utils/url_util.py
cat -n shared/utils/url_util.py
echo
echo "== focused provider access-check sections =="
sed -n '820,950p' backend/app/repository/github_provider.py
echo
sed -n '26,45p' backend/app/repository/gitlab_provider.py
sed -n '850,950p' backend/app/repository/gitlab_provider.py
echo
echo "== model/user git_info schema references =="
rg -n -C 6 'git_info|GitInfo|source_domain|source_type|source_url' backend/app/models backend/app/schemas backend/app/services/knowledge/code_wiki_source.py backend/app/repository/github_provider.py backend/app/repository/gitlab_provider.pyRepository: wecode-ai/Wegent
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== parse_repo_url implementations =="
rg -n -C 12 'def parse_repo_url|parse_repo_url\(' backend/app/services/git_skill backend/app/services || true
echo
echo "== git_info validation/creation calls =="
rg -n -C 6 'git_info=|create|add|token|decrypt|git_domain|parse_repo_url|check_user_project_access|assert_user_can_read_source|SourceRepository\.from_url|source\.source_domain\s*=' backend/app services shared || true
echo
echo "== gitlab provider api helper/base url sections =="
sed -n '95,125p' backend/app/repository/gitlab_provider.py
sed -n '95,123p' backend/app/repository/github_provider.py
echo
echo "== focused auth retry request section =="
rg -n -C 10 '_make_request_with_auth_retry|requests\.(get|put|patch|post|delete)\(' backend/app/repository/gitlab_provider.pyRepository: wecode-ai/Wegent
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import sys
for path in [
"backend/app/services/git_skill/utils.py",
"backend/app/services/git_skill/__init__.py",
"backend/app/services/git_skill/*.py",
]:
pass
PY
rg -n --glob 'backend/app/services/git_skill/**.py' \
'def parse_repo_url|parse_repo_url\(|def parse_git|git_domain|source_domain|SourceRepository' backend/app/services/git_skill || true
rg -n --glob 'backend/app/services/git_skill/**.py' \
'github\.com|gitlab\.com|GitHubEnterprise|custom|allowlist|allowed|valid' backend/app/services/git_skill || trueRepository: wecode-ai/Wegent
Length of output: 2399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/app/services/git_skill/utils.py parse_repo_url and git_info sections =="
sed -n '1,230p' backend/app/services/git_skill/utils.py
echo
sed -n '230,330p' backend/app/services/git_skill/utils.py
echo
echo "== repository provider _get_api_base_url full sections =="
sed -n '36,110p' backend/app/repository/github_provider.py
echo
sed -n '36,110p' backend/app/repository/gitlab_provider.pyRepository: wecode-ai/Wegent
Length of output: 16309
Enforce the configured Git hosts before sending repository tokens.
SourceRepository.from_url() derives source_domain from any https://host/owner/repo.git URL, but the provider access checks send decrypt_token to URLs built from that host. No allowlist in parse_repo_url() prevents an arbitrary custom source_domain, so the GitHub/GitLab token-check requests can send git_info tokens to an attacker-controlled host. Require an explicitly configured GitHub/GitLab host, and add the same validation before _get_api_base_url() builds custom API URLs.
📍 Affects 2 files
backend/app/repository/github_provider.py#L867-L871(this comment)backend/app/repository/github_provider.py#L889-L892backend/app/repository/github_provider.py#L934-L937backend/app/repository/gitlab_provider.py#L898-L903backend/app/repository/gitlab_provider.py#L925-L930
🤖 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 `@backend/app/repository/github_provider.py` around lines 867 - 871, Validate
that the repository’s source_domain matches an explicitly configured GitHub or
GitLab host before any token-check request is sent. Apply this guard to the
GitHub requests around user and repository access checks, the GitLab
access-check requests, and before _get_api_base_url() constructs custom API
URLs; reject unconfigured hosts without sending decrypt_token.
Source: Linters/SAST tools
Collaborator
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/wiki_service.py (1)
563-823: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit
save_generation_contentsto meet the 50-line function guideline.
save_generation_contentsspans roughly 260 lines, covering payload validation, path normalization, section upsert, removal application, and code-wiki status deferral. This PR's own additions (path identity handling,_apply_removalsintegration, deferred code-wiki status) grew the method further.The same extraction pattern already applied to
_apply_removalsand_finish_code_wikiin this PR can be extended to the rest of the method: for example, extract the section-upsert block (611-696) and the status-transition block (748-796) into private helpers. This keeps each unit testable and readable on its own.As per coding guidelines,
**/*.{py,ts,tsx,js,jsx}: "functions should remain focused, preferably under 50 lines."🤖 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 `@backend/app/services/wiki_service.py` around lines 563 - 823, Split save_generation_contents into focused private helpers, extracting at least the section validation/path normalization and upsert flow, plus the summary-driven status transition and code-wiki deferral logic. Keep save_generation_contents responsible for orchestration, removals, metadata, commit, and post-commit completion while preserving existing behavior and data passed between helpers.Source: Coding guidelines
🧹 Nitpick comments (6)
backend/tests/services/knowledge/test_code_wiki_generation.py (1)
80-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd type hints to the two helpers.
_write_pagehas no return annotation, and_publish_a_first_wikihas no annotations at all. The rest of the file is annotated, so these two are the exception.-def _write_page(test_db: Session, generation: WikiGeneration, path: str, body: str): +def _write_page( + test_db: Session, generation: WikiGeneration, path: str, body: str +) -> None: ... -def _publish_a_first_wiki(test_db, knowledge_base, test_user, effects, *, pages=3): +def _publish_a_first_wiki( + test_db: Session, + knowledge_base: Kind, + test_user: User, + effects: FakeEffects, + *, + pages: int = 3, +) -> WikiGeneration:As per coding guidelines: "Python code must follow PEP 8, Black with 88-column formatting, isort, and type hints".
🤖 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 `@backend/tests/services/knowledge/test_code_wiki_generation.py` around lines 80 - 112, Add complete type annotations to the helper functions _write_page and _publish_a_first_wiki, including their parameters and return types; annotate _write_page as returning None and use the existing concrete types for the database session, generation, knowledge base, user, and effects parameters consistently with the rest of the file.Source: Coding guidelines
backend/app/services/knowledge/code_wiki_side_effects.py (1)
129-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo adapters depend on private symbols of other modules.
knowledge_orchestrator._schedule_indexing_celeryandknowledge_service._run_async_in_new_loopare both underscore-prefixed. A rename inside either module breaks the publish path with no signal at import time in the module that owns them.Promote the two helpers to public names in their owning modules, then call the public names here. This keeps the interface between the projection adapters and the knowledge services explicit.
As per coding guidelines: "Favor cohesive modules, explicit interfaces, and standard practices".
🤖 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 `@backend/app/services/knowledge/code_wiki_side_effects.py` around lines 129 - 145, Promote knowledge_orchestrator._schedule_indexing_celery and knowledge_service._run_async_in_new_loop to public helper names in their owning modules, then update this adapter’s scheduling call and _run function to use those public names. Preserve the existing arguments and execution behavior while removing cross-module references to the underscore-prefixed symbols.Source: Coding guidelines
backend/tests/services/knowledge/test_code_wiki_projection_plan.py (1)
77-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: add a case-collision test for two desired pages.
test_matching_ignores_case_because_the_database_doescovers one desired page against one existing page. It does not cover two desired pages whose paths differ only by case, which both map to one collation key. That input produces two plan items for one document. A test would pin the intended behaviour, whatever the write path guarantees.🤖 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 `@backend/tests/services/knowledge/test_code_wiki_projection_plan.py` around lines 77 - 84, Extend test_matching_ignores_case_because_the_database_does, or add a focused test beside it, with two desired pages whose paths differ only by case and the same existing page. Assert the resulting projection plan reflects the intended write-path behavior for the shared case-insensitive collation key, including skips and deletes as appropriate.backend/app/services/knowledge/code_wiki_schedule.py (1)
40-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ScheduleTooFrequentis raised for errors that are not about frequency.The class docstring says the error means a schedule runs too often. The module raises it for an unsupported interval unit at Line 77, an invalid cron expression at Line 106, a missing cron expression at Line 162, a missing interval value at Line 167, event triggers at Line 172, and an unknown trigger type at Line 178. An API layer that maps this exception to a frequency message will report the wrong cause for those cases.
Consider a base
InvalidSchedule(ValueError)withScheduleTooFrequentas a subclass for the frequency case only. Callers that catch the base keep working, and the message stays accurate.Also applies to: 75-78, 105-106, 161-178
🤖 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 `@backend/app/services/knowledge/code_wiki_schedule.py` around lines 40 - 41, Introduce an InvalidSchedule(ValueError) base exception for general schedule validation failures, make ScheduleTooFrequent inherit from it, and update the non-frequency validation paths in the schedule creation logic—including invalid interval units, cron expressions, missing values, event triggers, and unknown trigger types—to raise InvalidSchedule instead. Keep ScheduleTooFrequent exclusively for schedules that exceed the allowed frequency.backend/app/services/knowledge/code_wiki_publisher.py (1)
243-262: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: read the superseded documents in one query.
_attachments_being_replacedcallsdb.getonce per touched document. A full rebuild of a large wiki makes one round trip per page. A singleINquery over the collected ids would do the same work in one statement.db.getcan serve from the identity map, so the current cost is often lower than it looks; treat this as optional.🤖 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 `@backend/app/services/knowledge/code_wiki_publisher.py` around lines 243 - 262, Optionally optimize _attachments_being_replaced by collecting the unique touched document IDs and loading all matching KnowledgeDocument rows with one IN query instead of calling db.get for each ID. Preserve the existing attachment_id and converted_attachment_id collection behavior, including skipping missing documents and returning the same tuple format.backend/tests/services/knowledge/test_code_wiki_content_write.py (1)
255-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for valid sections combined with a malformed removal path.
The current tests cover malformed removal paths alone (255-261) and valid write+remove together (264-277), but not a payload combining valid sections with an invalid removal path. Per
_apply_removalsinwiki_service.py, that combination rolls back the entire transaction, including the already-flushed sections. A test locking in this atomicity guarantee would catch a regression if the rollback behavior changes.def test_valid_sections_are_rolled_back_with_a_malformed_removal( test_db: Session, generation: WikiGeneration ): with pytest.raises(HTTPException) as exc: WikiService().save_generation_contents( test_db, WikiContentWriteRequest( generation_id=generation.id, sections=[_section("new-page", "New Page")], removed_paths=["../escape"], ), ) assert exc.value.status_code == 400 assert _pages(test_db, generation.id) == []🤖 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 `@backend/tests/services/knowledge/test_code_wiki_content_write.py` around lines 255 - 277, Add a test near test_a_malformed_removal_path_is_refused and test_a_page_written_and_removed_in_one_payload_ends_up_removed that calls WikiService().save_generation_contents with a valid sections entry and malformed removed_paths value "../escape". Assert it raises HTTPException with status code 400, then verify via _pages that no section was persisted, locking in transaction-wide rollback.
🤖 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 `@backend/app/api/endpoints/knowledge.py`:
- Around line 600-603: Update both HTTPException raises in start_code_wiki_run,
within the GenerationInFlight and CodeWikiRunError handlers, to explicitly chain
each raised exception from its caught exception e. Preserve the existing status
codes and details while retaining the original traceback.
In `@backend/app/schemas/knowledge.py`:
- Around line 336-344: Update the change_type field in CodeWikiChangedPath to
use Literal["A", "M", "D", "R"] while preserving its default value and
description, so unsupported status letters are rejected during schema
validation.
In `@backend/app/services/knowledge/code_wiki_generation.py`:
- Around line 127-141: Update start_generation’s concurrency guard around the
WikiGeneration in_flight query so it durably serializes starts for each
knowledge_base.kind_id: lock the owning Kind row before checking active
generations, or enforce an equivalent composite/partial uniqueness constraint
for non-terminal generations. Preserve the existing GenerationInFlight behavior
while ensuring concurrent calls cannot both insert RUNNING generations or seed
the wiki twice.
In `@backend/app/services/knowledge/code_wiki_projection.py`:
- Around line 197-210: Update the missing-document branch in the projection loop
to create and append a new KnowledgeDocument using the update’s source data and
attachment, rather than logging and continuing. Ensure it is included in the
published result and tracked consistently with added documents, and keep the
warning message aligned with this create-as-added behavior.
In `@backend/app/services/knowledge/code_wiki_publish_gate.py`:
- Around line 106-117: The publish gate must calculate removal from deleted
published paths rather than page-count differences. In
backend/app/services/knowledge/code_wiki_publish_gate.py lines 106-117, use
plan.deletes from compute_projection_plan to derive removed_share while
preserving the existing threshold verdict and messaging. In
backend/tests/services/knowledge/test_code_wiki_publish_gate.py lines 45-56, add
coverage for a same-count version that replaces most paths and assert the
intended gate verdict.
In `@backend/app/services/knowledge/code_wiki_publisher.py`:
- Around line 7-19: Update the docstring for publish_generation to match its
implementation: describe advancing spec.publishedGenerationId within the
transaction before commit, followed by post-commit cleanup. Preserve the
documented sequence and clarify that the pointer update occurs before the
transaction commits, not afterward.
- Around line 265-311: Normalize every ref to a canonical string in
_park_unfinished_cleanup before membership checks and storage, matching
retry_pending_index_cleanup. In retry_pending_index_cleanup, validate or safely
parse each ref before calling effects.delete_rag_document; treat non-numeric
refs as invalid and remove them from the pending list instead of retaining them
for endless retries.
In `@backend/init_data/02-public-resources.yaml`:
- Around line 412-431: Align the code-wiki-team Team configuration with
TeamSpec’s documented mode values: either add solo to the accepted
collaborationModel values, or remove collaborationModel: solo and represent solo
only through workflow.mode while retaining a documented collaborationModel
value.
In `@backend/init_data/skills/wiki_submit/SKILL.md`:
- Around line 39-45: Update the submit command example in the documentation to
use Bash ANSI-C quoting ($'...') for the multi-line --content value so \n
becomes actual line breaks, and add guidance to use --file for longer markdown
content.
- Around line 13-23: Update the “Page paths” section in SKILL.md to state that
paths must be unique case-insensitively and may not differ only by letter case.
Keep the rule consistent with the existing path constraints and wording used by
the related configuration and prompt documents.
In `@backend/init_data/skills/wiki_submit/wiki_submit.js`:
- Around line 216-226: Update main() to require and validate args.path alongside
the existing --title validation before submitting a page, so submit cannot
report success without a page identity. Preserve the existing section.path
assignment and ensure the missing-path case exits through the same
validation/error behavior used for a missing title.
In `@backend/tests/api/test_knowledge_code_wiki.py`:
- Around line 219-222: In the notebook-creation test flow, add an assertion that
the POST response has status 201 before accessing its JSON body, matching the
existing _create_wiki pattern; then retain the existing notebook_id extraction.
In `@backend/tests/services/knowledge/test_code_wiki_publisher.py`:
- Around line 347-360: The removal publish in
backend/tests/services/knowledge/test_code_wiki_publisher.py lines 347-360 must
use PublishPolicy(max_removed_share=1.0); capture its PublishResult and assert
published before reading PENDING_INDEX_CLEANUP_KEY. In
backend/tests/services/knowledge/test_code_wiki_publish_end_to_end.py lines
261-281, add a policy parameter to _publish, pass the same permissive policy for
the removal publish, and assert result.published before verifying the document
and attachment are removed.
---
Outside diff comments:
In `@backend/app/services/wiki_service.py`:
- Around line 563-823: Split save_generation_contents into focused private
helpers, extracting at least the section validation/path normalization and
upsert flow, plus the summary-driven status transition and code-wiki deferral
logic. Keep save_generation_contents responsible for orchestration, removals,
metadata, commit, and post-commit completion while preserving existing behavior
and data passed between helpers.
---
Nitpick comments:
In `@backend/app/services/knowledge/code_wiki_publisher.py`:
- Around line 243-262: Optionally optimize _attachments_being_replaced by
collecting the unique touched document IDs and loading all matching
KnowledgeDocument rows with one IN query instead of calling db.get for each ID.
Preserve the existing attachment_id and converted_attachment_id collection
behavior, including skipping missing documents and returning the same tuple
format.
In `@backend/app/services/knowledge/code_wiki_schedule.py`:
- Around line 40-41: Introduce an InvalidSchedule(ValueError) base exception for
general schedule validation failures, make ScheduleTooFrequent inherit from it,
and update the non-frequency validation paths in the schedule creation
logic—including invalid interval units, cron expressions, missing values, event
triggers, and unknown trigger types—to raise InvalidSchedule instead. Keep
ScheduleTooFrequent exclusively for schedules that exceed the allowed frequency.
In `@backend/app/services/knowledge/code_wiki_side_effects.py`:
- Around line 129-145: Promote knowledge_orchestrator._schedule_indexing_celery
and knowledge_service._run_async_in_new_loop to public helper names in their
owning modules, then update this adapter’s scheduling call and _run function to
use those public names. Preserve the existing arguments and execution behavior
while removing cross-module references to the underscore-prefixed symbols.
In `@backend/tests/services/knowledge/test_code_wiki_content_write.py`:
- Around line 255-277: Add a test near test_a_malformed_removal_path_is_refused
and test_a_page_written_and_removed_in_one_payload_ends_up_removed that calls
WikiService().save_generation_contents with a valid sections entry and malformed
removed_paths value "../escape". Assert it raises HTTPException with status code
400, then verify via _pages that no section was persisted, locking in
transaction-wide rollback.
In `@backend/tests/services/knowledge/test_code_wiki_generation.py`:
- Around line 80-112: Add complete type annotations to the helper functions
_write_page and _publish_a_first_wiki, including their parameters and return
types; annotate _write_page as returning None and use the existing concrete
types for the database session, generation, knowledge base, user, and effects
parameters consistently with the rest of the file.
In `@backend/tests/services/knowledge/test_code_wiki_projection_plan.py`:
- Around line 77-84: Extend
test_matching_ignores_case_because_the_database_does, or add a focused test
beside it, with two desired pages whose paths differ only by case and the same
existing page. Assert the resulting projection plan reflects the intended
write-path behavior for the shared case-insensitive collation key, including
skips and deletes as appropriate.
🪄 Autofix (Beta)
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: f2a5e3fc-b58b-480b-845c-eded5e263d45
📒 Files selected for processing (36)
backend/.env.examplebackend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.pybackend/app/api/endpoints/knowledge.pybackend/app/core/wiki_config.pybackend/app/repository/github_provider.pybackend/app/repository/gitlab_provider.pybackend/app/schemas/knowledge.pybackend/app/schemas/wiki.pybackend/app/services/knowledge/code_wiki_generation.pybackend/app/services/knowledge/code_wiki_projection.pybackend/app/services/knowledge/code_wiki_projection_plan.pybackend/app/services/knowledge/code_wiki_prompts.pybackend/app/services/knowledge/code_wiki_publish_gate.pybackend/app/services/knowledge/code_wiki_publisher.pybackend/app/services/knowledge/code_wiki_runner.pybackend/app/services/knowledge/code_wiki_schedule.pybackend/app/services/knowledge/code_wiki_side_effects.pybackend/app/services/knowledge/code_wiki_source.pybackend/app/services/knowledge/mermaid_check.pybackend/app/services/wiki_service.pybackend/init_data/02-public-resources.yamlbackend/init_data/skills/wiki_submit/SKILL.mdbackend/init_data/skills/wiki_submit/wiki_submit.jsbackend/tests/api/test_knowledge_code_wiki.pybackend/tests/services/knowledge/test_code_wiki_content_write.pybackend/tests/services/knowledge/test_code_wiki_generation.pybackend/tests/services/knowledge/test_code_wiki_projection.pybackend/tests/services/knowledge/test_code_wiki_projection_plan.pybackend/tests/services/knowledge/test_code_wiki_prompts.pybackend/tests/services/knowledge/test_code_wiki_publish_end_to_end.pybackend/tests/services/knowledge/test_code_wiki_publish_gate.pybackend/tests/services/knowledge/test_code_wiki_publisher.pybackend/tests/services/knowledge/test_code_wiki_runner.pybackend/tests/services/knowledge/test_code_wiki_schedule.pybackend/tests/services/knowledge/test_code_wiki_submit_to_publish.pybackend/tests/services/knowledge/test_mermaid_check.py
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/app/repository/gitlab_provider.py
- backend/app/services/knowledge/code_wiki_source.py
- backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py
- backend/app/services/knowledge/mermaid_check.py
- backend/tests/services/knowledge/test_mermaid_check.py
Comment on lines
+13
to
+23
| ## Page paths | ||
|
|
||
| A page is identified by its `--path`: `index`, `architecture/backend`, `modules/indexing`. | ||
| Lowercase, `/`-separated, no file extension, at most 4 folders deep. | ||
|
|
||
| **Keep a path stable across runs.** It is what lets an unchanged page keep its place, | ||
| its links and its search index. Changing it republishes the page as a deletion plus an | ||
| insertion, so reword titles freely and move paths rarely. | ||
|
|
||
| ### Submit a single section from a markdown file | ||
| Send a page's **complete content** every time. There is no patch format; what you send | ||
| replaces the page. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
State the case-uniqueness rule here as well.
backend/init_data/02-public-resources.yaml Line 361 and code_wiki_prompts.py Line 127 both tell the agent that two paths may not differ only by case. This file omits it. The projection matches paths through a case-folded key, so two paths that differ only by case collapse into one page. Add the rule so all three documents state the same contract.
📝 Proposed fix
A page is identified by its `--path`: `index`, `architecture/backend`, `modules/indexing`.
-Lowercase, `/`-separated, no file extension, at most 4 folders deep.
+Lowercase, `/`-separated, no file extension, at most 4 folders deep. Two paths may not
+differ only by case.📝 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
| ## Page paths | |
| A page is identified by its `--path`: `index`, `architecture/backend`, `modules/indexing`. | |
| Lowercase, `/`-separated, no file extension, at most 4 folders deep. | |
| **Keep a path stable across runs.** It is what lets an unchanged page keep its place, | |
| its links and its search index. Changing it republishes the page as a deletion plus an | |
| insertion, so reword titles freely and move paths rarely. | |
| ### Submit a single section from a markdown file | |
| Send a page's **complete content** every time. There is no patch format; what you send | |
| replaces the page. | |
| ## Page paths | |
| A page is identified by its `--path`: `index`, `architecture/backend`, `modules/indexing`. | |
| Lowercase, `/`-separated, no file extension, at most 4 folders deep. Two paths may not | |
| differ only by case. | |
| **Keep a path stable across runs.** It is what lets an unchanged page keep its place, | |
| its links and its search index. Changing it republishes the page as a deletion plus an | |
| insertion, so reword titles freely and move paths rarely. | |
| Send a page's **complete content** every time. There is no patch format; what you send | |
| replaces the page. |
🤖 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 `@backend/init_data/skills/wiki_submit/SKILL.md` around lines 13 - 23, Update
the “Page paths” section in SKILL.md to state that paths must be unique
case-insensitively and may not differ only by letter case. Keep the rule
consistent with the existing path constraints and wording used by the related
configuration and prompt documents.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/services/knowledge/code_wiki_runner.py (1)
302-311: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
logger.exceptionto keep the stack trace.
logger.error(..., exc)records only the exception message. Uselogger.exceptionso the full traceback is preserved for diagnosing unexpected failures inretry_pending_index_cleanup.♻️ Proposed fix
except Exception as exc: # One knowledge base's failure must not stop the others: they are # independent, and the whole point is to keep draining. db.rollback() - logger.error( - "[code_wiki] index cleanup sweep failed for kb %s: %s", + logger.exception( + "[code_wiki] index cleanup sweep failed for kb %s", knowledge_base.id, - exc, ) continue🤖 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 `@backend/app/services/knowledge/code_wiki_runner.py` around lines 302 - 311, Update the exception handler in retry_pending_index_cleanup to use logger.exception instead of logger.error while preserving the existing message, knowledge_base.id, and exc arguments, so failures retain their full traceback.Source: Linters/SAST tools
🤖 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 `@backend/app/repository/github_provider.py`:
- Around line 971-1004: Unvalidated git_domain values can receive caller tokens;
enforce the existing Git-host allowlist before constructing API URLs. In
backend/app/repository/github_provider.py:971-1004, update
get_default_branch_head; in backend/app/repository/gitlab_provider.py:961-1031,
update get_default_branch_head and get_changed_files; and in
backend/app/repository/gitea_provider.py:869-929, update get_default_branch_head
and get_changed_files. Perform the same allowlist check before each method calls
_get_api_base_url(), preserving the established rejection behavior for untrusted
hosts.
---
Nitpick comments:
In `@backend/app/services/knowledge/code_wiki_runner.py`:
- Around line 302-311: Update the exception handler in
retry_pending_index_cleanup to use logger.exception instead of logger.error
while preserving the existing message, knowledge_base.id, and exc arguments, so
failures retain their full traceback.
🪄 Autofix (Beta)
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: e11376b1-985a-47dc-90a8-ffef9ab57cb4
📒 Files selected for processing (13)
backend/app/core/celery_app.pybackend/app/repository/gitea_provider.pybackend/app/repository/github_provider.pybackend/app/repository/gitlab_provider.pybackend/app/services/knowledge/code_wiki_repo_state.pybackend/app/services/knowledge/code_wiki_runner.pybackend/app/services/knowledge/code_wiki_source.pybackend/app/tasks/knowledge_tasks.pybackend/tests/repository/test_repository_state_reads.pybackend/tests/services/knowledge/test_code_wiki_cleanup_sweep.pybackend/tests/services/knowledge/test_code_wiki_repo_state.pybackend/tests/services/knowledge/test_code_wiki_runner.pybackend/tests/services/knowledge/test_code_wiki_source.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/tests/services/knowledge/test_code_wiki_source.py
- backend/app/services/knowledge/code_wiki_source.py
Comment on lines
+971
to
+1004
|
|
||
| # ---- repository state, for deciding whether a code wiki needs regenerating ---- | ||
|
|
||
| def get_default_branch_head( | ||
| self, token: str, git_domain: str, repo_name: str | ||
| ) -> Dict[str, str]: | ||
| """Return the default branch and the commit it points at. | ||
|
|
||
| Two targeted calls rather than ``get_branches``, which pages through the | ||
| whole branch list to answer a question about one branch. | ||
| """ | ||
| api_base_url = self._get_api_base_url(git_domain) | ||
| headers = { | ||
| "Authorization": f"token {token}", | ||
| "Accept": "application/vnd.github.v3+json", | ||
| } | ||
|
|
||
| repo = requests.get( | ||
| f"{api_base_url}/repos/{repo_name}", | ||
| headers=headers, | ||
| timeout=ACCESS_CHECK_TIMEOUT_SECONDS, | ||
| ) | ||
| repo.raise_for_status() | ||
| branch_name = repo.json().get("default_branch") or "main" | ||
|
|
||
| branch = requests.get( | ||
| f"{api_base_url}/repos/{repo_name}/branches/{branch_name}", | ||
| headers=headers, | ||
| timeout=ACCESS_CHECK_TIMEOUT_SECONDS, | ||
| ) | ||
| branch.raise_for_status() | ||
| commit = (branch.json() or {}).get("commit") or {} | ||
| return {"branch": branch_name, "commit": commit.get("sha", "")} | ||
|
|
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Enforce a Git-host allowlist before these new repository-state methods send tokens.
All three providers build the repository-state request URL from git_domain via _get_api_base_url() and send the caller's token to it, with no allowlist restricting git_domain to a trusted host. A previous review on this PR stack already confirmed this gap for check_user_project_access in github_provider.py and gitlab_provider.py, tracing it to SourceRepository.from_url() deriving source_domain from any repository URL without validation. The new get_default_branch_head/get_changed_files methods added in this change reuse the exact same unvalidated URL construction, and extend the same gap into gitea_provider.py, a file not covered by the earlier finding.
backend/app/repository/github_provider.py#L971-L1004: add the host-allowlist check before_get_api_base_url()is used inget_default_branch_head.backend/app/repository/gitlab_provider.py#L961-L1031: add the same check before_get_api_base_url()is used inget_default_branch_headandget_changed_files.backend/app/repository/gitea_provider.py#L869-L929: add the same check before_get_api_base_url()is used inget_default_branch_headandget_changed_files.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 987-991: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(
f"{api_base_url}/repos/{repo_name}",
headers=headers,
timeout=ACCESS_CHECK_TIMEOUT_SECONDS,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
[warning] 995-999: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(
f"{api_base_url}/repos/{repo_name}/branches/{branch_name}",
headers=headers,
timeout=ACCESS_CHECK_TIMEOUT_SECONDS,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
📍 Affects 3 files
backend/app/repository/github_provider.py#L971-L1004(this comment)backend/app/repository/gitlab_provider.py#L961-L1031backend/app/repository/gitea_provider.py#L869-L929
🤖 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 `@backend/app/repository/github_provider.py` around lines 971 - 1004,
Unvalidated git_domain values can receive caller tokens; enforce the existing
Git-host allowlist before constructing API URLs. In
backend/app/repository/github_provider.py:971-1004, update
get_default_branch_head; in backend/app/repository/gitlab_provider.py:961-1031,
update get_default_branch_head and get_changed_files; and in
backend/app/repository/gitea_provider.py:869-929, update get_default_branch_head
and get_changed_files. Perform the same allowlist check before each method calls
_get_api_base_url(), preserving the established rejection behavior for untrusted
hosts.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/services/knowledge/code_wiki_generation.py (1)
127-133: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winKeep the internal Kind lock query consistent with the Kind lookup guideline.
Line 133 locks
KindbyKind.idalone. This internal row-lock is not an access-control lookup, but the guideline requires Kind resources to be queried withnamespace,name, anduser_id. Use the existing resolvedknowledge_basevalues in the three-identifier lock query so the lock cannot miss authorized rows because of isolation-level gap-lock behavior.🤖 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 `@backend/app/services/knowledge/code_wiki_generation.py` around lines 127 - 133, Update the internal Kind row-lock query near the knowledge-base start flow to filter by the resolved knowledge_base namespace, name, and user_id values, rather than Kind.id alone. Preserve the existing with_for_update().first() locking behavior and use the existing knowledge_base fields to keep the lookup aligned with the Kind query guideline.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@backend/app/services/knowledge/code_wiki_generation.py`:
- Around line 127-133: Update the internal Kind row-lock query near the
knowledge-base start flow to filter by the resolved knowledge_base namespace,
name, and user_id values, rather than Kind.id alone. Preserve the existing
with_for_update().first() locking behavior and use the existing knowledge_base
fields to keep the lookup aligned with the Kind query guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 468d7ac1-fdc0-48ae-94d0-2fa94aa85e18
📒 Files selected for processing (12)
backend/app/api/endpoints/knowledge.pybackend/app/schemas/kind.pybackend/app/schemas/knowledge.pybackend/app/services/knowledge/code_wiki_generation.pybackend/app/services/knowledge/code_wiki_projection.pybackend/app/services/knowledge/code_wiki_publish_gate.pybackend/app/services/knowledge/code_wiki_publisher.pybackend/init_data/skills/wiki_submit/SKILL.mdbackend/init_data/skills/wiki_submit/wiki_submit.jsbackend/tests/services/knowledge/test_code_wiki_cleanup_sweep.pybackend/tests/services/knowledge/test_code_wiki_projection.pybackend/tests/services/knowledge/test_code_wiki_publish_gate.py
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/tests/services/knowledge/test_code_wiki_projection.py
- backend/app/schemas/kind.py
- backend/app/services/knowledge/code_wiki_publisher.py
- backend/app/api/endpoints/knowledge.py
- backend/app/schemas/knowledge.py
- backend/init_data/skills/wiki_submit/wiki_submit.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/tests/services/knowledge/test_code_wiki_source.py`:
- Around line 187-201: Extend the url parameter list in
test_a_host_that_is_never_a_git_server to include IPv4-mapped IPv6 loopback,
IPv4-mapped IPv6 link-local, and unspecified IPv4/IPv6 address cases. Keep the
existing SourceAccessDenied assertion and error match so each literal-address
bypass regression is validated after host validation.
🪄 Autofix (Beta)
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: bdef323a-7ca2-45a2-908d-9142ad53378b
📒 Files selected for processing (2)
backend/app/services/knowledge/code_wiki_source.pybackend/tests/services/knowledge/test_code_wiki_source.py
Collaborator
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
backend/app/services/knowledge/code_wiki/mermaid_check.py (1)
56-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the Mermaid declaration tokens that the frontend accepts.
The allow-list contains
radarandtreemap, but the prior dependency review identifiedradar-betaandtreemap-betafor the supported Mermaid release. Valid diagrams with those declarations are still reported as broken and returned to the agent.Verify the resolved Mermaid version, then add its exact declaration tokens to
KNOWN_DIAGRAM_TYPES.For Mermaid 11.15.x, what are the exact diagram declaration tokens for radar and treemap diagrams?🤖 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 `@backend/app/services/knowledge/code_wiki/mermaid_check.py` around lines 56 - 63, Verify the resolved Mermaid 11.15.x version and update KNOWN_DIAGRAM_TYPES with the frontend-supported declaration tokens radar-beta and treemap-beta, replacing the unsupported radar and treemap entries while preserving the other diagram types.
🧹 Nitpick comments (4)
backend/app/services/knowledge/code_wiki/source.py (2)
195-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider splitting
assert_user_can_read_sourceinto smaller helpers.
assert_user_can_read_sourceperforms five distinct steps: source-type validation, credential lookup, credential-type match check, provider dispatch with error handling, and access-level validation plus logging. The function body is close to the 50-line guideline. Extract the credential validation (Lines 202-226) into a separate helper to keep this function focused on the top-level flow.As per coding guidelines, "Comments must be in English, names must be clear, and functions should remain focused, preferably under 50 lines."
🤖 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 `@backend/app/services/knowledge/code_wiki/source.py` around lines 195 - 261, Extract the credential lookup, missing-token validation, and configured-type consistency check from assert_user_can_read_source into a focused helper that accepts the user, source domain, and declared source type and returns validated git credentials. Update assert_user_can_read_source to call this helper while preserving the existing SourceAccessDenied messages and top-level validation, provider access check, and logging flow.Source: Coding guidelines
159-192: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd return/parameter type hints.
provider_for(Line 159) has no return type hint._check_access(Line 175) has an untypedproviderparameter and no return type hint. Add type hints for both, for exampleOptional[RepositoryProvider]forprovider_forandDict[str, Any]for_check_access's return type.As per coding guidelines, "Python code must follow PEP 8, Black with 88-column formatting, isort, and type hints."
🤖 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 `@backend/app/services/knowledge/code_wiki/source.py` around lines 159 - 192, Update provider_for with an Optional[RepositoryProvider] return annotation, using the repository provider base type already defined in the codebase. Add an appropriate RepositoryProvider annotation to _check_access’s provider parameter and annotate its return value as Dict[str, Any], importing the required typing symbols without changing the existing behavior.Source: Coding guidelines
backend/tests/services/knowledge/code_wiki/test_publisher.py (1)
405-472: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return type annotations to the new functions.
Add
-> Nonetotest_a_publish_settles_what_the_last_one_could_not,test_a_rejected_publish_still_settles_the_old_debt, and_update_spec_pending.As per coding guidelines, “Python code must follow PEP 8, Black with 88-column formatting, isort, and type hints.”
🤖 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 `@backend/tests/services/knowledge/code_wiki/test_publisher.py` around lines 405 - 472, Add explicit -> None return annotations to test_a_publish_settles_what_the_last_one_could_not, test_a_rejected_publish_still_settles_the_old_debt, and _update_spec_pending, preserving their existing behavior and formatting.Source: Coding guidelines
backend/app/services/knowledge/code_wiki/run_mode.py (1)
115-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider splitting
decide_run_modeinto smaller helpers.
decide_run_modeis roughly 110 lines including its docstring, well over the guideline of keeping functions under 50 lines. Extract each threshold check (manifest match, structural-move burst, changed-file count, changed-file ratio, periodic count/age) into small named helpers that each returnOptional[RunModeDecision].decide_run_modewould then iterate the checks and return the first non-Noneresult, which also makes each threshold independently testable.As per path instructions, "functions should remain focused, preferably under 50 lines."
🤖 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 `@backend/app/services/knowledge/code_wiki/run_mode.py` around lines 115 - 224, Split decide_run_mode into focused helpers for the manifest, structural-move, changed-file-count, changed-file-ratio, and periodic count/age checks, with each helper returning Optional[RunModeDecision]. Have decide_run_mode invoke these checks in the existing order and return the first non-None decision while preserving the current first-run, unchanged, unknown-diff, directory-change, empty-diff, and incremental behavior.Source: Path instructions
🤖 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 `@backend/app/services/knowledge/code_wiki/publisher.py`:
- Around line 316-323: Restore an eventual retry mechanism for references parked
during failed index cleanup, rather than relying on a later publish_generation
call. Update the cleanup flow around finish_projection to enqueue durable retry
work or use a serialized background cleanup worker after document deletion
commits, while preserving the existing single-writer coordination and
warning-level debt recording.
In `@backend/app/services/knowledge/code_wiki/run_mode.py`:
- Around line 115-224: Update start_generation to supply decide_run_mode with
the persisted incremental count, elapsed days since the last full rebuild, and
previous/current top-level directory sets from generation state or published
history. Ensure these values reflect production history rather than the
defaults, while preserving the existing decision logic in decide_run_mode.
In `@backend/init_data/skills/wiki_submit/wiki_submit.js`:
- Around line 159-169: Update readPage to verify that endpoint ends with and is
successfully transformed by the expected /generations/contents suffix before
constructing the URL; if no match occurs, fail loudly with a clear
invalid-endpoint configuration error rather than issuing the malformed request.
Preserve the existing URL construction for valid endpoints and ensure cmdRead
does not misclassify this configuration failure as a missing page.
- Around line 292-302: Update the absent-page check in the result.status error
handling to match the backend’s specific “has no page at” phrase instead of the
generic “404” pattern. Preserve the existing success exit and message for
genuinely absent pages, while allowing unrelated 404 errors to follow the normal
error path.
---
Duplicate comments:
In `@backend/app/services/knowledge/code_wiki/mermaid_check.py`:
- Around line 56-63: Verify the resolved Mermaid 11.15.x version and update
KNOWN_DIAGRAM_TYPES with the frontend-supported declaration tokens radar-beta
and treemap-beta, replacing the unsupported radar and treemap entries while
preserving the other diagram types.
---
Nitpick comments:
In `@backend/app/services/knowledge/code_wiki/run_mode.py`:
- Around line 115-224: Split decide_run_mode into focused helpers for the
manifest, structural-move, changed-file-count, changed-file-ratio, and periodic
count/age checks, with each helper returning Optional[RunModeDecision]. Have
decide_run_mode invoke these checks in the existing order and return the first
non-None decision while preserving the current first-run, unchanged,
unknown-diff, directory-change, empty-diff, and incremental behavior.
In `@backend/app/services/knowledge/code_wiki/source.py`:
- Around line 195-261: Extract the credential lookup, missing-token validation,
and configured-type consistency check from assert_user_can_read_source into a
focused helper that accepts the user, source domain, and declared source type
and returns validated git credentials. Update assert_user_can_read_source to
call this helper while preserving the existing SourceAccessDenied messages and
top-level validation, provider access check, and logging flow.
- Around line 159-192: Update provider_for with an Optional[RepositoryProvider]
return annotation, using the repository provider base type already defined in
the codebase. Add an appropriate RepositoryProvider annotation to
_check_access’s provider parameter and annotate its return value as Dict[str,
Any], importing the required typing symbols without changing the existing
behavior.
In `@backend/tests/services/knowledge/code_wiki/test_publisher.py`:
- Around line 405-472: Add explicit -> None return annotations to
test_a_publish_settles_what_the_last_one_could_not,
test_a_rejected_publish_still_settles_the_old_debt, and _update_spec_pending,
preserving their existing behavior and formatting.
🪄 Autofix (Beta)
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: 17e1ced4-d842-4a7c-929b-8b28e4ef20e9
📒 Files selected for processing (44)
backend/app/api/endpoints/knowledge.pybackend/app/api/endpoints/wiki.pybackend/app/repository/file_status.pybackend/app/repository/gitea_provider.pybackend/app/repository/github_provider.pybackend/app/schemas/wiki.pybackend/app/services/knowledge/code_wiki/__init__.pybackend/app/services/knowledge/code_wiki/generation.pybackend/app/services/knowledge/code_wiki/mermaid_check.pybackend/app/services/knowledge/code_wiki/page_path.pybackend/app/services/knowledge/code_wiki/projection.pybackend/app/services/knowledge/code_wiki/projection_plan.pybackend/app/services/knowledge/code_wiki/prompts.pybackend/app/services/knowledge/code_wiki/publish_gate.pybackend/app/services/knowledge/code_wiki/publisher.pybackend/app/services/knowledge/code_wiki/repo_state.pybackend/app/services/knowledge/code_wiki/run_mode.pybackend/app/services/knowledge/code_wiki/runner.pybackend/app/services/knowledge/code_wiki/side_effects.pybackend/app/services/knowledge/code_wiki/source.pybackend/app/services/knowledge/code_wiki/version_store.pybackend/app/services/knowledge/orchestrator.pybackend/app/services/wiki_service.pybackend/init_data/skills/wiki_submit/SKILL.mdbackend/init_data/skills/wiki_submit/wiki_submit.jsbackend/tests/api/test_knowledge_code_wiki.pybackend/tests/services/knowledge/__init__.pybackend/tests/services/knowledge/code_wiki/__init__.pybackend/tests/services/knowledge/code_wiki/test_content_write.pybackend/tests/services/knowledge/code_wiki/test_generation.pybackend/tests/services/knowledge/code_wiki/test_mermaid_check.pybackend/tests/services/knowledge/code_wiki/test_page_path.pybackend/tests/services/knowledge/code_wiki/test_projection.pybackend/tests/services/knowledge/code_wiki/test_projection_plan.pybackend/tests/services/knowledge/code_wiki/test_prompts.pybackend/tests/services/knowledge/code_wiki/test_publish_end_to_end.pybackend/tests/services/knowledge/code_wiki/test_publish_gate.pybackend/tests/services/knowledge/code_wiki/test_publisher.pybackend/tests/services/knowledge/code_wiki/test_repo_state.pybackend/tests/services/knowledge/code_wiki/test_run_mode.pybackend/tests/services/knowledge/code_wiki/test_runner.pybackend/tests/services/knowledge/code_wiki/test_source.pybackend/tests/services/knowledge/code_wiki/test_submit_to_publish.pybackend/tests/services/knowledge/code_wiki/test_version_store.py
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/app/services/knowledge/orchestrator.py
- backend/app/repository/gitea_provider.py
- backend/tests/api/test_knowledge_code_wiki.py
- backend/app/api/endpoints/knowledge.py
- backend/app/repository/github_provider.py
- backend/init_data/skills/wiki_submit/SKILL.md
A code wiki used to be its own world: its own tables, its own reader, its own permission rules, its own list. It is now a knowledge base whose content an agent generates from a source repository — which means retrieval, sharing, permissions and the document reader all apply to it without being rebuilt. **Versions, and a projection.** The agent writes into `wiki_generations` / `wiki_contents`, which no reader ever touches; publishing projects the chosen version into the knowledge base. The point is where the atomicity boundary sits: around a deterministic projection that takes seconds and can be retried, rather than around an LLM run that takes hours and fails in the middle. `spec.publishedGenerationId` is the sole authority for which version is live — never inferred from the newest completed generation, because a generation can finish and still be refused. **A page's path is its identity.** Documents are keyed by it, so rewording a heading revises the page instead of deleting and recreating it — which would change the document id that the RAG index and every stored citation depend on, and pay to re-embed the content. **Incremental versions are seeded before the agent starts**, so every version is a complete snapshot and orphans are a plain set difference rather than a reconciliation. **Ordering is fixed and asserted.** Blobs are written before the transaction commits and deleted after it: a failure before the commit leaves litter, while deleting first would destroy live content on a rollback. Attachments live in object storage and cannot join the transaction, so the ordering is the only thing protecting them. **The publish gate is what makes agent-declared deletion safe.** Removal is measured over the set of published paths a version no longer contains, not over page counts — a same-sized version under different paths is a mass deletion that counting cannot see. A refused version stays in the store with its verdict attached, and the published one is untouched. **Content ownership.** `origin` defaults to `user`, so anything the projection did not create is excluded from it. Getting that backwards would let a regeneration delete content nobody can restore. Generation is triggered directly for now; the repository's HEAD and diff are read from the provider so an unchanged repository costs one comparison instead of a full pass through the model. Every such read degrades to "unknown", which means a rebuild — a partial diff mistaken for a complete one would pick an incremental run for a change that reshaped the repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Index cleanup moves into the publish path and the periodic sweep is deleted. Parking and draining were two writers doing read-modify-write on one spec key, and the interleaving that loses a ref needs no unusual timing: a sweep reads the list, a concurrent publish appends to it, the sweep writes back what it read. A distributed lock between sweepers would not have fixed that, because the publish never held it. One writer removes the race outright, and with it the lock, the beat entry and the frequency question. The cost is stated rather than hidden: a wiki nobody regenerates keeps its orphaned chunks. The debt stays recorded on the knowledge base and logged when parked, so it is visible if that ever turns out to matter. The agent can now read a page before revising it. Incremental runs were told to "revise only the pages the changes affect" while having no way to see a word of what any page said — so revising meant rewriting from source, and whatever a page held that was not re-derivable was silently dropped. Reading is scoped to the agent's own generation, which in an incremental run is a complete copy of the published wiki: the capability it needs, and the narrowest scope that provides it. Also from review: - The folder cleanup walks the tree once, deepest-first, instead of re-querying every folder and every document until the result stopped changing. Two tests were added for multi-level emptying, which the old loop handled and nothing covered. - Folder resolution loads the knowledge base's folders once instead of querying per distinct folder. - The attachments a plan supersedes are read in one query. They were being served from the session identity map because of what ran just before, which nothing stated and nothing enforced. - GitHub and Gitea shared a near-identical status mapping in two copies; it is now one. GitLab is left out because it reports booleans, not words. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fourteen modules named code_wiki_* sat flat in services/knowledge/, which is the point at which a prefix is standing in for a directory. They move to services/knowledge/code_wiki/ with the prefix dropped, mirrored by the tests. This is package-by-feature, matching services/subscription/ next door. mermaid_check moves in with them: the publish gate is its only caller. content_scope stays put, because knowledge_service uses it too. __init__.py re-exports nothing. app.services.knowledge resolves its own exports lazily to avoid import cycles, and eagerly importing this package's modules from there would pull that whole chain in at package-import time. Importing by module path also keeps publisher and publish_gate — different things with similar names — distinguishable at the call site. Dropping the prefix collided test_generation.py with tests/schemas/, because tests/services/knowledge/ had no __init__.py while its siblings all do. Added to both levels, following the convention already in the tree. Renames are recorded as renames, so git log --follow and blame still reach the reasoning behind each of these files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kissghosts
force-pushed
the
refactor/code-wiki-kb
branch
from
August 3, 2026 06:48
188b94d to
7931f70
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
backend/app/services/knowledge/code_wiki/mermaid_check.py (1)
26-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
radar-betaandtreemap-betatoKNOWN_DIAGRAM_TYPES.
check_mermaid_blocks()lowercases the first token and compares it verbatim against this set. Mermaid 11.15.0 declares radar and treemap diagrams withradar-betaandtreemap-beta, so valid diagrams opened with those keywords are reported as broken. If bareradarortreemapshould also be accepted, add it as an alias only if Mermaid supports it.🐛 Proposed fix
"quadrantchart", - "radar", + "radar-beta", "requirementdiagram", "sankey-beta", "sequencediagram", "statediagram", "statediagram-v2", "timeline", - "treemap", + "treemap-beta", "xychart-beta", "zenuml",🤖 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 `@backend/app/services/knowledge/code_wiki/mermaid_check.py` around lines 26 - 67, Update KNOWN_DIAGRAM_TYPES to include the Mermaid 11.15.0 keywords radar-beta and treemap-beta, preserving the existing exact-token matching behavior; do not add bare aliases unless they are confirmed supported by Mermaid.
🧹 Nitpick comments (5)
backend/app/services/knowledge/code_wiki/version_store.py (2)
291-297: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe published version consumes a retention slot.
The loop skips the published generation with
continue, butpositionstill advances for it. If the published generation is among the newestkeep_successfulrows, onlykeep_successful - 1other successful versions survive. Count only the generations that are eligible for collection.♻️ Proposed fix
age_cutoff = reference - timedelta(days=max_age_days) - for position, generation in enumerate(successful): + position = 0 + for generation in successful: if generation.id == published_generation_id: continue over_count = position >= keep_successful over_age = (generation.created_at or reference) < age_cutoff + position += 1 if over_count or over_age: doomed.append(generation.id)🤖 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 `@backend/app/services/knowledge/code_wiki/version_store.py` around lines 291 - 297, Update the retention calculation in the loop over successful generations so the published generation is excluded before assigning its position, ensuring only collectible generations consume the keep_successful limit. Preserve the existing age cutoff behavior and deletion tracking in doomed.
195-210: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLoad only the columns needed to match a page path.
remove_pageloads everyWikiContentrow of the generation, including the fullcontentof each page, to compare one path. A wiki with many large pages pays that cost for a single removal. Queryidandextonly, then delete the matched row by id.♻️ Proposed refactor
normalized = normalize_page_path(path) wanted = collation_key(normalized) - for content in ( - db.query(WikiContent).filter(WikiContent.generation_id == generation_id).all() - ): - if collation_key(page_path_of(content)) == wanted: - db.delete(content) - db.flush() - logger.info( - "[code_wiki] removed page '%s' from generation %s", - normalized, - generation_id, - ) - return True - return False + rows = db.query(WikiContent.id, WikiContent.ext).filter( + WikiContent.generation_id == generation_id + ) + for content_id, ext in rows: + stored = str((ext or {}).get(PATH_EXT_KEY, "") or "") + if collation_key(stored) == wanted: + content = db.get(WikiContent, content_id) + if content is not None: + db.delete(content) + db.flush() + logger.info( + "[code_wiki] removed page '%s' from generation %s", + normalized, + generation_id, + ) + return True + return False🤖 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 `@backend/app/services/knowledge/code_wiki/version_store.py` around lines 195 - 210, Update remove_page to query only WikiContent.id and WikiContent.ext for the requested generation, use the returned ext value with page_path_of for path matching, and delete the matched row by its id instead of loading full WikiContent objects. Preserve the existing normalization, logging, flush, and True/False return behavior.backend/app/services/knowledge/code_wiki/runner.py (1)
129-141: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
changed_paths=Noneis only read from the provider whenhead_commitis empty.The docstring states that
changed_paths=Noneasks for the diff to be read from the provider. The provider read sits insideif not head_commit:. If a caller supplieshead_commitand leaveschanged_pathsasNone, no diff is read, andstart_generationsees an unknown change set. Move the diff read out of the commit branch, or correct the docstring.♻️ Proposed fix
- if not head_commit: + if not head_commit or changed_paths is None: state = read_repository_state( db, user_id=task_user.id, source=source, since_commit=previous_commit, ) - head_commit = state.head_commit + head_commit = head_commit or state.head_commit if changed_paths is None: changed_paths = state.changed_paths🤖 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 `@backend/app/services/knowledge/code_wiki/runner.py` around lines 129 - 141, Ensure start_generation reads provider changes whenever changed_paths is None, even when head_commit is supplied. Update the control flow around read_repository_state so commit resolution remains conditional on head_commit, while the changed_paths population is performed independently and preserves the existing task user, source, and since_commit inputs.backend/app/api/endpoints/knowledge.py (1)
574-585: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the public knowledge-base lookup instead of the private record accessor.
Line 574 calls
KnowledgeService._get_knowledge_base_record, a private method, from the API layer. The module already has_validate_knowledge_base_access_or_raise(Line 151), which uses the publicKnowledgeService.get_knowledge_baseand raises the same 404. Reusing it keeps one lookup path and keeps the private accessor inside the service.The manage-permission check at Line 579 can stay after that call.
As per coding guidelines: "Before adding code, search for and reuse existing components, services, utilities, and patterns".
🤖 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 `@backend/app/api/endpoints/knowledge.py` around lines 574 - 585, Replace the direct KnowledgeService._get_knowledge_base_record call in the endpoint with the existing _validate_knowledge_base_access_or_raise helper, preserving its 404 behavior and keeping the subsequent KnowledgeService.can_manage_knowledge_base permission check unchanged.Source: Coding guidelines
backend/app/services/knowledge/code_wiki/publisher.py (1)
157-253: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
superseded_attachmentsinsideapply_projection_planis never used.The publisher computes the superseded ids itself with
_attachments_being_replaced(Line 231) and passes them tofinish_projection(Line 247). Inbackend/app/services/knowledge/code_wiki/projection.py,apply_projection_planbuilds its ownsuperseded_attachmentslist (Lines 194, 209, 222, 225) and neither returns nor uses it. Remove that list fromprojection.pyso one place owns the superseded ids.As per coding guidelines: "Delete dead code and do not add compatibility shims or fallback paths without agreement".
🤖 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 `@backend/app/services/knowledge/code_wiki/publisher.py` around lines 157 - 253, Remove the unused superseded_attachments collection and all related mutations from apply_projection_plan in projection.py. Keep _attachments_being_replaced in publish_generation as the sole source of superseded attachment IDs passed to finish_projection, without adding compatibility or fallback logic.Source: Coding guidelines
🤖 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
`@backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py`:
- Around line 28-32: Resolve the Alembic revision graph around revision
bd9c871a93d2 and the other listed heads by merging or relinking the migration
branches into one upgrade path. Update the affected revision identifiers and add
any required merge revision so upgrade head resolves to a single linear sequence
while preserving the existing schema changes.
In `@backend/app/api/endpoints/wiki.py`:
- Around line 221-247: Enforce the documented own-generation scope in
read_wiki_generation_page by resolving the JWT caller and verifying that
generation_id belongs to that caller, or otherwise require the internal token;
return 403 for unauthorized generations. In backend/app/api/endpoints/wiki.py
lines 221-247, update the endpoint authorization while preserving its page
lookup and 404 behavior. In backend/init_data/skills/wiki_submit/SKILL.md lines
53-63, keep the “Only your own generation is readable” wording once the endpoint
enforces this restriction; if authorization remains open to any signed-in user,
remove that claim instead.
In `@backend/app/services/knowledge/code_wiki/mermaid_check.py`:
- Around line 101-109: Update _diagram_type_of to skip a leading Mermaid YAML
frontmatter block delimited by matching --- lines before selecting the first
meaningful diagram token. Preserve existing handling of blank lines and %%
comments, and continue returning the first lowercased diagram type or an empty
string when none exists.
In `@backend/app/services/knowledge/code_wiki/projection.py`:
- Around line 91-119: Remove the unused user_id parameter from _folder_resolver
and update every call site to stop passing it. Keep the folder cache keyed only
by parent_id and normalized name, since KnowledgeFolder has no ownership field.
---
Duplicate comments:
In `@backend/app/services/knowledge/code_wiki/mermaid_check.py`:
- Around line 26-67: Update KNOWN_DIAGRAM_TYPES to include the Mermaid 11.15.0
keywords radar-beta and treemap-beta, preserving the existing exact-token
matching behavior; do not add bare aliases unless they are confirmed supported
by Mermaid.
---
Nitpick comments:
In `@backend/app/api/endpoints/knowledge.py`:
- Around line 574-585: Replace the direct
KnowledgeService._get_knowledge_base_record call in the endpoint with the
existing _validate_knowledge_base_access_or_raise helper, preserving its 404
behavior and keeping the subsequent KnowledgeService.can_manage_knowledge_base
permission check unchanged.
In `@backend/app/services/knowledge/code_wiki/publisher.py`:
- Around line 157-253: Remove the unused superseded_attachments collection and
all related mutations from apply_projection_plan in projection.py. Keep
_attachments_being_replaced in publish_generation as the sole source of
superseded attachment IDs passed to finish_projection, without adding
compatibility or fallback logic.
In `@backend/app/services/knowledge/code_wiki/runner.py`:
- Around line 129-141: Ensure start_generation reads provider changes whenever
changed_paths is None, even when head_commit is supplied. Update the control
flow around read_repository_state so commit resolution remains conditional on
head_commit, while the changed_paths population is performed independently and
preserves the existing task user, source, and since_commit inputs.
In `@backend/app/services/knowledge/code_wiki/version_store.py`:
- Around line 291-297: Update the retention calculation in the loop over
successful generations so the published generation is excluded before assigning
its position, ensuring only collectible generations consume the keep_successful
limit. Preserve the existing age cutoff behavior and deletion tracking in
doomed.
- Around line 195-210: Update remove_page to query only WikiContent.id and
WikiContent.ext for the requested generation, use the returned ext value with
page_path_of for path matching, and delete the matched row by its id instead of
loading full WikiContent objects. Preserve the existing normalization, logging,
flush, and True/False return behavior.
🪄 Autofix (Beta)
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: 8396ae9e-0f6d-46ce-b589-a24a78d24552
📒 Files selected for processing (57)
backend/.env.examplebackend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.pybackend/app/api/endpoints/knowledge.pybackend/app/api/endpoints/wiki.pybackend/app/core/wiki_config.pybackend/app/models/knowledge.pybackend/app/models/wiki.pybackend/app/repository/file_status.pybackend/app/repository/gitea_provider.pybackend/app/repository/github_provider.pybackend/app/repository/gitlab_provider.pybackend/app/schemas/kind.pybackend/app/schemas/knowledge.pybackend/app/schemas/wiki.pybackend/app/services/knowledge/code_wiki/__init__.pybackend/app/services/knowledge/code_wiki/generation.pybackend/app/services/knowledge/code_wiki/mermaid_check.pybackend/app/services/knowledge/code_wiki/page_path.pybackend/app/services/knowledge/code_wiki/projection.pybackend/app/services/knowledge/code_wiki/projection_plan.pybackend/app/services/knowledge/code_wiki/prompts.pybackend/app/services/knowledge/code_wiki/publish_gate.pybackend/app/services/knowledge/code_wiki/publisher.pybackend/app/services/knowledge/code_wiki/repo_state.pybackend/app/services/knowledge/code_wiki/run_mode.pybackend/app/services/knowledge/code_wiki/runner.pybackend/app/services/knowledge/code_wiki/side_effects.pybackend/app/services/knowledge/code_wiki/source.pybackend/app/services/knowledge/code_wiki/version_store.pybackend/app/services/knowledge/content_scope.pybackend/app/services/knowledge/knowledge_service.pybackend/app/services/knowledge/orchestrator.pybackend/app/services/wiki_service.pybackend/init_data/02-public-resources.yamlbackend/init_data/skills/wiki_submit/SKILL.mdbackend/init_data/skills/wiki_submit/wiki_submit.jsbackend/tests/api/test_knowledge_code_wiki.pybackend/tests/repository/test_repository_state_reads.pybackend/tests/services/knowledge/__init__.pybackend/tests/services/knowledge/code_wiki/__init__.pybackend/tests/services/knowledge/code_wiki/test_content_write.pybackend/tests/services/knowledge/code_wiki/test_generation.pybackend/tests/services/knowledge/code_wiki/test_mermaid_check.pybackend/tests/services/knowledge/code_wiki/test_page_path.pybackend/tests/services/knowledge/code_wiki/test_projection.pybackend/tests/services/knowledge/code_wiki/test_projection_plan.pybackend/tests/services/knowledge/code_wiki/test_prompts.pybackend/tests/services/knowledge/code_wiki/test_publish_end_to_end.pybackend/tests/services/knowledge/code_wiki/test_publish_gate.pybackend/tests/services/knowledge/code_wiki/test_publisher.pybackend/tests/services/knowledge/code_wiki/test_repo_state.pybackend/tests/services/knowledge/code_wiki/test_run_mode.pybackend/tests/services/knowledge/code_wiki/test_runner.pybackend/tests/services/knowledge/code_wiki/test_source.pybackend/tests/services/knowledge/code_wiki/test_submit_to_publish.pybackend/tests/services/knowledge/code_wiki/test_version_store.pybackend/tests/services/knowledge/test_content_scope.py
🚧 Files skipped from review as they are similar to previous changes (39)
- backend/app/models/wiki.py
- backend/tests/services/knowledge/code_wiki/test_projection_plan.py
- backend/app/core/wiki_config.py
- backend/tests/services/knowledge/init.py
- backend/tests/services/knowledge/code_wiki/test_version_store.py
- backend/tests/services/knowledge/code_wiki/test_generation.py
- backend/app/services/knowledge/code_wiki/page_path.py
- backend/app/services/knowledge/code_wiki/init.py
- backend/app/schemas/wiki.py
- backend/tests/services/knowledge/code_wiki/init.py
- backend/app/services/knowledge/code_wiki/publish_gate.py
- backend/tests/services/knowledge/code_wiki/test_content_write.py
- backend/app/repository/file_status.py
- backend/app/services/knowledge/code_wiki/projection_plan.py
- backend/app/services/knowledge/code_wiki/generation.py
- backend/tests/services/knowledge/code_wiki/test_mermaid_check.py
- backend/app/services/knowledge/orchestrator.py
- backend/tests/services/knowledge/code_wiki/test_submit_to_publish.py
- backend/app/services/knowledge/code_wiki/side_effects.py
- backend/init_data/02-public-resources.yaml
- backend/app/services/knowledge/code_wiki/source.py
- backend/app/schemas/kind.py
- backend/tests/services/knowledge/code_wiki/test_projection.py
- backend/app/models/knowledge.py
- backend/tests/services/knowledge/code_wiki/test_runner.py
- backend/tests/services/knowledge/code_wiki/test_run_mode.py
- backend/tests/services/knowledge/test_content_scope.py
- backend/app/repository/gitlab_provider.py
- backend/init_data/skills/wiki_submit/wiki_submit.js
- backend/app/services/knowledge/content_scope.py
- backend/app/services/wiki_service.py
- backend/tests/services/knowledge/code_wiki/test_publisher.py
- backend/app/services/knowledge/knowledge_service.py
- backend/app/services/knowledge/code_wiki/run_mode.py
- backend/tests/services/knowledge/code_wiki/test_publish_gate.py
- backend/tests/services/knowledge/code_wiki/test_page_path.py
- backend/app/schemas/knowledge.py
- backend/tests/api/test_knowledge_code_wiki.py
- backend/app/services/knowledge/code_wiki/prompts.py
Comment on lines
+28
to
+32
| # revision identifiers, used by Alembic. | ||
| revision: str = "bd9c871a93d2" | ||
| down_revision: Union[str, Sequence[str], None] = "b9c0d1e2f3a4" | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List all revision/down_revision declarations to detect missing parents or multiple heads.
fd -e py . backend/alembic/versions --exec rg -n '^revision(: str)? *=|^down_revision' {} \; -x true
python3 - <<'PY'
import ast, pathlib
revs, downs = {}, {}
for p in sorted(pathlib.Path("backend/alembic/versions").glob("*.py")):
src = p.read_text()
tree = ast.parse(src, filename=str(p))
a = {}
for node in tree.body:
if isinstance(node, ast.Assign):
for t in node.targets:
if isinstance(t, ast.Name):
try:
a[t.id] = ast.literal_eval(node.value)
except Exception:
pass
if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name):
try:
a[node.target.id] = ast.literal_eval(node.value)
except Exception:
pass
if "revision" in a:
revs[a["revision"]] = str(p)
downs[a["revision"]] = a.get("down_revision")
parents = {d for d in downs.values() if d}
print("missing parents:", sorted(p for p in parents if p not in revs))
print("heads:", sorted(r for r in revs if r not in parents))
PYRepository: wecode-ai/Wegent
Length of output: 2396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Candidate migration files containing bd9c871a93d2 / b9c0d1e2f3a4"
rg -n "bd9c871a93d2|b9c0d1e2f3a4|revision = \"(c8d2e3f4a5b6|e6f7a8b9c013|e6f7a8b9c012|a2b3c4d5e6f7)\"" backend/alembic/versions || true
echo
echo "## Alembic env/config files"
git ls-files 'backend/alembic/**'
python3 - <<'PY'
import pathlib
for p in pathlib.Path("backend/alembic").rglob("*"):
if p.is_file() and p.suffix in {".py", ".ini"}:
if any(x in p.parts for x in {"versions", "env.py", "script.py.mako", "config.py"}):
print(p)
PYRepository: wecode-ai/Wegent
Length of output: 5842
Resolve the Alembic graph before applying this migration.
bd9c871a93d2 correctly points to b9c0d1e2f3a4, but the migration tree still has multiple heads (bd9c871a93d2, c8d2e3f4a5b6, e6f7a8b9c012, e6f7a8b9c013, and a2b3c4d5e6f7). Merge or relink the branches so upgrade head applies one linear sequence and the schema is not split across heads.
🤖 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
`@backend/alembic/versions/20260731_bd9c871a93d2_add_knowledge_content_origin_and_wiki_.py`
around lines 28 - 32, Resolve the Alembic revision graph around revision
bd9c871a93d2 and the other listed heads by merging or relinking the migration
branches into one upgrade path. Update the affected revision identifiers and add
any required merge revision so upgrade head resolves to a single linear sequence
while preserving the existing schema changes.
Source: Coding guidelines
Comment on lines
+221
to
+247
| @internal_router.get("/generations/{generation_id}/pages", response_model=WikiPageRead) | ||
| def read_wiki_generation_page( | ||
| generation_id: int, | ||
| path: str = Query(..., min_length=1, description="Stable page path to read"), | ||
| _: None = Depends(_verify_internal_token), | ||
| wiki_db: Session = Depends(get_wiki_db), | ||
| ): | ||
| """Read one page of the version the agent is writing into (internal use). | ||
|
|
||
| An incremental version begins as a complete copy of the published wiki, so the | ||
| agent's own generation is also the current wiki — which makes "read your own | ||
| version" both the capability it needs and the narrowest scope that provides it. | ||
| Without this the instruction to revise a page cannot be followed: the agent knows | ||
| the page's path and cannot see a word of what it says. | ||
|
|
||
| Answers 404 when the path holds no page. That is a useful answer rather than a | ||
| failure — in an incremental run it means the page is new. | ||
| """ | ||
| page = wiki_service.get_generation_page( | ||
| wiki_db=wiki_db, generation_id=generation_id, path=path | ||
| ) | ||
| if page is None: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail=f"Generation {generation_id} has no page at '{path}'", | ||
| ) | ||
| return page |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
The "own generation only" scope is documented but not enforced. _verify_internal_token authorizes any active user's JWT and does not bind the caller to a generation, so read_wiki_generation_page returns page content for any generation_id. The skill documentation states a narrower scope than the endpoint implements.
backend/app/api/endpoints/wiki.py#L221-L247: resolve the caller from the JWT and verify the generation belongs to that caller, or require the internal token; reject other callers with 403.backend/init_data/skills/wiki_submit/SKILL.md#L53-L63: after the endpoint enforces the scope, keep the wording; if the endpoint stays open to any signed-in user, remove the "Only your own generation is readable" claim.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 226-226: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
📍 Affects 2 files
backend/app/api/endpoints/wiki.py#L221-L247(this comment)backend/init_data/skills/wiki_submit/SKILL.md#L53-L63
🤖 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 `@backend/app/api/endpoints/wiki.py` around lines 221 - 247, Enforce the
documented own-generation scope in read_wiki_generation_page by resolving the
JWT caller and verifying that generation_id belongs to that caller, or otherwise
require the internal token; return 403 for unauthorized generations. In
backend/app/api/endpoints/wiki.py lines 221-247, update the endpoint
authorization while preserving its page lookup and 404 behavior. In
backend/init_data/skills/wiki_submit/SKILL.md lines 53-63, keep the “Only your
own generation is readable” wording once the endpoint enforces this restriction;
if authorization remains open to any signed-in user, remove that claim instead.
The frontend builds its navigation from page paths, and three things it needs were either discarded or never recorded. **The title was thrown away.** Documents were named after the path's last segment, so "Backend Architecture" at architecture/backend became a document called `backend`, and two pages at architecture/backend and services/backend were indistinguishable anywhere the path was not also shown. The title is now the document's name; the path stays the identity in source_config, so rewording a heading renames the page in place and the id the RAG index is keyed on survives. That inverts an earlier trade-off, which a test recorded as "the projection compares content, not headings". That was right while the title was discarded. Now that it reaches the knowledge base, the fingerprint has to cover it, and the cost is rewriting an attachment whose bytes did not change — paid rarely, because a title almost always moves with the body it heads. **Order was nowhere.** The agent has been sending structure_order since this work began and it was recorded and never read. Paths carry hierarchy and say nothing about which section comes first, and alphabetically "api" precedes the overview. It is now written to spec.pageOrder — as one array in the publish transaction, not per document, because a reorder must not touch a page whose content did not change: the fingerprint would still match, the projection would skip it, and the new position would never land. **lastPublishedAt / lastPublishedCommit** join it there, so a list can show when a wiki was last built without joining every wiki against its generations. A section that holds pages but has no page of its own is reported as a warning, not refused. It renders as a heading a reader cannot open — worse to read, and nowhere near worth discarding an otherwise complete version, which is the same trade already made for diagrams that will not render. The instructions ask for section pages so the warning stays rare. `block_on_mermaid` deliberately sees only the diagram warnings; a policy named for diagrams must not start rejecting versions over a navigation nit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/services/knowledge/code_wiki/projection_plan.py (1)
35-46: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse a delimiter-safe fingerprint to avoid title/content collisions.
content_fingerprintjoinstitleandcontentwith a single"\n"separator before hashing. Iftitlecontains a newline, two different(title, content)pairs can produce the same joined string and therefore the same fingerprint. For example,title="A\nB", content="C"andtitle="A", content="B\nC"both join to"A\nB\nC".A collision here means
compute_projection_planwould treat a page whose title and content both changed as unchanged and skip the rewrite, which contradicts the stated purpose of covering the title in the fingerprint. Titles come from agent-generated text and are not validated against embedded newlines.Hash the two parts independently, or use a length-prefixed encoding, to remove the ambiguity.
🔧 Proposed fix
def content_fingerprint(title: str, content: str) -> str: """Return the fingerprint used to decide whether a page needs rewriting. ... """ - return hashlib.sha256(f"{title}\n{content}".encode("utf-8")).hexdigest() + digest = hashlib.sha256() + title_bytes = title.encode("utf-8") + digest.update(len(title_bytes).to_bytes(8, "big")) + digest.update(title_bytes) + digest.update(content.encode("utf-8")) + return digest.hexdigest()🤖 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 `@backend/app/services/knowledge/code_wiki/projection_plan.py` around lines 35 - 46, Update content_fingerprint to encode title and content unambiguously before hashing, replacing the single newline join with independent length-prefixed or separately hashed components. Preserve coverage of both fields so compute_projection_plan detects changes to either title or content without allowing embedded newlines to create collisions.
🤖 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 `@backend/app/services/knowledge/code_wiki/publish_gate.py`:
- Around line 145-161: Update _sections_without_a_page so missing is keyed by
collation_key(section path), while retaining one representative raw section path
for display. Preserve the existing present check and sorted list return,
ensuring differently cased paths for the same logical section produce a single
warning.
---
Nitpick comments:
In `@backend/app/services/knowledge/code_wiki/projection_plan.py`:
- Around line 35-46: Update content_fingerprint to encode title and content
unambiguously before hashing, replacing the single newline join with independent
length-prefixed or separately hashed components. Preserve coverage of both
fields so compute_projection_plan detects changes to either title or content
without allowing embedded newlines to create collisions.
🪄 Autofix (Beta)
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: b1ae4026-6874-43da-b04d-6db5de8d9464
📒 Files selected for processing (11)
backend/app/services/knowledge/code_wiki/projection.pybackend/app/services/knowledge/code_wiki/projection_plan.pybackend/app/services/knowledge/code_wiki/prompts.pybackend/app/services/knowledge/code_wiki/publish_gate.pybackend/app/services/knowledge/code_wiki/publisher.pybackend/tests/services/knowledge/code_wiki/test_projection.pybackend/tests/services/knowledge/code_wiki/test_projection_plan.pybackend/tests/services/knowledge/code_wiki/test_prompts.pybackend/tests/services/knowledge/code_wiki/test_publish_gate.pybackend/tests/services/knowledge/code_wiki/test_publisher.pybackend/tests/services/knowledge/code_wiki/test_submit_to_publish.py
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/tests/services/knowledge/code_wiki/test_projection_plan.py
- backend/tests/services/knowledge/code_wiki/test_submit_to_publish.py
- backend/app/services/knowledge/code_wiki/projection.py
- backend/app/services/knowledge/code_wiki/prompts.py
…r asked Ownership moves to the configured wiki account. That account's Git credentials are what clone the repository, so it is the identity the wiki actually depends on; attributing it to the requester made a repository's documentation disappear when one person left, and let two colleagues each own a private copy of the same thing. The consequence is deliberate and load-bearing. Knowledge-base ACLs now grant nobody else anything, so **who may read a code wiki is decided by who may read its repository** — the rule that needs no second copy of the repository's membership kept in sync. That check runs wegent → git, using the signed-in user's own credentials, and this is why it works where syncing members would not: syncing runs git → wegent and needs every member mapped back to a wegent account through credentials stored in a JSON column, silently omitting everyone who has never configured a token. Here there is no mapping — the identity is theirs by construction. It usually costs nothing, because the provider layer already keeps each user's accessible repositories in Redis; only a cold cache asks about the one repository, and an unreachable provider is refused rather than waved through. One repository has one wiki, registered on wiki_projects, whose source_url is already UNIQUE. That constraint is what settles two simultaneous requests; a check against a JSON field on the knowledge base would leave a window exactly where it matters. The loser of that race gets the winner's wiki, and asking for a repository that already has one answers 200 rather than 201. Code wikis are kept out of general knowledge base listings by a scope, wired into all four paths that produce them — including the MCP tool, where an agent shown one may write into it and have the next publish delete what it added. Leaving that to ownership would be incidental rather than stated: it holds only while the wiki account is somebody else, and stops holding for that account or for an administrator. The scope spells out NULL, because a knowledge base predating kbType compares NULL against the literal and would vanish from every listing at once. Also: - GET /knowledge-bases/code-wikis lists them, reading the repository, published time and commit straight from spec. - The model-binding check can be asked about the code wiki team; it reported on the legacy one, which says a model is bound when the run about to start has none. - Creation now verifies the wiki account can read the repository, not just the requester. That mismatch used to surface as a failed generation, with nothing in the failure saying the account simply needed adding to the repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
backend/tests/api/test_knowledge_code_wiki.py (1)
411-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the exclusion assertion to the remaining buckets.
This test only inspects
personal.created_by_meandpersonal.shared_with_me./all-groupedalso returnsgroupsandorganization. Those are the buckets whose queries inget_all_knowledge_bases_groupeddo not applyexclude_code_wikis(seebackend/app/services/knowledge/knowledge_service.pyLines 2793-2802 and Lines 2842-2853), so this test cannot detect a code wiki leaking through them.💚 Proposed extension
listed = [ kb["id"] for bucket in ( grouped["personal"]["created_by_me"], grouped["personal"]["shared_with_me"], + *[group["knowledge_bases"] for group in grouped["groups"]], + grouped["organization"]["knowledge_bases"], ) for kb in bucket ] assert kb_id not in listed🤖 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 `@backend/tests/api/test_knowledge_code_wiki.py` around lines 411 - 423, Extend the `listed` collection in this test to also include knowledge-base IDs from `grouped["groups"]` and `grouped["organization"]`, preserving the existing personal buckets. Keep the final `assert kb_id not in listed` unchanged so the exclusion check covers every `/all-grouped` response bucket.backend/tests/services/knowledge/code_wiki/test_read_access.py (1)
63-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese patches depend on the function-local import staying function-local.
_live_checkpatchesassert_user_can_read_sourceonapp.services.knowledge.code_wiki.source. That intercepts the call only because_repository_is_readable_nowimports the name inside the function body (read_access.pyLines 100-103), which resolves the attribute at call time. If that import moves to module scope,read_accessbinds its own reference and these patches stop intercepting. The refusal tests would then perform real provider work and could pass for the wrong reason.Patch the name where
read_accessuses it, so the test does not depend on the import position:💚 Proposed change
def _live_check(allowed: bool): if allowed: return patch( - "app.services.knowledge.code_wiki.source.assert_user_can_read_source", + "app.services.knowledge.code_wiki.read_access.assert_user_can_read_source", return_value={"has_access": True}, ) return patch( - "app.services.knowledge.code_wiki.source.assert_user_can_read_source", + "app.services.knowledge.code_wiki.read_access.assert_user_can_read_source", side_effect=SourceAccessDenied("no access"), )This requires hoisting the import in
read_access.pyto module scope, which also removes the duplicate import of a module already imported at Line 35. Apply the same change to theRuntimeErrorpatch at Lines 110-113.🤖 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 `@backend/tests/services/knowledge/code_wiki/test_read_access.py` around lines 63 - 72, Update the patches in _live_check and the RuntimeError test to target assert_user_can_read_source where read_access uses its module-level binding, rather than app.services.knowledge.code_wiki.source. Hoist the import used by _repository_is_readable_now to module scope and remove the duplicate local import, ensuring refusal and runtime-error tests continue intercepting the call without relying on import placement.
🤖 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 `@backend/app/api/endpoints/knowledge.py`:
- Around line 574-604: Update the code-wiki listing flow around the query and
_code_wiki_list_item to batch document counts: collect the readable wiki IDs,
call KnowledgeService.get_document_counts(db, ids) once, and pass the resulting
mapping into item construction instead of calling get_document_count per wiki.
Add offset and limit parameters and apply them to the active code-wiki query,
following the existing list_knowledge_bases_paginated pattern while preserving
readability filtering and response totals.
- Around line 554-559: Update the existing code-wiki response path after
KnowledgeService._get_knowledge_base_record to compute or retrieve the wiki’s
actual document count and pass it to KnowledgeBaseResponse.from_kind(kind).
Preserve the 404 behavior and ensure the returned document_count matches the
count reported by GET /code-wikis.
In `@backend/app/services/knowledge/code_wiki/read_access.py`:
- Around line 121-131: Refactor readable_wiki_ids to fetch cached repository
names once per (source_type, source_domain) pair, then evaluate each knowledge
base against that shared name set and invoke the live readability check only for
cache misses. Extract a _cached_repository_names helper from
_repository_is_in_the_users_cache and update may_read_code_wiki to reuse it,
preserving may_read_code_wiki as the single-wiki entry point and making the
readable_wiki_ids docstring accurate.
- Around line 60-68: Update _repository_is_in_the_users_cache to emit a warning
through the available module logger when _get_all_repositories_from_cache is
absent, immediately before returning False. Include enough context to identify
the provider or source repository while preserving the existing fallback
behavior.
In `@backend/app/services/knowledge/knowledge_service.py`:
- Around line 515-522: Apply exclude_code_wikis consistently to every
knowledge-base listing query: the ORGANIZATION branch of list_knowledge_bases in
backend/app/services/knowledge/knowledge_service.py:564-575; personal_kbs and
org_kbs in get_accessible_knowledge at :2011-2021 and :2138-2149; the equivalent
queries in get_personal_knowledge_bases_grouped at :2218-2228 and :2251-2262;
and get_all_knowledge_bases_grouped at :2793-2802 and :2842-2853. The anchor
query at backend/app/services/knowledge/knowledge_service.py:515-522 already
uses the helper and requires no direct change. Extend
backend/tests/api/test_knowledge_code_wiki.py:411-423 to assert the code wiki is
absent from both groups and organization buckets.
---
Nitpick comments:
In `@backend/tests/api/test_knowledge_code_wiki.py`:
- Around line 411-423: Extend the `listed` collection in this test to also
include knowledge-base IDs from `grouped["groups"]` and
`grouped["organization"]`, preserving the existing personal buckets. Keep the
final `assert kb_id not in listed` unchanged so the exclusion check covers every
`/all-grouped` response bucket.
In `@backend/tests/services/knowledge/code_wiki/test_read_access.py`:
- Around line 63-72: Update the patches in _live_check and the RuntimeError test
to target assert_user_can_read_source where read_access uses its module-level
binding, rather than app.services.knowledge.code_wiki.source. Hoist the import
used by _repository_is_readable_now to module scope and remove the duplicate
local import, ensuring refusal and runtime-error tests continue intercepting the
call without relying on import placement.
🪄 Autofix (Beta)
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: 91c8ef63-efd2-4d63-ae21-ba19aa55fe6e
📒 Files selected for processing (12)
backend/alembic/versions/20260804_2b5791acc5fa_link_wiki_projects_to_their_code_wiki.pybackend/app/api/endpoints/knowledge.pybackend/app/api/endpoints/wiki.pybackend/app/models/wiki.pybackend/app/schemas/knowledge.pybackend/app/services/knowledge/code_wiki/read_access.pybackend/app/services/knowledge/code_wiki/registry.pybackend/app/services/knowledge/content_scope.pybackend/app/services/knowledge/knowledge_service.pybackend/tests/api/test_knowledge_code_wiki.pybackend/tests/services/knowledge/code_wiki/test_read_access.pybackend/tests/services/knowledge/test_content_scope.py
Comment on lines
+60
to
+68
| def _repository_is_in_the_users_cache(user: User, source: SourceRepository) -> bool: | ||
| """Look for the repository in the list the provider layer already caches.""" | ||
| provider = provider_for(source.source_type) | ||
| reader = getattr(provider, "_get_all_repositories_from_cache", None) | ||
| if reader is None: | ||
| return False | ||
|
|
||
| try: | ||
| cached = _run_async_in_new_loop(reader(user, source.source_domain)) |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm each repository provider defines the cache reader this code depends on.
fd -e py . backend/app/repository --exec rg -n -C2 '_get_all_repositories_from_cache'
rg -nP -C3 'def provider_for' backend/app/services/knowledge/code_wiki/source.pyRepository: wecode-ai/Wegent
Length of output: 5939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files 'backend/app/services/knowledge/code_wiki/read_access.py' 'backend/app/services/knowledge/code_wiki/source.py' 'backend/app/repository/*.py' | sed -n '1,120p'
echo "== read_access helper =="
cat -n backend/app/services/knowledge/code_wiki/read_access.py | sed -n '1,170p'
echo "== provider_for and SourceRepository =="
cat -n backend/app/services/knowledge/code_wiki/source.py | sed -n '130,185p'
echo "== provider type names and cache reader definitions =="
python3 - <<'PY'
import ast
from pathlib import Path
root = Path('backend/app/repository')
defs = []
classes = []
for path in sorted(root.glob('*.py')):
tree = ast.parse(path.read_text())
for node in ast.walk(tree):
if isinstance(node, ast.AsyncFunctionDef) and node.name == '_get_all_repositories_from_cache':
defs.append((path, node.lineno))
if isinstance(node, ast.ClassDef):
classes.append((path, node.lineno, node.name))
print("\nDefinitions:")
for path, line in defs:
print(f"{path}:{line}")
print("\nClasses:")
for path, line, name in sorted(classes):
print(f"{path}:{line}:{name}")
PY
echo "== exact read_access implementation around cache lookup =="
python3 - <<'PY'
from pathlib import Path
p=Path('backend/app/services/knowledge/code_wiki/read_access.py')
text=p.read_text().splitlines()
for i,line in enumerate(text[55:90], start=60):
print(f"{i}: {line}")
PYRepository: wecode-ai/Wegent
Length of output: 10974
Add logging when the repository cache reader is absent.
_repository_is_in_the_users_cache returns False silently if _get_all_repositories_from_cache is missing, which sends every wiki read through the live provider check instead of one per user cache read. Log this at warning level before returning.
reader = getattr(provider, "_get_all_repositories_from_cache", None)
if reader is None:
+ logger.warning(
+ "[code_wiki] provider %s exposes no repository cache; "
+ "read access will be checked live per wiki",
+ source.source_type,
+ )
return False📝 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
| def _repository_is_in_the_users_cache(user: User, source: SourceRepository) -> bool: | |
| """Look for the repository in the list the provider layer already caches.""" | |
| provider = provider_for(source.source_type) | |
| reader = getattr(provider, "_get_all_repositories_from_cache", None) | |
| if reader is None: | |
| return False | |
| try: | |
| cached = _run_async_in_new_loop(reader(user, source.source_domain)) | |
| def _repository_is_in_the_users_cache(user: User, source: SourceRepository) -> bool: | |
| """Look for the repository in the list the provider layer already caches.""" | |
| provider = provider_for(source.source_type) | |
| reader = getattr(provider, "_get_all_repositories_from_cache", None) | |
| if reader is None: | |
| logger.warning( | |
| "[code_wiki] provider %s exposes no repository cache; " | |
| "read access will be checked live per wiki", | |
| source.source_type, | |
| ) | |
| return False | |
| try: | |
| cached = _run_async_in_new_loop(reader(user, source.source_domain)) |
🤖 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 `@backend/app/services/knowledge/code_wiki/read_access.py` around lines 60 -
68, Update _repository_is_in_the_users_cache to emit a warning through the
available module logger when _get_all_repositories_from_cache is absent,
immediately before returning False. Include enough context to identify the
provider or source repository while preserving the existing fallback behavior.
Comment on lines
+515
to
+522
| all_kbs = exclude_code_wikis( | ||
| db.query(Kind).filter( | ||
| Kind.kind == "KnowledgeBase", | ||
| Kind.is_active == True, | ||
| ((Kind.user_id == user_id) & (Kind.namespace == "default")) | ||
| | (Kind.id.in_(shared_kb_ids) if shared_kb_ids else False), | ||
| ) | ||
| .all() | ||
| ) | ||
| ).all() |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
exclude_code_wikis is applied inconsistently across knowledge-base listing queries. The helper reaches six queries and misses several siblings in the same methods. A code wiki lives in the "default" namespace under the wiki account, so ownership hides it only while that account differs from the requester. When WIKI_DEFAULT_USER_ID=0, wiki_owner returns the requester and every unfiltered listing exposes the wiki, which is the case the helper's own docstring warns about.
backend/app/services/knowledge/knowledge_service.py#L515-L522: applyexclude_code_wikisto the ORGANIZATION branch at Lines 564-575, so all four branches oflist_knowledge_basesagree.backend/app/services/knowledge/knowledge_service.py#L2101-L2107: applyexclude_code_wikistopersonal_kbsat Lines 2011-2021 andorg_kbsat Lines 2138-2149 inget_accessible_knowledge, and to the equivalent queries inget_personal_knowledge_bases_groupedat Lines 2218-2228 and Lines 2251-2262 and inget_all_knowledge_bases_groupedat Lines 2793-2802 and Lines 2842-2853.backend/tests/api/test_knowledge_code_wiki.py#L411-L423: assert the absence of the code wiki in thegroupsandorganizationbuckets as well, so the coverage matches the filtered surface.
🧰 Tools
🪛 Ruff (0.16.0)
[error] 518-518: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
📍 Affects 2 files
backend/app/services/knowledge/knowledge_service.py#L515-L522(this comment)backend/app/services/knowledge/knowledge_service.py#L2101-L2107backend/tests/api/test_knowledge_code_wiki.py#L411-L423
🤖 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 `@backend/app/services/knowledge/knowledge_service.py` around lines 515 - 522,
Apply exclude_code_wikis consistently to every knowledge-base listing query: the
ORGANIZATION branch of list_knowledge_bases in
backend/app/services/knowledge/knowledge_service.py:564-575; personal_kbs and
org_kbs in get_accessible_knowledge at :2011-2021 and :2138-2149; the equivalent
queries in get_personal_knowledge_bases_grouped at :2218-2228 and :2251-2262;
and get_all_knowledge_bases_grouped at :2793-2802 and :2842-2853. The anchor
query at backend/app/services/knowledge/knowledge_service.py:515-522 already
uses the helper and requires no direct change. Extend
backend/tests/api/test_knowledge_code_wiki.py:411-423 to assert the code wiki is
absent from both groups and organization buckets.
…kipped The 2026-08-03 round went unread. Going back through every thread that is neither outdated nor resolved: **A documented scope that was not enforced.** The page-read endpoint says only your own generation is readable, but the internal dependency accepts any active user's JWT and never binds the caller to a generation — so any signed-in user could read any generation's pages, which are a copy of a wiki whose repository they may have no access to. The caller is now checked against the generation's owner; the fixed internal token stays unscoped, being an operator rather than a person. A run is now owned by the account that executes it rather than the one that asked, so that check agrees with the identity the agent authenticates as — and with the account that owns the knowledge base it publishes into. **Two branches that were never given their inputs.** decide_run_mode has had the periodic-rebuild thresholds since it was written and start_generation never passed them, so they defaulted to "no drift yet" and a wiki running on increments was never rebuilt however long it ran. Wired, with one query. The top-level-module guard is removed rather than wired. It exists for a diff filtered to documented file types, where a module of assets could vanish without appearing in the diff; ours is unfiltered, so the case it guards cannot arise, and an unwired branch is a defence that is not there. Performance, on the listing path: - readable_wiki_ids read the cache once per wiki while its docstring claimed once per host — N event loops and N Redis reads for N wikis, plus one live provider call each on a cold cache. Now keyed by host, with only the misses falling through, and a test that counts the reads so the docstring cannot drift again. - Document counts come from one grouped query rather than a COUNT per wiki, and the listing is paginated. Access is judged before paging, or a page would come back short with a wrong total. - Returning an existing wiki reported zero documents while the listing reported the real number for the same wiki. exclude_code_wikis reached six queries and missed siblings in the same methods. With WIKI_DEFAULT_USER_ID=0 the wiki account *is* the requester, so ownership hides nothing and every unfiltered listing exposed the wiki — the case the helper's own docstring warned about. Now applied to all thirteen. Also: Mermaid frontmatter no longer reads as the diagram type (the types added for the pinned version are exactly the ones that use it, so this was newly reachable); both the -beta and stable spellings are accepted rather than guessed between; a section is reported once however its prefix is cased; the skill fails loudly when it cannot derive the read URL instead of reporting the resulting 404 as "page does not exist"; and _folder_resolver drops a user_id it never used. Not taken: the Alembic graph is a single head (2b5791acc5fa) — verified, the branchpoints reconverge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/knowledge/knowledge_service.py (1)
1495-1501: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCount only browsable wiki pages.
wiki_pagesexcludes code-targetKnowledgeDocumentrows from both list operations.get_document_countandget_document_countsstill count every row. The code-wiki response and list endpoints use those unscoped helpers.A code wiki can return a page list that omits code targets but report a
document_countthat includes them. Add a scoped page-count helper for the code-wiki response paths. Keep the unscoped count for deletion checks if retrieval artifacts must still block deletion.Also applies to: 1529-1534
🤖 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 `@backend/app/services/knowledge/knowledge_service.py` around lines 1495 - 1501, Add a scoped page-count helper alongside get_document_count/get_document_counts that applies the same wiki_pages filtering used by the listing query, excluding code-target KnowledgeDocument rows. Update the code-wiki response and list paths around those helpers to use the scoped count, while keeping the existing unscoped count in deletion checks.
🤖 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 `@backend/app/api/endpoints/wiki.py`:
- Around line 93-100: Update _internal_caller to use and return the
authenticated User result from _verify_internal_token instead of re-verifying
the token or returning None on exceptions. Preserve the explicit
INTERNAL_API_TOKEN handling, but ensure any JWT validation failure propagates as
rejection so _assert_caller_owns_generation cannot bypass ownership checks.
---
Outside diff comments:
In `@backend/app/services/knowledge/knowledge_service.py`:
- Around line 1495-1501: Add a scoped page-count helper alongside
get_document_count/get_document_counts that applies the same wiki_pages
filtering used by the listing query, excluding code-target KnowledgeDocument
rows. Update the code-wiki response and list paths around those helpers to use
the scoped count, while keeping the existing unscoped count in deletion checks.
🪄 Autofix (Beta)
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: 8e2ae14f-b055-46aa-9ac3-8918dca0f8cc
📒 Files selected for processing (15)
backend/app/api/endpoints/knowledge.pybackend/app/api/endpoints/wiki.pybackend/app/services/knowledge/code_wiki/generation.pybackend/app/services/knowledge/code_wiki/mermaid_check.pybackend/app/services/knowledge/code_wiki/projection.pybackend/app/services/knowledge/code_wiki/publish_gate.pybackend/app/services/knowledge/code_wiki/read_access.pybackend/app/services/knowledge/code_wiki/run_mode.pybackend/app/services/knowledge/code_wiki/runner.pybackend/app/services/knowledge/knowledge_service.pybackend/init_data/skills/wiki_submit/wiki_submit.jsbackend/tests/services/knowledge/code_wiki/test_generation.pybackend/tests/services/knowledge/code_wiki/test_mermaid_check.pybackend/tests/services/knowledge/code_wiki/test_read_access.pybackend/tests/services/knowledge/code_wiki/test_run_mode.py
💤 Files with no reviewable changes (2)
- backend/tests/services/knowledge/code_wiki/test_run_mode.py
- backend/app/services/knowledge/code_wiki/run_mode.py
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/tests/services/knowledge/code_wiki/test_generation.py
- backend/app/services/knowledge/code_wiki/publish_gate.py
- backend/app/services/knowledge/code_wiki/runner.py
- backend/app/services/knowledge/code_wiki/mermaid_check.py
- backend/app/api/endpoints/knowledge.py
- backend/app/services/knowledge/code_wiki/projection.py
- backend/init_data/skills/wiki_submit/wiki_submit.js
Comment on lines
+93
to
+100
| _verify_internal_token(authorization=authorization, db=db) | ||
| token = authorization[7:].strip() | ||
| if token == wiki_settings.INTERNAL_API_TOKEN: | ||
| return None | ||
| try: | ||
| return security.get_current_user_from_token(token, db) | ||
| except Exception: # pragma: no cover - _verify_internal_token already accepted it | ||
| return None |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not convert a JWT verification failure into internal-token access.
Line 93 already accepts the JWT. Lines 97-100 verify it again. If that second call raises, this function returns None. _assert_caller_owns_generation then treats the caller as the fixed internal token and skips ownership checks.
Return the authenticated User from _verify_internal_token, or authenticate once in _internal_caller. If JWT validation fails, reject the request instead of returning None.
As per coding guidelines, “do not add compatibility shims or fallback paths without agreement; correct the primary path.”
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 99-99: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@backend/app/api/endpoints/wiki.py` around lines 93 - 100, Update
_internal_caller to use and return the authenticated User result from
_verify_internal_token instead of re-verifying the token or returning None on
exceptions. Preserve the explicit INTERNAL_API_TOKEN handling, but ensure any
JWT validation failure propagates as rejection so _assert_caller_owns_generation
cannot bypass ownership checks.
Source: Coding guidelines
The old wiki reader was a parallel stack: its own markdown renderer, its own copy button, its own LaTeX block, and its own Mermaid component with no error handling at all — beside a knowledge base viewer that already had all of it, including a Mermaid fallback to the raw source. This builds on the shared one. Three regions. The wiki's structure on the left, the page in the middle, its own outline on the right. The middle switches between reading and a conversation rather than splitting: they never need to be visible at once, and going back returns to the page still scrolled where it was, which opening the conversation on its own route would lose. The navigation is assembled server-side. Hierarchy is in the page paths and order is on the knowledge base, so a client merging them would be a second place for the tree to be wrong — and it would have to page through documents to do it. A section that holds pages but has none of its own comes back as a node with no document, because the publish gate allows that and the reader has to render it: a heading that expands but cannot be opened. The outline is new; nothing in the repository did this. It skips fenced blocks, where `# ` is a comment rather than a heading, and gives repeated titles distinct ids, or every entry sharing a title would scroll to the first one. The active entry is the section being read rather than the one just scrolled past. The list is one request. The old one fanned out a generations call per project — twenty projects meant twenty requests and two hundred records — to find two fields per card, which now come from the knowledge base itself. Creating asks for a repository and a name, and nothing else: a code wiki belongs to the wiki account and its repository decides who may read it, so asking anyone to place it would be asking a question whose answer is ignored. The old wiki stack goes with it rather than being left for PR5b: once the code tab stopped using it, every binding it fed was dead, and lint was right to refuse them. Also adds a seed script. Everything after the agent — gate, projection, attachment storage, index queue, reader — can be exercised without a model, because the agent's only interface is the write API, and the script plays it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The YAML reference described retrieval and summary configuration but not the fields this work added, so a knowledge base bound to a repository was not documented at all. Records them alongside the invariants that make them readable: which one decides the live version, what decides who may read, and why there is only ever one wiki per repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (3)
frontend/src/features/knowledge/code-wiki/CodeWikiCreateDialog.tsx (2)
85-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssociate the label with a real form control.
htmlFor="code-wiki-repo"points at adiv. A label cannot label adiv, so clicking the label does not move focus, and screen readers do not announce the association for the repository selector.Move the
idonto the control thatRepositorySelectorrenders, or replace theLabelwith a plain heading element and give the selector its own accessible name.🤖 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 `@frontend/src/features/knowledge/code-wiki/CodeWikiCreateDialog.tsx` around lines 85 - 96, Update the repository label and selector in CodeWikiCreateDialog so the label targets the actual form control rendered by RepositorySelector rather than the surrounding div; move the identifier onto that control if supported, or replace Label with a heading and provide RepositorySelector an accessible name.
112-120: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReset the dialog state when the user cancels.
handleSubmitcallsreset()after a successful create. The cancel button and any outside dismissal do not. The next time the dialog opens, it still shows the previous repository and name.♻️ Proposed change
<Button variant="outline" - onClick={() => onOpenChange(false)} + onClick={() => { + onOpenChange(false) + reset() + }} disabled={submitting} data-testid="code-wiki-create-cancel" >Apply the same reset in the
DialogonOpenChangehandler so dismissal by overlay or Escape also clears the state.🤖 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 `@frontend/src/features/knowledge/code-wiki/CodeWikiCreateDialog.tsx` around lines 112 - 120, Update the Dialog onOpenChange handler and cancel-button dismissal flow in CodeWikiCreateDialog so every close action, including cancel, overlay clicks, and Escape, invokes the existing reset() before or alongside onOpenChange(false). Preserve the successful-submit reset behavior and ensure reopening starts with cleared repository and name state.frontend/src/features/knowledge/code-wiki/WikiNavigation.tsx (1)
47-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGive the toggle button its own accessible name.
The toggle button and the page button in the same row both announce
node.title. A screen reader user hears two identical controls and cannot tell which one expands the section.Use a distinct, translated label that states the action.
♿ Proposed change
onClick={() => onToggle(node.path)} - aria-label={node.title} + aria-label={t( + isOpen ? 'knowledge:codeWiki.reader.collapseSection' : 'knowledge:codeWiki.reader.expandSection', + { title: node.title } + )} aria-expanded={isOpen}Add both keys to
frontend/src/i18n/locales/en/knowledge.jsonandfrontend/src/i18n/locales/zh-CN/knowledge.json.As per coding guidelines: "Add every translation key to both
src/i18n/locales/en/andsrc/i18n/locales/zh-CN/".🤖 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 `@frontend/src/features/knowledge/code-wiki/WikiNavigation.tsx` around lines 47 - 61, Update the toggle button in WikiNavigation to use a distinct translated aria-label describing the expand/collapse action rather than node.title, while preserving the page button label. Add the required translation keys to both English and zh-CN knowledge locale files and use the existing i18n access pattern.Source: Coding guidelines
🤖 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 `@backend/app/api/endpoints/knowledge.py`:
- Around line 742-755: The _readable_code_wiki function must reject
non-Code-Wiki knowledge bases before applying repository authorization. After
loading and null-checking knowledge_base, validate that
knowledge_base.spec.kbType is KnowledgeBaseType.CODE_WIKI; otherwise raise the
same 404 response, then call may_read_code_wiki only for matching code wikis.
- Around line 732-739: Add concrete type hints to both recursive navigation
helpers: in backend/app/api/endpoints/knowledge.py lines 732-739, type the
_as_page_node node parameter as PageNode; in
backend/tests/services/knowledge/code_wiki/test_navigation.py lines 67-68, type
nodes as list[PageNode] and declare the helper’s recursive shape return type.
In `@backend/app/services/knowledge/code_wiki/navigation.py`:
- Around line 97-105: In the loop over by_key.items(), rename the unused key
binding from key to _key while leaving node and the parent/roots navigation
logic unchanged.
In `@backend/scripts/seed_code_wiki.py`:
- Around line 76-92: Update the validation in the seed script before the
requester lookup and start_generation call to also require
knowledge_base.spec.kbType to equal "code_wiki". Route records with any other
type through the existing “No knowledge base” invalid-target error path, while
preserving the current missing-record and kind checks.
In `@frontend/src/app/`(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsx:
- Around line 22-24: Validate params.knowledgeBaseId before converting it in the
page component: accept only a defined string route parameter, convert it to a
number, and handle invalid or NaN values without running the knowledge-base list
request. Update the effect and not-found flow around knowledgeBaseId so requests
occur only for a valid identifier.
- Line 74: Replace the hardcoded “404” in the knowledge-base page component with
a translated `knowledge` namespace key via `useTranslation`, using
`codeWiki.reader.notFound`; add the corresponding English and zh-CN entries to
their respective `knowledge.json` locale files.
In `@frontend/src/app/`(tasks)/knowledge/page.tsx:
- Around line 69-72: Remove the stale comment immediately above the codeWikis
and createWikiOpen declarations; leave the existing state and hook logic
unchanged.
In `@frontend/src/features/knowledge/code-wiki/CodeWikiList.tsx`:
- Around line 23-27: Update formatWhen to accept the active language and pass it
to toLocaleDateString instead of relying on the browser locale. In the
CodeWikiList component, read the language from useTranslation and provide it
when calling formatWhen at the existing date-rendering call site.
In `@frontend/src/features/knowledge/code-wiki/CodeWikiReader.tsx`:
- Around line 140-151: Update CodeWikiReader’s responsive layout so
WikiNavigation remains available below the lg breakpoint through a mobile/tablet
drawer or collapsible control, while preserving the existing desktop navigation
and selection behavior. Hide PageOutline below the desktop breakpoint (lg), and
use the defined mobile (≤767px), tablet (768–1023px), and desktop (≥1024px)
responsive behavior.
In `@frontend/src/features/knowledge/code-wiki/PageOutline.tsx`:
- Around line 40-53: Update EnhancedMarkdown’s generated Markdown heading
elements to include anchor IDs, using the same slug and duplicate-numbering
logic currently implemented by collectHeadings in PageOutline. Extract or reuse
a shared slug-generation approach so both rendering and outline collection
produce identical IDs, preserving heading text and level behavior while ensuring
outline links and scroll spying target the rendered headings.
In `@frontend/src/features/knowledge/code-wiki/useCodeWikis.ts`:
- Around line 23-34: Update
frontend/src/features/knowledge/code-wiki/useCodeWikis.ts lines 23-34 in the
load callback to use the list response total and fetch all remaining pages, or
expose pagination state and controls so callers can retrieve wikis beyond the
first window. Update
frontend/src/app/(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsx lines
29-45 to resolve the wiki with the by-id API request instead of scanning the
first list response, and add a catch path that handles request failures without
incorrectly rendering the not-found branch.
In `@frontend/src/features/knowledge/code-wiki/WikiPageContent.tsx`:
- Around line 46-63: Update the document-loading effect around
getDocumentContent to request the full initial content range, using the existing
100k default or loadAllContent instead of limit 1. Replace the catch behavior
that clears the article with propagation of the read error message to the UI and
established retry/fallback path, while preserving cancellation checks and
loading cleanup.
In `@frontend/src/types/code-wiki.ts`:
- Around line 20-24: Update the Code Wiki type declaration for
last_published_commit to match last_published_at by allowing null while
preserving its existing required-field semantics and string type; use the
truthiness guard in CodeWikiList.tsx as the expected usage contract.
---
Nitpick comments:
In `@frontend/src/features/knowledge/code-wiki/CodeWikiCreateDialog.tsx`:
- Around line 85-96: Update the repository label and selector in
CodeWikiCreateDialog so the label targets the actual form control rendered by
RepositorySelector rather than the surrounding div; move the identifier onto
that control if supported, or replace Label with a heading and provide
RepositorySelector an accessible name.
- Around line 112-120: Update the Dialog onOpenChange handler and cancel-button
dismissal flow in CodeWikiCreateDialog so every close action, including cancel,
overlay clicks, and Escape, invokes the existing reset() before or alongside
onOpenChange(false). Preserve the successful-submit reset behavior and ensure
reopening starts with cleared repository and name state.
In `@frontend/src/features/knowledge/code-wiki/WikiNavigation.tsx`:
- Around line 47-61: Update the toggle button in WikiNavigation to use a
distinct translated aria-label describing the expand/collapse action rather than
node.title, while preserving the page button label. Add the required translation
keys to both English and zh-CN knowledge locale files and use the existing i18n
access pattern.
🪄 Autofix (Beta)
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: 79f00716-150d-4887-9c7b-5c7d4ddb3a96
📒 Files selected for processing (20)
backend/app/api/endpoints/knowledge.pybackend/app/schemas/knowledge.pybackend/app/services/knowledge/code_wiki/navigation.pybackend/scripts/seed_code_wiki.pybackend/tests/services/knowledge/code_wiki/test_navigation.pydocs/en/wegent/reference/yaml-specification.mddocs/zh/wegent/reference/yaml-specification.mdfrontend/src/apis/code-wiki.tsfrontend/src/app/(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsxfrontend/src/app/(tasks)/knowledge/page.tsxfrontend/src/features/knowledge/code-wiki/CodeWikiCreateDialog.tsxfrontend/src/features/knowledge/code-wiki/CodeWikiList.tsxfrontend/src/features/knowledge/code-wiki/CodeWikiReader.tsxfrontend/src/features/knowledge/code-wiki/PageOutline.tsxfrontend/src/features/knowledge/code-wiki/WikiNavigation.tsxfrontend/src/features/knowledge/code-wiki/WikiPageContent.tsxfrontend/src/features/knowledge/code-wiki/useCodeWikis.tsfrontend/src/i18n/locales/en/knowledge.jsonfrontend/src/i18n/locales/zh-CN/knowledge.jsonfrontend/src/types/code-wiki.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/schemas/knowledge.py
Comment on lines
+732
to
+739
| def _as_page_node(node) -> CodeWikiPageNode: | ||
| return CodeWikiPageNode( | ||
| path=node.path, | ||
| title=node.title, | ||
| document_id=node.document_id, | ||
| has_content=node.has_content, | ||
| children=[_as_page_node(child) for child in node.children], | ||
| ) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add concrete Python type hints at both sites.
backend/app/api/endpoints/knowledge.py#L732-L739: Typenodeas the navigationPageNode.backend/tests/services/knowledge/code_wiki/test_navigation.py#L67-L68: Typenodesaslist[PageNode]and declare the recursive shape return type.
As per coding guidelines, “Python code must follow PEP 8, Black with 88-column formatting, isort, and type hints.”
📍 Affects 2 files
backend/app/api/endpoints/knowledge.py#L732-L739(this comment)backend/tests/services/knowledge/code_wiki/test_navigation.py#L67-L68
🤖 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 `@backend/app/api/endpoints/knowledge.py` around lines 732 - 739, Add concrete
type hints to both recursive navigation helpers: in
backend/app/api/endpoints/knowledge.py lines 732-739, type the _as_page_node
node parameter as PageNode; in
backend/tests/services/knowledge/code_wiki/test_navigation.py lines 67-68, type
nodes as list[PageNode] and declare the helper’s recursive shape return type.
Source: Coding guidelines
Comment on lines
+742
to
+755
| def _readable_code_wiki(db: Session, user: User, knowledge_base_id: int) -> Kind: | ||
| """Load a code wiki the caller may read, or refuse.""" | ||
| knowledge_base = KnowledgeService._get_knowledge_base_record(db, knowledge_base_id) | ||
| if knowledge_base is None: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, detail="Code wiki not found" | ||
| ) | ||
| if not may_read_code_wiki(db, user, knowledge_base): | ||
| # Indistinguishable from missing on purpose: whether a repository has a wiki | ||
| # is itself something only its members should learn. | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, detail="Code wiki not found" | ||
| ) | ||
| return knowledge_base |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject non-Code-Wiki knowledge bases.
_readable_code_wiki accepts every active KnowledgeBase and then applies repository access rules. If a non-Code-Wiki knowledge base has a readable spec.source, this route can apply Code Wiki authorization instead of the normal knowledge-base ACL path. Check spec.kbType == KnowledgeBaseType.CODE_WIKI before may_read_code_wiki, and return the same 404 otherwise.
🤖 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 `@backend/app/api/endpoints/knowledge.py` around lines 742 - 755, The
_readable_code_wiki function must reject non-Code-Wiki knowledge bases before
applying repository authorization. After loading and null-checking
knowledge_base, validate that knowledge_base.spec.kbType is
KnowledgeBaseType.CODE_WIKI; otherwise raise the same 404 response, then call
may_read_code_wiki only for matching code wikis.
Comment on lines
+97
to
+105
| for key, node in by_key.items(): | ||
| parent_key = ( | ||
| collation_key(node.path.rsplit("/", 1)[0]) if "/" in node.path else None | ||
| ) | ||
| parent = by_key.get(parent_key) if parent_key else None | ||
| if parent is None: | ||
| roots.append(node) | ||
| else: | ||
| parent.children.append(node) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve Ruff B007.
Line 97 binds key but never reads it. Rename it to _key.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 97-97: Loop control variable key not used within loop body
Rename unused key to _key
(B007)
🤖 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 `@backend/app/services/knowledge/code_wiki/navigation.py` around lines 97 -
105, In the loop over by_key.items(), rename the unused key binding from key to
_key while leaving node and the parent/roots navigation logic unchanged.
Source: Linters/SAST tools
Comment on lines
+76
to
+92
| with SessionLocal() as db: | ||
| knowledge_base = db.get(Kind, args.kb_id) | ||
| if knowledge_base is None or knowledge_base.kind != "KnowledgeBase": | ||
| print(f"No knowledge base {args.kb_id}", file=sys.stderr) | ||
| return 1 | ||
|
|
||
| requester = db.get(User, knowledge_base.user_id) | ||
| if requester is None: | ||
| print(f"Knowledge base {args.kb_id} has no owner", file=sys.stderr) | ||
| return 1 | ||
| owner: User = wiki_owner(db, requester) | ||
| started = start_generation( | ||
| db, | ||
| knowledge_base=knowledge_base, | ||
| user=owner, | ||
| head_commit=args.commit, | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the Code Wiki type before writing.
The script accepts any KnowledgeBase. It can then start a Code Wiki generation and write generated content for a regular knowledge base. Reject records whose spec.kbType is not code_wiki with the existing invalid-target error path.
🤖 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 `@backend/scripts/seed_code_wiki.py` around lines 76 - 92, Update the
validation in the seed script before the requester lookup and start_generation
call to also require knowledge_base.spec.kbType to equal "code_wiki". Route
records with any other type through the existing “No knowledge base”
invalid-target error path, while preserving the current missing-record and kind
checks.
Comment on lines
+22
to
+24
| const params = useParams() | ||
| const router = useRouter() | ||
| const knowledgeBaseId = Number(params.knowledgeBaseId) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the route parameter before use.
useParams() types params.knowledgeBaseId as string | string[] | undefined. Number() on an array or undefined produces NaN. The effect then runs a list request that can never match, and the page shows the not-found branch.
🛡️ Proposed guard
- const knowledgeBaseId = Number(params.knowledgeBaseId)
+ const rawId = params.knowledgeBaseId
+ const knowledgeBaseId = Number(Array.isArray(rawId) ? rawId[0] : rawId)🤖 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 `@frontend/src/app/`(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsx
around lines 22 - 24, Validate params.knowledgeBaseId before converting it in
the page component: accept only a defined string route parameter, convert it to
a number, and handle invalid or NaN values without running the knowledge-base
list request. Update the effect and not-found flow around knowledgeBaseId so
requests occur only for a valid identifier.
Comment on lines
+140
to
+151
| return ( | ||
| <div className="flex min-h-0 flex-1" data-testid="code-wiki-reader"> | ||
| <div className="hidden lg:flex"> | ||
| <WikiNavigation | ||
| pages={pages} | ||
| activePath={activePath} | ||
| onSelect={node => { | ||
| setActivePath(node.path) | ||
| setMode('read') | ||
| }} | ||
| /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The reader has no page navigation below 1024px.
WikiNavigation is wrapped in hidden lg:flex. No mobile or tablet replacement exists. On a viewport under 1024px the reader opens the first readable page and the user cannot reach any other page. PageOutline on Line 218 has no responsive treatment either, so it competes for width on narrow screens.
Add a drawer or collapsible navigation for mobile and tablet, and hide PageOutline below the desktop breakpoint.
As per coding guidelines: "Split mobile and desktop components when layout or interaction differs materially; otherwise use responsive classes" and "Use responsive breakpoints of mobile <=767px, tablet 768px–1023px, and desktop >=1024px".
Also applies to: 218-218
🤖 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 `@frontend/src/features/knowledge/code-wiki/CodeWikiReader.tsx` around lines
140 - 151, Update CodeWikiReader’s responsive layout so WikiNavigation remains
available below the lg breakpoint through a mobile/tablet drawer or collapsible
control, while preserving the existing desktop navigation and selection
behavior. Hide PageOutline below the desktop breakpoint (lg), and use the
defined mobile (≤767px), tablet (768–1023px), and desktop (≥1024px) responsive
behavior.
Source: Coding guidelines
Comment on lines
+40
to
+53
| const text = match[2].replace(/[*_`]/g, '').trim() | ||
| if (!text) continue | ||
|
|
||
| // Two sections can share a title, and an id that repeats would scroll to the | ||
| // first one from every entry. | ||
| const base = text.toLowerCase().replace(/[^\w一-龥]+/g, '-') | ||
| const count = seen.get(base) ?? 0 | ||
| seen.set(base, count + 1) | ||
|
|
||
| headings.push({ | ||
| id: count === 0 ? base : `${base}-${count}`, | ||
| text, | ||
| level: Math.min(match[1].length, 3), | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline frontend/src/features/knowledge/code-wiki/WikiPageContent.tsx --items all
rg -n -C 6 'id=|slug|heading|rehype|remark|Markdown' \
frontend/src/features/knowledge/code-wiki/WikiPageContent.tsx
rg -n -C 4 'PageOutline|collectHeadings|code-wiki-outline' \
frontend/src/features/knowledge/code-wikiRepository: wecode-ai/Wegent
Length of output: 9309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- PageOutline relevant section ---\n'
sed -n '1,150p' frontend/src/features/knowledge/code-wiki/PageOutline.tsx
printf '\n--- EnhancedMarkdown files ---\n'
fd -a 'EnhancedMarkdown' . | sed 's#^\./##'
printf '\n--- search for heading id/slugs in frontend ---\n'
rg -n -C 5 'id=|slug|slugify|slugify-heading|rehypeHeading|heading|remark|markdown' frontend/src --glob '*.{ts,tsx,js,jsx}'Repository: wecode-ai/Wegent
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PageOutline relevant section ---'
sed -n '1,150p' frontend/src/features/knowledge/code-wiki/PageOutline.tsx
printf '%s\n'
printf '%s\n' '--- EnhancedMarkdown files ---'
fd -a 'EnhancedMarkdown' . | sed 's#^\./##'
printf '%s\n'
printf '%s\n' '--- search for heading id/slugs in frontend ---'
rg -n -C 5 'id=|slug|slugify|slugify-heading|rehypeHeading|heading|remark|markdown' frontend/src --glob '*.{ts,tsx,js,jsx}'Repository: wecode-ai/Wegent
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PageOutline.tsx ---'
sed -n '1,160p' frontend/src/features/knowledge/code-wiki/PageOutline.tsx
printf '%s\n' '--- EnhancedMarkdown components around markdown renderers ---'
fd -a 'EnhancedMarkdown' . | sed 's#^\./##' | while read -r f; do
echo "=== $f ==="
sed -n '1,220p' "$f"
done
printf '%s\n' '--- focused heading/slug/rheme searches in common/components ---'
rg -n -C 4 'rehypeHeading|heading|slug|remarkGfm|remarkRehype|rehypeSlug|remarkParse|Markdown|MDEditor' \
frontend/src/components frontend/src/utils frontend/src --glob '*.{ts,tsx,js,jsx}' \
--max-count 300
printf '%s\n' '--- package markdown deps ---'
rg -n '"(next-mdx-remote|remark|rehype|mdast|unified|react-markdown|marked|markdown-it|`@uiw/react-md`|`@milkdown`|`@codemirror`|`@lexical`|flexmark|mdframed|html-to-image|lucide|remark|rehype|unified|mdx)"|react|markdown|remark|rehype' package.json frontend/package.json package*.json 2>/dev/null | head -n 200 || trueRepository: wecode-ai/Wegent
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PageOutline.tsx ---'
sed -n '1,160p' frontend/src/features/knowledge/code-wiki/PageOutline.tsx
printf '%s\n' '--- EnhancedMarkdown heading component section ---'
sed -n '640,775p' frontend/src/components/common/EnhancedMarkdown.tsx
printf '%s\n' '--- focused package headings ---'
sed -n '1,220p' package.json
sed -n '1,220p' frontend/package.json 2>/dev/null || true
printf '%s\n' '--- focused slug/heading plugin occurrences ---'
rg -n -C 3 'rehypeHeading|slug|slugify|remarkHeading|rehypeSlug|ReactMarkdown|heading:|h1:h' \
frontend/src/components/common/EnhancedMarkdown.tsx frontend/src/components/common/SmartUrlRenderer.tsx frontend/src --glob '*.{ts,tsx}' \
--max-match 200Repository: wecode-ai/Wegent
Length of output: 16929
Align the outline IDs with Markdown heading IDs.
PageOutline builds heading IDs with collectHeadings, while EnhancedMarkdown renders Markdown through react-markdown without a custom heading component or slug plugin. Add anchor IDs to generated headings and reuse that slug logic in collectHeadings so outline links and scroll spying find the correct elements.
🤖 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 `@frontend/src/features/knowledge/code-wiki/PageOutline.tsx` around lines 40 -
53, Update EnhancedMarkdown’s generated Markdown heading elements to include
anchor IDs, using the same slug and duplicate-numbering logic currently
implemented by collectHeadings in PageOutline. Extract or reuse a shared
slug-generation approach so both rendering and outline collection produce
identical IDs, preserving heading text and level behavior while ensuring outline
links and scroll spying target the rendered headings.
Comment on lines
+23
to
+34
| const load = useCallback(async () => { | ||
| setLoading(true) | ||
| setError(null) | ||
| try { | ||
| const response = await codeWikiApi.list() | ||
| setWikis(response.items) | ||
| } catch (caught) { | ||
| setError(caught instanceof Error ? caught.message : String(caught)) | ||
| } finally { | ||
| setLoading(false) | ||
| } | ||
| }, []) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Both call sites treat the first page of codeWikiApi.list() as the complete set. The endpoint returns a window plus total, and neither caller reads total or requests further pages. The list omits wikis beyond the window, and the detail page reports a wiki as missing when it lies outside that window.
frontend/src/features/knowledge/code-wiki/useCodeWikis.ts#L23-L34: readtotalfrom the response and load the remaining pages, or expose pagination controls to the caller.frontend/src/app/(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsx#L29-L45: resolve the wiki through a by-id request instead of scanning one list page, and add a.catchso a request failure does not render the not-found branch.
📍 Affects 2 files
frontend/src/features/knowledge/code-wiki/useCodeWikis.ts#L23-L34(this comment)frontend/src/app/(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsx#L29-L45
🤖 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 `@frontend/src/features/knowledge/code-wiki/useCodeWikis.ts` around lines 23 -
34, Update frontend/src/features/knowledge/code-wiki/useCodeWikis.ts lines 23-34
in the load callback to use the list response total and fetch all remaining
pages, or expose pagination state and controls so callers can retrieve wikis
beyond the first window. Update
frontend/src/app/(tasks)/knowledge/code-wiki/[knowledgeBaseId]/page.tsx lines
29-45 to resolve the wiki with the by-id API request instead of scanning the
first list response, and add a catch path that handles request failures without
incorrectly rendering the not-found branch.
Comment on lines
+46
to
+63
| let cancelled = false | ||
| setLoading(true) | ||
| getDocumentContent(page.document_id, 0, 1) | ||
| .then(response => { | ||
| if (cancelled) return | ||
| const body = response.content ?? '' | ||
| setMarkdown(body) | ||
| onContentChange(body) | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled) { | ||
| setMarkdown('') | ||
| onContentChange('') | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) setLoading(false) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the document content endpoint and its offset/limit semantics.
rg -nP -C 12 'documents/\{document_id\}/content|def .*document_content' backend/appRepository: wecode-ai/Wegent
Length of output: 24544
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate WikiPageContent and calls =="
fd -a 'WikiPageContent\.tsx$' . || true
rg -n "getDocumentContent|WikiPageContent|onContentChange|setMarkdown|DocumentContentReadResponse|MAX_DOCUMENT_READ_LIMIT|offset=|limit=1" frontend backend -g '*.ts' -g '*.tsx' -g '*.py' | sed -n '1,220p'
echo "== inspect frontend component =="
file=$(fd 'WikiPageContent\.tsx$' . | head -n 1)
if [ -n "${file:-}" ]; then
wc -l "$file"
cat -n "$file"
fi
echo "== inspect relevant backend endpoint/service slice =="
sed -n '300,370p' backend/app/api/endpoints/knowledge_open.py
sed -n '830,890p' backend/app/services/knowledge/orchestrator.py
sed -n '416,450p' backend/app/mcp_server/tools/knowledge_external.py
echo "== inspect schema/default =="
rg -n "class DocumentContentReadResponse|MAX_DOCUMENT_READ_LIMIT" backend app frontend -g '*.py' -g '*.ts' -g '*.tsx' | sed -n '1,120p'Repository: wecode-ai/Wegent
Length of output: 34236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== frontend knowledge API =="
sed -n '1,180p' frontend/src/apis/knowledge.ts
sed -n '450,490p' frontend/src/types/knowledge.ts
echo "== useDocumentDetail API usage =="
sed -n '1,180p' frontend/src/features/knowledge/document/hooks/useDocumentDetail.ts
echo "== backend document read service implementation =="
sed -n '1,270p' backend/app/services/knowledge/document_read_service.py
sed -n '890,925p' backend/app/services/knowledge/orchestrator.py
sed -n '1010,1035p' backend/app/schemas/knowledge.py
echo "== deterministic slice semantics =="
python3 - <<'PY'
content = '0123456789'
limit = 1
print(repr(content[limit:limit + 1])) # 1-byte/char slice if backend copies prefix
print(repr(content[:limit + 1])) # possible first-byte read semantics
PYRepository: wecode-ai/Wegent
Length of output: 23278
Do not call getDocumentContent(..., limit=1).
offset and limit are character range parameters, so this reads only the second character when the document is 2+ characters. Request the full first range, such as the existing 100k default, or use loadAllContent.
Do not swallow document-read failures.
The current .catch renders an empty article and clears onContentChange, making failure indistinguishable from an empty document. Propagate the error message to the UI and retry/fallback path.
🛠️ Proposed change for the error handling
- .catch(() => {
+ .catch(error => {
if (!cancelled) {
setMarkdown('')
onContentChange('')
+ toast.error(error instanceof Error ? error.message : String(error))
}
})🤖 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 `@frontend/src/features/knowledge/code-wiki/WikiPageContent.tsx` around lines
46 - 63, Update the document-loading effect around getDocumentContent to request
the full initial content range, using the existing 100k default or
loadAllContent instead of limit 1. Replace the catch behavior that clears the
article with propagation of the read error message to the UI and established
retry/fallback path, while preserving cancellation checks and loading cleanup.
Comment on lines
+20
to
+24
| /** When the live version was published; null when nothing has been. */ | ||
| last_published_at?: string | null | ||
| /** Commit the live version documents. */ | ||
| last_published_commit: string | ||
| document_count: number |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align the nullability of last_published_commit with last_published_at.
last_published_at accepts null, but last_published_commit is declared as a required string. CodeWikiList.tsx Line 100 still guards it with a truthiness check, which shows the value is not always present. Declare the two fields the same way so the type matches the payload.
🔧 Proposed change
/** Commit the live version documents. */
- last_published_commit: string
+ last_published_commit?: string | nullRun the following script to confirm the backend field types:
#!/bin/bash
# Check the Code Wiki summary schema field nullability.
rg -nP -C 4 'last_published_commit|last_published_at' backend/app/schemas🤖 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 `@frontend/src/types/code-wiki.ts` around lines 20 - 24, Update the Code Wiki
type declaration for last_published_commit to match last_published_at by
allowing null while preserving its existing required-field semantics and string
type; use the truthiness guard in CodeWikiList.tsx as the expected usage
contract.
Listing repositories returned an empty list for any user whose token was bound through the UI. The ciphertext was passed to the provider API as if it were the credential, which answers 401, and two layers of `continue` turned that into `200 []` -- indistinguishable from owning no repositories. The decryption used to happen once, when the session user was loaded: get_current_user called user_service.get_user_by_name, whose last line is `return self.decrypt_user_git_info(user)`. wecode-ai#2185 replaced those three call sites with a plain query so that authentication would not depend on Git crypto configuration -- correct in itself, and likely forced by wecode-ai#2110 making GIT_TOKEN_AES_IV mandatory two days earlier, since decrypting during login would otherwise reject every session on a deployment without the IV. But the decryption was removed rather than moved, and all five providers had been relying on it. Decrypt at the provider boundary instead, in each _get_git_infos, which is the single place entries are built. The placeholder '***' and empty strings are passed through untouched: the first marks a credential a deployment overlay substitutes at call time, the second is what callers test to raise "not configured". gitee.validate_token got the same treatment; it was the only provider that did not decrypt even there. Report the domains that drop out of an aggregated result, separating a refused credential from an unreachable host. The helper lives on the base class rather than in five copies -- five copies of _get_git_infos is how one omission became five. gerrit already logged this and is unchanged. Existing provider tests could not have caught any of it: their fixtures use plaintext tokens, which pass through decrypt_token untouched. The new tests encrypt, and are parametrized over all five providers so a sixth cannot be added with the omission intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A code wiki was filed under a configured wiki account so that a repository would have one wiki outliving whoever asked for it. The knowledge-base ACL then granted nobody else access, which made repository permission the only authorisation -- and that check existed only in the endpoints written for it. Every path that reused an existing one fell back to the ACL and refused everybody: triggering a run, reading page content, chat citation, and the MCP tool. Public repositories were refused too, since the check is membership-based and never looks at visibility. Make it an ordinary knowledge base instead. The creator owns it, the ordinary ACL decides who reads it, and all four of those paths work without being taught anything. The repository is consulted once, when the wiki is created; sharing it afterwards is the creator's decision, the same as for any other knowledge base holding private material. A repository may now have several wikis, one per person who built one: a wiki created by A is invisible to B under A's ACL, so refusing B one of their own would take it away on a first-come basis. wiki_projects therefore holds one row per (repository, wiki) and the UNIQUE moves to that pair -- still a database constraint rather than a check-then-insert, and it lets COUNT(*) answer "how many wikis exist for this repository" without reading a JSON field. Triggering a run requires write access to the repository rather than merely being able to read the wiki: a run rewrites every page, so a wiki shared with a reader would otherwise let them spend a generation on somebody else's knowledge base. Creating a wiki now starts its first run, since a new wiki that sits empty until somebody finds the regenerate button is not a flow anyone would guess; a run that cannot start is logged rather than raised, because the knowledge base is already committed. read_access.py and exclude_code_wikis go with it. The second is worth naming: hiding code wikis from the general listing would also hide them from chat and the MCP tool, where being citable is the point. Spec section 6 is rewritten, including why this was reversed -- the rule was never written as a checkable invariant and had no list of enforcement points, so missing one was a matter of time rather than of attention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Creating a code wiki refused anyone without a token for the host, and the access check both providers use is membership-based and never looks at visibility. A repository anyone can read in a browser therefore could not be documented -- a wiki more closed than its own source. Two paths reach it now. Without a credential the repository is described anonymously, and being public is itself read access. With a credential that does not reach it, the refusal falls through to the same probe: "not a member" is also what both providers answer for a public repository nobody has joined, so a stale or unrelated token would otherwise make a world-readable repository undocumentable. A successful public probe is positive evidence obtained from the provider we just failed to reach with a credential, so this does not turn an unreachable provider into an open door: if it is down, the probe fails too and the refusal stands. The anonymous grant reports read access and no more, so it can never satisfy the gate on regenerating. Absent and private are reported identically throughout. GitHub answers 404 rather than 403 for a private repository an anonymous caller cannot see, and that is the behaviour to preserve: "not readable" is the whole answer a caller is entitled to. POST /code-wikis/resolve answers the three questions the create form would otherwise ask separately -- may I read it, what is its default branch, what is it called. The branch matters most: listing branches has no anonymous path, and taking the default from here means one need not be opened up. It also reports how many wikis already document the repository, so somebody can ask for a share instead of paying for a second generation. Anonymous GitHub requests are capped at 60 an hour per address, so results are cached for ten minutes, keyed by whether a credential was used -- the same repository legitimately answers differently with and without one. An unreadable result is deliberately not cached: a repository about to be granted to the caller should not stay unreadable because they asked one moment early. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dialog A code wiki is a knowledge base owned by its creator, so it belongs in the same list and the same create dialog as every other one. The separate document/code tabs went with that: they were the visible half of a separation that no longer exists, and the code tab's own list and dialog are removed rather than kept alongside. The dialog asks which kind first, because that decides which fields even apply. A code wiki has no notebook/classic opening view -- it has a reader of its own -- and it does have a repository, a branch and a generation language. Its name is optional: left blank, the server fills in the repository's own. The blank is deliberately not pre-filled in the browser, where it would read as the caller's own input. The repository can be picked or named. Both are needed: the selector's list is membership-scoped on the server, so a public repository the caller can read in a browser never appears in it, and one that could not be documented would leave the wiki more closed than its source. The shared RepositorySelector is embedded rather than extended -- task creation uses it too, and the scoping is not something a component can change. Naming a URL resolves it, which reports whether it is readable, its default branch, and how many wikis already document it. The probe is debounced: anonymous GitHub requests are capped at 60 an hour per address. KnowledgeBaseType gains 'code_wiki', and with it DocumentViewType for the four components that render one of the two document views. They stay narrow on purpose: a code wiki has neither view, so handing one to them should not typecheck. documentViewOf makes each conversion visible. Removing the tabs took the sidebar-expand button that lived in them; the document page has its own (onExpandTree), so the affordance is not lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…go unnamed Two things reported from smoke testing. Creating a wiki without a name was rejected with a 422: the schema still required one, though the whole point of leaving it blank is that the server fills in the repository's own name. It does that now, preferring what the provider calls the repository over what the URL parses to -- the provider is the authority, renames included -- and falling back to the URL when the repository could not be described. "1 other wiki already documents this repository" was not actionable: asking for a share needs somebody to ask. The wikis are named instead of counted, with their owner, and one the caller can already open is rendered as a link since there is nothing to ask for. Wikis the caller cannot open are still listed: they asked about a repository they can read, and what is disclosed is that a colleague documented it. The first version of the unnamed-wiki test could not tell the two name sources apart -- both were "wecode-ai/Wegent" -- so removing the resolved name left it passing. It now uses a name the URL does not parse to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ng a wiki Filling in a blank name on the server meant resolving the repository there, after the access gate had already asked the provider about it. That was only cheap because the create form had just resolved the same repository for the same user and warmed the cache -- an implicit coupling that costs a network round-trip whenever it does not hold, such as a cold cache or a direct API call. The client sends what it already resolved instead. The input box is still not pre-filled, which is the property worth keeping: a filled box reads as the caller's own input. The server keeps one fallback, the name parsed from the URL, so that a caller who skipped the probe does not create a knowledge base with no name at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 by CodeRabbit
New Features
Bug Fixes