docs: sync from Metrics-Library#2
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @RR-Gary-Stringham, your pull request is larger than the review limit of 150000 diff characters
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request performs a comprehensive overhaul of the Metrics Library documentation, restructuring existing files and introducing new specifications for CoStar processing, forecast/budget models, and source file ingestion. The feedback highlights several critical formatting and grammatical errors, such as missing frontmatter delimiters in the table architecture file, typos like trailing asterisks in the pricing model, and various comma splices or run-on sentences across the updated documentation.
|
|
||
| title: Metrics Table Architecture | ||
| nav_order: 3 | ||
| has_toc: true | ||
| permalink: /metrics-table-architecture/ | ||
|
|
There was a problem hiding this comment.
The frontmatter is missing the required --- delimiters, which are necessary for static site generators (like Jekyll or Hugo) to parse the metadata correctly.
---
title: Metrics Table Architecture
nav_order: 3
has_toc: true
permalink: /metrics-table-architecture/
---
| - and automation workflows can infer semantic meaning directly from structure. | ||
|
|
||
| **The objective is not shorter column names. The objective is operational clarity at scale.** No newline at end of file | ||
| **The objective is not shorter column names., the objective is operational clarity at scale.** |
There was a problem hiding this comment.
There is a typo in this line: a period followed by a comma (.,). Additionally, this sentence contains a comma splice; using a semicolon would improve clarity.
**The objective is not shorter column names; the objective is operational clarity at scale.**
| - planning inputs, | ||
| - observed market pricing, | ||
| - user-generated assumptions, | ||
| - override logic,**** |
There was a problem hiding this comment.
There are trailing asterisks (****) at the end of this line, which appear to be a typo.
- override logic,
| Because of this, the Metrics Platform models events as a reusable contextual layer that can integrate across multiple reporting datasets without duplicating event text across operational fact tables. | ||
|
|
||
| --- | ||
| **The objective is not simply attaching notes to dates, the objective is preserving the market conditions surrounding hotel performance.** |
There was a problem hiding this comment.
This sentence contains a comma splice. Using a semicolon or period would improve the grammatical structure.
**The objective is not simply attaching notes to dates; the objective is preserving the market conditions surrounding hotel performance.**
| - property-specific impact behavior, | ||
| - and forecasting continuity. | ||
|
|
||
| **The objective is not simply storing event names the objective is preserving the operational story surrounding hotel performance.** |
There was a problem hiding this comment.
This sentence is missing punctuation between "names" and "the", resulting in a run-on sentence.
**The objective is not simply storing event names; the objective is preserving the operational story surrounding hotel performance.**
| * source lineage, | ||
| * and analytical reproducibility. | ||
|
|
||
| **The goal is not simply loading files, the goal is creating a reliable operational reporting system from source data that was never designed to work together in the first place.** |
There was a problem hiding this comment.
This sentence contains a comma splice. Using a semicolon or period would improve the grammatical structure.
**The goal is not simply loading files; the goal is creating a reliable operational reporting system from source data that was never designed to work together in the first place.**
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afbd06b333
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,217 +0,0 @@ | |||
| --- | |||
There was a problem hiding this comment.
This sync deletes multiple files that are still referenced by the Metrics Library navigation in docs.json (for example metrics-library/table-prefix-standards, table-architecture, costar-processing-spec, costar-processing-guide, forecast-budget-pricing, source-file-ingestion, and column-propagation-audit), so the sidebar now points to pages that no longer exist; before this commit those paths resolved, so this introduces broken navigation/build-time page resolution failures.
Useful? React with 👍 / 👎.
| title: Metrics Table Architecture | ||
| nav_order: 3 | ||
| has_toc: true | ||
| permalink: /metrics-table-architecture/ |
There was a problem hiding this comment.
The top metadata is missing --- frontmatter delimiters, so title, nav_order, has_toc, and permalink are rendered as plain body text instead of page metadata. In this file that means /metrics-table-architecture/ is not registered as intended, which breaks links that rely on that permalink.
Useful? React with 👍 / 👎.
| * [CoStar Processing Standards](./costar-processing-standards/) | ||
| * [Forecast & Pricing Table Models](./forecast-pricing-models/) |
There was a problem hiding this comment.
These links target routes that are not produced by any file introduced in this commit: ./costar-processing-standards/ and ./forecast-pricing-models/ have no matching page file or permalink, so users clicking them from the library homepage will hit dead links.
Useful? React with 👍 / 👎.
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Request Changes
This PR modernizes the documentation structure but introduces critical issues: broken navigation links due to mismatched file references and missing YAML front matter in key new files, which will break the published documentation site.
🌟 Strengths
- Consolidates and expands documentation for the Metrics Library, improving clarity and organization.
| Priority | File | Category | Impact Summary | Anchors |
|---|---|---|---|---|
| P1 | metrics-library/index.mdx |
Architecture | Broken navigation links from mismatched references | path:docs.json |
| P1 | metrics-library/.../revrebel-bi-table-architecture.mdx |
Bug | Missing YAML front matter; page won't render | |
| P2 | metrics-library/.../column-naming-standards.mdx |
Maintainability | Typo: extra comma in closing statement |
🔍 Notable Themes
- Cross-cutting documentation synchronization issue: New files and navigation configuration are out of sync, requiring a coordinated update across
index.mdx,docs.json, and front matter to ensure site integrity.
📈 Risk Diagram
This diagram illustrates the two critical issues causing documentation site breakage: broken navigation links and missing YAML front matter.
sequenceDiagram
participant User
participant DocsSite as Docs Site
participant FS as File System
User->>DocsSite: Request index.mdx
DocsSite->>FS: Get index.mdx
FS-->>DocsSite: Returns index.mdx content
DocsSite->>FS: Follow link to ./costar-processing-standards/
FS-->>DocsSite: File not found (404)
note over DocsSite: R1(P1): Broken navigation link due to mismatched file reference
User->>DocsSite: Request revrebel-bi-table-architecture.mdx
DocsSite->>FS: Get file
FS-->>DocsSite: Returns file content without front matter
DocsSite->>DocsSite: Front matter parsing fails
note over DocsSite: R2(P1): Missing YAML front matter; page not rendered
note over DocsSite: Documentation site has broken navigation and missing pages
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| * [Database Column Naming Standards](./column-naming-standards/) | ||
| * [Metrics Table Architecture](./metrics-table-architecture/) | ||
| * [CoStar Processing Standards](./costar-processing-standards/) | ||
| * [Forecast & Pricing Table Models](./forecast-pricing-models/) | ||
| * [Events Table Model](./events-table-model/) |
There was a problem hiding this comment.
P1 | Confidence: High
The PR updates index.mdx to reference new documentation pages, but the actual file names differ from the linked paths. The links ./costar-processing-standards/ and ./forecast-pricing-models/ do not exist; the actual added files are costar_metrics_data_model_processing_guide.mdx, costar_metrics_data_model__processing_specification.mdx, and forecast-budget-and-pricing-table-models.mdx. Additionally, the docs.json navigation configuration (visible in related context) still references the removed files (costar-processing-guide, forecast-budget-pricing, table-prefix-standards, table-architecture, source-file-ingestion). This combination will cause broken navigation links and missing sidebar items on the published documentation site. A coordinated update of both index.mdx and docs.json is required to match the new file structure.
| title: Metrics Table Architecture | ||
| nav_order: 3 | ||
| has_toc: true | ||
| permalink: /metrics-table-architecture/ |
There was a problem hiding this comment.
P1 | Confidence: High
The new file revrebel-bi-table-architecture.mdx is missing the opening and closing --- YAML front matter delimiters. The provided lines show a bare title: field without the required structure. This will cause the documentation site (likely Docusaurus or similar) to either ignore the front matter entirely or fail to parse the page, resulting in missing title, navigation order, and permalink. The page may not render correctly or appear in the sidebar. Other added files (costar_metrics_data_model__processing_specification.mdx, costar_metrics_data_model_processing_guide.mdx, forecast-budget-and-pricing-table-models.mdx) also lack YAML front matter entirely, but this file has the most visible partial structure.
| title: Metrics Table Architecture | |
| nav_order: 3 | |
| has_toc: true | |
| permalink: /metrics-table-architecture/ | |
| --- | |
| title: Metrics Table Architecture | |
| nav_order: 3 | |
| has_toc: true | |
| permalink: /metrics-table-architecture/ | |
| --- | |
| # Metrics Table Architecture |
| - and automation workflows can infer semantic meaning directly from structure. | ||
|
|
||
| **The objective is not shorter column names. The objective is operational clarity at scale.** No newline at end of file | ||
| **The objective is not shorter column names., the objective is operational clarity at scale.** |
There was a problem hiding this comment.
P2 | Confidence: High
The final sentence introduces an extra comma between “column names” and “the objective”, altering the intended phrasing from two separate declarative statements into a grammatically incorrect run-on. This is a minor typographical error that detracts from the document's professionalism.
| **The objective is not shorter column names., the objective is operational clarity at scale.** | |
| **The objective is not shorter column names. The objective is operational clarity at scale.** |
1 participant
Automated sync from
REVREBEL/Metrics-Library@1b678ec833e28b73e92bbc65d927c49ab746ef6f.Source folder:
docs/Target folder:
metrics-library/Merge to publish.